Merge lp:~cjohnston/ubuntu-ci-services-itself/ts-project-write-api into lp:ubuntu-ci-services-itself

[Chris Johnston]

Add a write API to the TS project app

L52/53 is just changing the name to better describe the test

[Francis Ginther (fginther)]

A binary package may not be associated with a source package, it may just refer to a binary package that is available in the archive, but not otherwise seeded in the base image.

The changes to the binary package list should only become permanent after the ticket is successfully processed. Once the ticket is completed, they can then be integrated into the package list for future builds.

For example:

After a few successful builds, the binary package list contains unity8, mir and libmir. The most recently built package will contain these packages.

A new request is made to build a source package from the autopilot source package and add python3-autopilot and python3 to the binary package list. When this builds, python3-autopilot and python3 needs to be included in the image, but it only gets promoted to the master binary package list after a successful build and test.

With two different API calls to add binary and source packages, it's unclear to me how a build request with source and binary package updates would be atomic.

review: Needs Information

[Ursula Junque (ursinha)]

> A binary package may not be associated with a source package, it may just
> refer to a binary package that is available in the archive, but not otherwise
> seeded in the base image.
>
> The changes to the binary package list should only become permanent after the
> ticket is successfully processed. Once the ticket is completed, they can then
> be integrated into the package list for future builds.

I believe we have a different understanding of what this binary package list is. In my understanding for phase 0 this would be a list of seeded/relevant binary packages that should be tested when building and testing images, but as dep8 tests are part of the sources this list would be used only as input to an external script that should come up with the sources list, hence we storing only the source packages list, not binary (as described in the docs).

This is only the initial implementation of what used to be the "project manager" component (similar to cu2d-config). In your scenario, this list would change per ticket? It seems to me that this is completely different than what was understood when we discussed the tests manager/project manager/ticket system split.

>
> For example:
>
> After a few successful builds, the binary package list contains unity8, mir
> and libmir. The most recently built package will contain these packages.
>
> A new request is made to build a source package from the autopilot source
> package and add python3-autopilot and python3 to the binary package list. When
> this builds, python3-autopilot and python3 needs to be included in the image,
> but it only gets promoted to the master binary package list after a successful
> build and test.

>
> With two different API calls to add binary and source packages, it's unclear
> to me how a build request with source and binary package updates would be
> atomic.

review: Needs Information

[Chris Johnston (cjohnston)]

On Mon, Dec 16, 2013 at 8:28 PM, Ursula Junque wrote:

> Review: Needs Information
>
> > A binary package may not be associated with a source package, it may just
> > refer to a binary package that is available in the archive, but not
> otherwise
> > seeded in the base image.
> >
> > The changes to the binary package list should only become permanent
> after the
> > ticket is successfully processed. Once the ticket is completed, they can
> then
> > be integrated into the package list for future builds.
>
>
> I believe we have a different understanding of what this binary package
> list is. In my understanding for phase 0 this would be a list of
> seeded/relevant binary packages that should be tested when building and
> testing images, but as dep8 tests are part of the sources this list would
> be used only as input to an external script that should come up with the
> sources list, hence we storing only the source packages list, not binary
> (as described in the docs).
>
> This is only the initial implementation of what used to be the "project
> manager" component (similar to cu2d-config). In your scenario, this list
> would change per ticket? It seems to me that this is completely different
> than what was understood when we discussed the tests manager/project
> manager/ticket system split.
>
>
>
I had the same understanding that Ursula did.

> >
> > For example:
> >
> > After a few successful builds, the binary package list contains unity8,
> mir
> > and libmir. The most recently built package will contain these packages.
> >
> > A new request is made to build a source package from the autopilot source
> > package and add python3-autopilot and python3 to the binary package
> list. When
> > this builds, python3-autopilot and python3 needs to be included in the
> image,
> > but it only gets promoted to the master binary package list after a
> successful
> > build and test.
>
>
> >
> > With two different API calls to add binary and source packages, it's
> unclear
> > to me how a build request with source and binary package updates would be
> > atomic.
>
>

