Bug#761083: #761083 - debsources: inject binary packages metadata into the DB

[Matthieu Caneill <matt@brokenwa.re>]

Hi Luciano,

On Thu, Jun 09, 2016 at 12:01:09PM +0200, Luciano Bello wrote:
> In the security team we would like to give information about which packages 
> you should update when we release a DSA (currently, we give to the user the 
> source package name). It would be easier for us if we have a way to get the 
> binaries for packages in (old)stable. Sources.d.n is the way to go, I think. 
> For that, this bug needs to be done :)

I don't mind at all you using sources.d.n for that purpose, but why
not tracker.d.o?

Code comments below:

> diff --git a/debsources/app/infobox.py b/debsources/app/infobox.py
> index dd6585c..3f75131 100644
> --- a/debsources/app/infobox.py
> +++ b/debsources/app/infobox.py
> @@ -14,7 +14,7 @@ from __future__ import absolute_import
>  from flask import url_for, current_app
>  
>  from debsources.models import (
> -    PackageName, Package, Suite, SlocCount, Metric, Ctag)
> +    PackageName, Package, Suite, SlocCount, Metric, Ctag, Binary, BinaryName)
>  from debsources.excepts import Http500Error, Http404Error
>  
>  PTS_PREFIX = "https://tracker.debian.org/pkg/";
> @@ -63,6 +63,22 @@ class Infobox(object):
>  
>          return [x[0] for x in suites]
>  
> +    def _get_binary_names(self):
> +        """ binary package names """
> +        try:
> +            print self.package, self.version

unnecessary debug line?

> +            binaries = (self.session.query(BinaryName.name)
> +                        .filter(PackageName.name == self.package,
> +                              Package.version == self.version,
> +                              Package.name_id == PackageName.id,
> +                              Binary.package_id == Package.id,
> +                              Binary.name_id == BinaryName.id)
> +                        .all())
> +        except Exception as e:  # pragma: no cover
> +            raise Http500Error(e)
> +
> +        return [x[0] for x in binaries]
> +
>      def _get_sloc(self):
>          """ sloccount """
>          try:
> @@ -160,3 +176,9 @@ class Infobox(object):
>              pkg_infos['copyright'] = False
>  
>          return pkg_infos
> +
> +    def get_binaries(self):
> +        """
> +        Retrieves the binary package names, in a list.
> +        """
> +        return self._get_binary_names()

I don't understand why you don't store the binary names in pkg_infos,
instead of creating a different method?

> diff --git a/debsources/app/views.py b/debsources/app/views.py
> index e241f83..8ede294 100644
> --- a/debsources/app/views.py
> +++ b/debsources/app/views.py
> @@ -510,7 +510,9 @@ class PackageVersionsView(GeneralView):
>  # INFO PAGES #
>  class InfoPackageView(GeneralView):
>      def get_objects(self, package, version):
> -        pkg_infos = Infobox(session, package, version).get_infos()
> +        ib = Infobox(session, package, version)
> +        pkg_infos = ib.get_infos()
> +        binaries = ib.get_binaries()

Same remark as above, having everything in pkg_infos.get_infos would
be simpler IMHO. Unless I'm missing something?

>          return dict(pkg_infos=pkg_infos,
>                      package=package,
> -                    version=version)
> +                    version=version,binaries=binaries)
> diff --git a/debsources/db_storage.py b/debsources/db_storage.py
> index 7f58c10..e6ed437 100644
> --- a/debsources/db_storage.py
> +++ b/debsources/db_storage.py
> @@ -14,7 +14,7 @@ from __future__ import absolute_import
>  import logging
>  
>  from debsources import fs_storage
> -from debsources.models import File, Package, PackageName, SuiteInfo, Suite
> +from debsources.models import File, Package, PackageName, SuiteInfo, Suite, BinaryName, Binary
>  from debsources.models import VCS_TYPES
>  
>  
> @@ -65,6 +65,21 @@ def add_package(session, pkg, pkgdir, sticky=False):
>              session.flush()
>              file_table[relpath] = file_.id
>  
> +        for binpkg in pkg['binary'].split(', '):
> +            logging.debug('add to db %s (binary) ...' % binpkg)
> +
> +            binary_name = session.query(BinaryName) \
> +                              .filter_by(name=binpkg) \
> +                              .first()
> +
> +            if not binary_name:
> +                binary_name = BinaryName(binpkg)
> +                session.add(binary_name)
> +                session.flush()
> +
> +            session.add(Binary(name_id=binary_name.id,package_id=db_package.id))
> +
> +        session.flush()
>          return file_table
>  
>  
> diff --git a/debsources/models.py b/debsources/models.py
> index c0e4583..468d28b 100644
> --- a/debsources/models.py
> +++ b/debsources/models.py
> @@ -207,8 +207,10 @@ class Binary(Base):
>                          ForeignKey('packages.id', ondelete="CASCADE"),
>                          index=True, nullable=False)
>  
> -    def __init__(self, version, area="main"):
> -        self.version = version
> +    def __init__(self, name_id, package_id, version=None):
> +        self.version    = version
> +        self.name_id    = name_id
> +        self.package_id = package_id
>  
>      def __repr__(self):
>          return self.version

Overall, looks really good! If you fix the (very small) comments and
add a test case I'll happily merge it.
Thanks for your work!

Cheers,
--
Matthieu