Merge lp:~elopio/cloudspacesclient/remove_extra_root_fields into lp:cloudspacesclient

[Leo Arias]

Check that the fields that shouldn't be present on the root metadata are not present. Check that the filename is present.

[lp:~elopio/cloudspacesclient/remove_extra_root_fields]

26. By Leo Arias on 2013-11-25

client-modified was duplicated.

27. By Leo Arias on 2013-11-25

Remove the debug print.;

28. By Leo Arias on 2013-11-25

Rename

29. By Leo Arias on 2013-11-25

Fixed pep8.

[Łukasz Czyżykowski (lukasz-czyzykowski)]

Small change needed, now that we have filename field too: http://paste.ubuntu.com/6478476/

review: Needs Fixing

[Leo Arias (elopio)]

Updated. I'm going to merge, thank you!

[lp:~elopio/cloudspacesclient/remove_extra_root_fields]

30. By Leo Arias on 2013-11-26

Updated the u1 root path.

31. By Leo Arias on 2013-11-26

Check the filename.

=== modified file 'src/cloudspacesclient/schemas.py'
--- src/cloudspacesclient/schemas.py	2013-11-22 09:22:15 +0000
+++ src/cloudspacesclient/schemas.py	2013-11-26 16:41:32 +0000
@@ -55,23 +55,39 @@
         'client_modified': {
             'type': 'string',
             # TODO ask what should be the date format. --elopio - 2013-11-21
-            'format': 'date-time'
+            'format': 'date-time',
+            'required': False
         },
         'content_path': {'type': 'string'},
+        'contents': {
+            'type': 'array',
+            'required': False
+        },
+        'filename': {'type': 'string'},
         'is_deleted': {'type': 'boolean'},
         'is_folder': {'type': 'boolean'},
         'is_root': {'type': 'boolean'},
         'node_id': {'type': 'string'},