[Francis Ginther (fginther)]

Based on our conversation, this is ok to go.

review: Approve

=== modified file 'docs/components/ticket-system.rst'
--- docs/components/ticket-system.rst	2013-12-12 16:53:14 +0000
+++ docs/components/ticket-system.rst	2013-12-16 19:00:41 +0000
@@ -219,6 +219,39 @@
  curl --dump-header - http://localhost:8000/api/v1/person/?is_team=True
  curl --dump-header - http://localhost:8000/api/v1/person/?is_team=False
 
+Project
+-------
+
+get_source_package
+~~~~~~~~~~~~~~~~~~
+
+*return all binary packages*
+
+::
+ curl --dump-header - http://localhost:8000/api/v1/sourcepackage/
+
+add_source_package
+~~~~~~~~~~~~~~~~~~
+
+::
+
+ curl --dump-header - -H "Content-Type: application/json" -X POST --data '{"name": "my-package"}' http://localhost:8000/api/v1/sourcepackage/
+
+ get_binary_package
+~~~~~~~~~~~~~~~~~~
+
+*return all binary packages*
+
+::
+ curl --dump-header - http://localhost:8000/api/v1/binarypackage/
+
+add_binary_package
+~~~~~~~~~~~~~~~~~~
+
+::
+
+ curl --dump-header - -H "Content-Type: application/json" -X POST --data '{"name": "my-package", "sourcepackage": "/api/v1/sourcepackage/1/", "seeded": "False"}' http://localhost:8000/api/v1/binarypackage/
+
 Models
 ******
 

=== modified file 'ticket_system/people/tests.py'
--- ticket_system/people/tests.py	2013-12-12 16:53:14 +0000
+++ ticket_system/people/tests.py	2013-12-16 19:00:41 +0000
@@ -90,7 +90,7 @@
         # Verify a new one has been added.
         self.assertEqual(Person.objects.filter(is_team=True).count(), 1)
 
-    def test_put_detail(self):
+    def test_patch_detail(self):
         # Grab the current data & modify it slightly.
         original_data = self.getResource(self.detail_url)
         new_data = original_data.copy()

=== modified file 'ticket_system/project/api.py'
--- ticket_system/project/api.py	2013-12-09 19:35:22 +0000
+++ ticket_system/project/api.py	2013-12-16 19:00:41 +0000
@@ -14,6 +14,7 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 from tastypie import fields
+from tastypie.authorization import Authorization
 from tastypie.resources import ModelResource
 from tastypie.constants import ALL
 from project.models import SourcePackage, BinaryPackage
@@ -23,9 +24,10 @@
 
     class Meta:
         queryset = SourcePackage.objects.all()
-        allowed_methods = ['get']
+        allowed_methods = ['get', 'post', 'patch']
+        authorization = Authorization()
         filtering = {
-            "name": ('exact', 'startswith'),
+            "name": ('exact', 'iexact', 'startswith', 'istartswith'),
         }
 
 
@@ -35,8 +37,9 @@
 
     class Meta:
         queryset = BinaryPackage.objects.all()
-        allowed_methods = ['get']
+        allowed_methods = ['get', 'post', 'patch']
+        authorization = Authorization()
         filtering = {
-            "name": ('exact', 'startswith'),
+            "name": ('exact', 'iexact', 'startswith', 'istartswith'),
             "seeded": ALL,
         }

=== modified file 'ticket_system/project/tests.py'
--- ticket_system/project/tests.py	2013-12-10 18:29:34 +0000
+++ ticket_system/project/tests.py	2013-12-16 19:00:41 +0000
@@ -14,105 +14,122 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
 from django.test import TestCase
+from model_mommy import mommy
 from ci_utils.tastypie.test import TastypieTestCase
 from project.models import SourcePackage, BinaryPackage
 
 
-def create_sourcepackage(name="awesome-package"):
-    package = SourcePackage()
-    package.name = name
-    package.save()
-    return package
-
-
-def create_binarypackage(name="awesome-binary", sourcepackage=None,
-                         seeded=False):
-    binary = BinaryPackage()
-    binary.name = name
-    binary.sourcepackage = sourcepackage
-    binary.seeded = seeded
-    binary.save()
-    return binary
-
-
 class SourcePackageModelTest(TestCase):
 