-        'parent_node_id': {'type': ['string', 'null']},
+        'parent_node_id': {
+            'type': 'string',
+            'required': False
+        },
         'path': {'type': 'string'},
         'resource_path': {'type': 'string'},
         'server_modified': {
             'type': 'string',
             # TODO ask what should be the date format. --elopio - 2013-11-21
-            'format': 'date-time'
-        },
-        'user_id': {'type': 'string'},
-        'version': {'type': 'integer'},
+            'format': 'date-time',
+            'required': False
+        },
+        'user_id': {
+            'type': 'string',
+            'required': False
+        },
+        'version': {
+            'type': 'integer',
+            'required': False
+        },
         'volume_id': {'type': 'string'},
         'x_attributes': {
             'type': 'object',

=== modified file 'src/cloudspacesclient/tests/conformance/test_cloudspaces_api.py'
--- src/cloudspacesclient/tests/conformance/test_cloudspaces_api.py	2013-11-25 14:53:15 +0000
+++ src/cloudspacesclient/tests/conformance/test_cloudspaces_api.py	2013-11-26 16:41:32 +0000
@@ -17,6 +17,7 @@
 
 """Conformance tests for Cloudspaces API."""
 
+import os
 import unittest
 import urlparse
 from datetime import datetime
@@ -93,19 +94,24 @@
 
     def test_get_root_metadata(self):
         user_rep = self.cloudspaces_api_client.get_user_representation().json()
-        node_id = self._get_root_volume(user_rep.get('volumes')).get('node_id')
+        root_volume = self._get_root_volume(user_rep.get('volumes'))
+        node_id = root_volume.get('node_id')
+        filename = os.path.split(root_volume.get('name'))[-1]
         response = self.cloudspaces_api_client.get_metadata(node_id)
         self.assertTrue(response.ok)
         response_json = response.json()
         validictory.validate(
             response_json, schemas.GET_METADATA_RESPONSE_SCHEMA)
+        self._assert_normal_folder_keys_not_in_root(response_json)
         self._assert_content_path(node_id, response_json.get('content_path'))
         # TODO Check that the contents are not present because the folder is
         # empty. --elopio - 2013-11-25
         self.assertTrue(response_json.get('is_folder'))
         self.assertFalse(response_json.get('is_deleted'))
         self.assertTrue(response_json.get('is_root'))
-        # TODO Check the filename field. --elopio - 2013-11-25
+        # TODO The spec doesn't include it in the filename in the examples.
+        # --elopio - 2013-11-25
+        self.assertEqual(response_json.get('filename'), filename)
         self.assertEqual(response_json.get('node_id'), node_id)
         # TODO The spec says the path shouldn't be returned on the root
         # metadata, but I think it's useful. Should we change the spec?
@@ -113,12 +119,23 @@
         self.assertEqual(
             response_json.get('path'), self.test_server.get_root_path())
         self._assert_resource_path(node_id, response_json.get('resource_path'))
+        # TODO Check the volume_id field. --elopio - 2013-11-25
 
     def _get_root_volume(self, volumes):
         # FIXME The first volume returned is not necessarily the root.
         # --elopio - 2013-11-23
         return volumes[0]
 
+    def _assert_normal_folder_keys_not_in_root(self, response_json):
+        normal_folder_keys_not_in_root = [
+            'client_modified', 'parent_node_id', 'server_modified', 'version',
+            'user_id'
+        ]
+        for key in normal_folder_keys_not_in_root:
+            self.assertFalse(
+                key in response_json,
+                "The root folder shouldn't include the {} key".format(key))
+
     def _assert_content_path(self, node_id, content_path):
         path = urlparse.urlparse(self.service_root).path
         self.assertEqual(

=== modified file 'src/cloudspacesclient/tests/unit/test_schemas.py'
--- src/cloudspacesclient/tests/unit/test_schemas.py	2013-11-22 09:22:15 +0000
+++ src/cloudspacesclient/tests/unit/test_schemas.py	2013-11-26 16:41:32 +0000
@@ -72,9 +72,11 @@
 _VALID_METADATA = {
     'client_modified': '2013-11-20T05:45:13Z',
     'content_path': '/api/content/538757639',
+    'contents': [],
     'is_deleted': False,
     'is_folder': True,
     'is_root': False,
+    'filename': 'Documents',
     'node_id': '538757639',
     'parent_node_id': '0',
     'path': '/Documents',
@@ -91,19 +93,23 @@
     'valid_object': _VALID_METADATA,
     'required_properties': {
         'content_path': 'string',
+        'filename': 'string',
         'is_deleted': 'boolean',
         'is_folder': 'boolean',
         'is_root': 'boolean',
         'node_id': 'string',
-        'parent_node_id': 'string null',
         'path': 'string',
         'resource_path': 'string',
+        'volume_id': 'string'
+    },
+    'optional_properties': {
+        'contents': 'array',
+        'client_modified': 'string',
+        'parent_node_id': 'string',
         'server_modified': 'string',
+        'version': 'integer',
         'user_id': 'string',
-        'version': 'integer',
-        'volume_id': 'string'
     },
-    'optional_properties': {},
     'allows_x_attributes': True,
     'allows_extra_attributes': False
 }
@@ -160,6 +166,27 @@
         self.assert_raises_validation_error(self.json, self.schema)
 
 
+class ObjectOptionalPropertiesTestCase(
+        testscenarios.TestWithScenarios, ResponseSchemaTestCase):
+
+    scenarios = [
+        ('{0} without {1}'.format(object_.get('name'), property_), dict(
+            schema=object_.get('schema'),
+            valid_object=object_.get('valid_object'),
+            property_=property_))
+        for object_ in _OBJECTS
+        for property_ in object_.get('optional_properties')
+    ]
+
+    def setUp(self):
+        super(ObjectOptionalPropertiesTestCase, self).setUp()
+        self.json = copy.deepcopy(self.valid_object)
+
+    def test_valid_json_without_optional_property(self):
+        del self.json[self.property_]
+        validictory.validate(self.json, self.schema)
+
+
 _TYPES = types = {
     'string': 'test string',
     'integer': 1,
@@ -195,7 +222,10 @@
              property_=property_,
              value=wrong_value))
         for object_ in _OBJECTS
-        for property_, types in object_.get('required_properties').items()
+        for property_, types in (
+            object_.get('required_properties').items() +
+            object_.get('optional_properties').items()
+        )
         for wrong_type, wrong_value in _get_wrong_types(types).items()
     ]
 

=== modified file 'src/cloudspacesclient/ubuntuone.py'
--- src/cloudspacesclient/ubuntuone.py	2013-11-24 05:49:47 +0000
+++ src/cloudspacesclient/ubuntuone.py	2013-11-26 16:41:32 +0000
@@ -130,9 +130,7 @@
 
     def get_root_path(self):
         """Return the path to the root folder."""
-        # TODO ask why on the root path it has a trailing / and on the volume
-        # names it doesn't. --elopio - 2013-11-23
-        return '~/Ubuntu One/'
+        return '~'
 
     def get_new_user_volume_names(self):
         """Return a list with the names of the volumes of a new user."""