-    def test_creating_a_sourcepackage(self):
-        package = create_sourcepackage()
+    def setUp(self):
+        self.sourcepackage = mommy.make('SourcePackage')
+
+    def test_creating_sourcepackage(self):
         package_in_database = SourcePackage.objects.all()
         self.assertEquals(len(package_in_database), 1)
         only_package_in_database = package_in_database[0]
-        self.assertEquals(only_package_in_database, package)
+        self.assertEquals(only_package_in_database, self.sourcepackage)
 
-        self.assertEquals(only_package_in_database.name, "awesome-package")
+        self.assertEquals(only_package_in_database.name,
+                          self.sourcepackage.name)
 
 
 class BinaryPackageModelTest(TestCase):
 
     def setUp(self):
-        self.sourcepackage = create_sourcepackage()
-        self.binarypackage = create_binarypackage(
-            sourcepackage=self.sourcepackage)
+        self.sourcepackage = mommy.make('SourcePackage')
+        self.binarypackage = mommy.make('BinaryPackage',
+                                        sourcepackage=self.sourcepackage)
 
-    def test_creating_a_binarypackage(self):
+    def test_creating_binarypackage(self):
         binarypackage_in_database = BinaryPackage.objects.all()
         self.assertEquals(len(binarypackage_in_database), 1)
         only_binarypackage_in_database = binarypackage_in_database[0]
         self.assertEquals(only_binarypackage_in_database, self.binarypackage)
 
         self.assertEquals(only_binarypackage_in_database.name,
-                          "awesome-binary")
+                          self.binarypackage.name)
 
 
 class SourcePackageResourceTest(TastypieTestCase):
 
     def setUp(self):
         super(SourcePackageResourceTest, self).setUp('/api/v1')
-        create_sourcepackage()
-        self.sourcepackage_1 = SourcePackage.objects.get(
-            name='awesome-package')
-        self.detail_url = 'sourcepackage/{0}/'.format(self.sourcepackage_1.pk)
+        self.sourcepackage = mommy.make('SourcePackage')
+        self.detail_url = 'sourcepackage/{0}/'.format(self.sourcepackage.pk)
+        self.post_sourcepackage_data = {
+            'name': 'ubuntu-ci',
+        }
 
     def test_get_sourcepackage_list_json(self):
         obj = self.getResource('sourcepackage/')
         self.assertEqual(obj['objects'][0], {
-            u'id': self.sourcepackage_1.pk,
-            u'name': u'awesome-package',
+            u'id': self.sourcepackage.pk,
+            u'name': unicode(self.sourcepackage.name),
             u'resource_uri': u'/api/v1/sourcepackage/{0}/'.format(
-                self.sourcepackage_1.pk)
+                self.sourcepackage.pk)
         })
 
     def test_get_sourcepackage_detail_json(self):
         obj = self.getResource(self.detail_url)
         # We use ``assertKeys`` here to just verify the keys, not all the data.
         self.assertKeys(obj, ['id', 'name', 'resource_uri'])
-        self.assertEqual(obj['name'], 'awesome-package')
+        self.assertEqual(obj['name'], self.sourcepackage.name)
+
+    def test_post_sourcepackage(self):
+        # Check how many are there first.
+        self.assertEqual(SourcePackage.objects.count(), 1)
+        self.post(resource='sourcepackage/',
+                  params=self.post_sourcepackage_data)
+        # Verify a new one has been added.
+        self.assertEqual(SourcePackage.objects.count(), 2)
+
+    def test_patch_detail(self):
+        # Grab the current data & modify it slightly.
+        original_data = self.getResource(self.detail_url)
+        new_data = original_data.copy()
+        new_data['name'] = 'ci-ubuntu'
+
+        self.assertEqual(SourcePackage.objects.count(), 1)
+        self.patch(resource=self.detail_url, params=new_data)
+        # Make sure the count hasn't changed & we did an update.
+        self.assertEqual(SourcePackage.objects.count(), 1)
+        # Check for updated data.
+        self.assertEqual(SourcePackage.objects.get(
+                         pk=self.sourcepackage.pk).name, new_data['name'])
+
+    def test_delete_sourcepackage_not_allowed(self):
+        resp = self.delete(resource=self.detail_url)
+        self.assertHttpMethodNotAllowed(resp)
 
 
 class BinaryPackageResourceTest(TastypieTestCase):
 
     def setUp(self):
         super(BinaryPackageResourceTest, self).setUp('/api/v1')
-        create_sourcepackage()
-        self.sourcepackage_1 = SourcePackage.objects.get(
-            name='awesome-package')
-        create_binarypackage(sourcepackage=self.sourcepackage_1)
-        self.binarypackage_1 = BinaryPackage.objects.get(name='awesome-binary')
-        self.detail_url = 'binarypackage/{0}/'.format(self.binarypackage_1.pk)
+        self.sourcepackage = mommy.make('SourcePackage')
+        self.binarypackage = mommy.make('BinaryPackage',
+                                        sourcepackage=self.sourcepackage)
+        self.detail_url = 'binarypackage/{0}/'.format(self.binarypackage.pk)
+        self.post_binarypackage_data = {
+            'name': 'ubuntu-ci',
+            'sourcepackage': '/api/v1/sourcepackage/{0}/'.format(
+                self.sourcepackage.pk)
+        }
 
     def test_get_binarypackage_list_json(self):
         obj = self.getResource('binarypackage/')
         self.assertEqual(obj['objects'][0], {
-            u'id': self.binarypackage_1.pk,
-            u'name': u'awesome-binary',
-            u'seeded': False,
+            u'id': self.binarypackage.pk,
+            u'name': unicode(self.binarypackage.name),
+            u'seeded': self.binarypackage.seeded,
             u'sourcepackage': {
-                u'id': self.sourcepackage_1.pk,
-                u'name': u'awesome-package',
+                u'id': self.sourcepackage.pk,
+                u'name': unicode(self.sourcepackage.name),
                 u'resource_uri': u'/api/v1/sourcepackage/{0}/'.format(
-                    self.sourcepackage_1.pk)},
+                    self.sourcepackage.pk)},
             u'resource_uri': u'/api/v1/binarypackage/{0}/'.format(
-                self.binarypackage_1.pk)
+                self.binarypackage.pk)
         })
 
     def test_get_binarypackage_detail_json(self):
@@ -121,4 +138,30 @@
         # We use ``assertKeys`` here to just verify the keys, not all the data.
         self.assertKeys(obj, ['id', 'name', 'resource_uri', 'seeded',
                         'sourcepackage'])
-        self.assertEqual(obj['name'], 'awesome-binary')
+        self.assertEqual(obj['name'], self.binarypackage.name)
+
+    def test_post_binarypackage(self):
+        # Check how many are there first.
+        self.assertEqual(BinaryPackage.objects.count(), 1)
+        self.post(resource='binarypackage/',
+                  params=self.post_binarypackage_data)
+        # Verify a new one has been added.
+        self.assertEqual(BinaryPackage.objects.count(), 2)
+
+    def test_patch_detail(self):
+        # Grab the current data & modify it slightly.
+        original_data = self.getResource(self.detail_url)
+        new_data = original_data.copy()
+        new_data['name'] = 'ci-ubuntu'
+
+        self.assertEqual(BinaryPackage.objects.count(), 1)
+        self.patch(resource=self.detail_url, params=new_data)
+        # Make sure the count hasn't changed & we did an update.
+        self.assertEqual(BinaryPackage.objects.count(), 1)
+        # Check for updated data.
+        self.assertEqual(BinaryPackage.objects.get(
+                         pk=self.binarypackage.pk).name, new_data['name'])
+
+    def test_delete_binarypackage_not_allowed(self):
+        resp = self.delete(resource=self.detail_url)
+        self.assertHttpMethodNotAllowed(resp)