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

Proposed by Leo Arias
Status: Merged
Merged at revision: 25
Proposed branch: lp:~elopio/cloudspacesclient/remove_extra_root_fields
Merge into: lp:cloudspacesclient
Diff against target: 211 lines (+77/-16)
4 files modified
src/cloudspacesclient/schemas.py (+22/-6)
src/cloudspacesclient/tests/conformance/test_cloudspaces_api.py (+19/-2)
src/cloudspacesclient/tests/unit/test_schemas.py (+35/-5)
src/cloudspacesclient/ubuntuone.py (+1/-3)
To merge this branch: bzr merge lp:~elopio/cloudspacesclient/remove_extra_root_fields
Reviewer Review Type Date Requested Status
Łukasz Czyżykowski (community) Needs Fixing
Review via email: mp+196606@code.launchpad.net

Commit message

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

To post a comment you must log in.
26. By Leo Arias

client-modified was duplicated.

27. By Leo Arias

Remove the debug print.;

28. By Leo Arias

Rename

29. By Leo Arias

Fixed pep8.

Revision history for this message
Łukasz Czyżykowski (lukasz-czyzykowski) wrote :

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

review: Needs Fixing
Revision history for this message
Leo Arias (elopio) wrote :

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

30. By Leo Arias

Updated the u1 root path.

31. By Leo Arias

Check the filename.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
55 'client_modified': {55 'client_modified': {
56 'type': 'string',56 'type': 'string',
57 # TODO ask what should be the date format. --elopio - 2013-11-2157 # TODO ask what should be the date format. --elopio - 2013-11-21
58 'format': 'date-time'58 'format': 'date-time',
59 'required': False
59 },60 },
60 'content_path': {'type': 'string'},61 'content_path': {'type': 'string'},
62 'contents': {
63 'type': 'array',
64 'required': False
65 },
66 'filename': {'type': 'string'},
61 'is_deleted': {'type': 'boolean'},67 'is_deleted': {'type': 'boolean'},
62 'is_folder': {'type': 'boolean'},68 'is_folder': {'type': 'boolean'},
63 'is_root': {'type': 'boolean'},69 'is_root': {'type': 'boolean'},
64 'node_id': {'type': 'string'},70 'node_id': {'type': 'string'},
65 'parent_node_id': {'type': ['string', 'null']},71 'parent_node_id': {
72 'type': 'string',
73 'required': False
74 },
66 'path': {'type': 'string'},75 'path': {'type': 'string'},
67 'resource_path': {'type': 'string'},76 'resource_path': {'type': 'string'},
68 'server_modified': {77 'server_modified': {
69 'type': 'string',78 'type': 'string',
70 # TODO ask what should be the date format. --elopio - 2013-11-2179 # TODO ask what should be the date format. --elopio - 2013-11-21
71 'format': 'date-time'80 'format': 'date-time',
72 },81 'required': False
73 'user_id': {'type': 'string'},82 },
74 'version': {'type': 'integer'},83 'user_id': {
84 'type': 'string',
85 'required': False
86 },
87 'version': {
88 'type': 'integer',
89 'required': False
90 },
75 'volume_id': {'type': 'string'},91 'volume_id': {'type': 'string'},
76 'x_attributes': {92 'x_attributes': {
77 'type': 'object',93 'type': 'object',
7894
=== 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 @@
1717
18"""Conformance tests for Cloudspaces API."""18"""Conformance tests for Cloudspaces API."""
1919
20import os
20import unittest21import unittest
21import urlparse22import urlparse
22from datetime import datetime23from datetime import datetime
@@ -93,19 +94,24 @@
9394
94 def test_get_root_metadata(self):95 def test_get_root_metadata(self):
95 user_rep = self.cloudspaces_api_client.get_user_representation().json()96 user_rep = self.cloudspaces_api_client.get_user_representation().json()
96 node_id = self._get_root_volume(user_rep.get('volumes')).get('node_id')97 root_volume = self._get_root_volume(user_rep.get('volumes'))
98 node_id = root_volume.get('node_id')
99 filename = os.path.split(root_volume.get('name'))[-1]
97 response = self.cloudspaces_api_client.get_metadata(node_id)100 response = self.cloudspaces_api_client.get_metadata(node_id)
98 self.assertTrue(response.ok)101 self.assertTrue(response.ok)
99 response_json = response.json()102 response_json = response.json()
100 validictory.validate(103 validictory.validate(
101 response_json, schemas.GET_METADATA_RESPONSE_SCHEMA)104 response_json, schemas.GET_METADATA_RESPONSE_SCHEMA)
105 self._assert_normal_folder_keys_not_in_root(response_json)
102 self._assert_content_path(node_id, response_json.get('content_path'))106 self._assert_content_path(node_id, response_json.get('content_path'))
103 # TODO Check that the contents are not present because the folder is107 # TODO Check that the contents are not present because the folder is
104 # empty. --elopio - 2013-11-25108 # empty. --elopio - 2013-11-25
105 self.assertTrue(response_json.get('is_folder'))109 self.assertTrue(response_json.get('is_folder'))
106 self.assertFalse(response_json.get('is_deleted'))110 self.assertFalse(response_json.get('is_deleted'))
107 self.assertTrue(response_json.get('is_root'))111 self.assertTrue(response_json.get('is_root'))
108 # TODO Check the filename field. --elopio - 2013-11-25112 # TODO The spec doesn't include it in the filename in the examples.
113 # --elopio - 2013-11-25
114 self.assertEqual(response_json.get('filename'), filename)
109 self.assertEqual(response_json.get('node_id'), node_id)115 self.assertEqual(response_json.get('node_id'), node_id)
110 # TODO The spec says the path shouldn't be returned on the root116 # TODO The spec says the path shouldn't be returned on the root
111 # metadata, but I think it's useful. Should we change the spec?117 # metadata, but I think it's useful. Should we change the spec?
@@ -113,12 +119,23 @@
113 self.assertEqual(119 self.assertEqual(
114 response_json.get('path'), self.test_server.get_root_path())120 response_json.get('path'), self.test_server.get_root_path())
115 self._assert_resource_path(node_id, response_json.get('resource_path'))121 self._assert_resource_path(node_id, response_json.get('resource_path'))
122 # TODO Check the volume_id field. --elopio - 2013-11-25
116123
117 def _get_root_volume(self, volumes):124 def _get_root_volume(self, volumes):
118 # FIXME The first volume returned is not necessarily the root.125 # FIXME The first volume returned is not necessarily the root.
119 # --elopio - 2013-11-23126 # --elopio - 2013-11-23
120 return volumes[0]127 return volumes[0]
121128
129 def _assert_normal_folder_keys_not_in_root(self, response_json):
130 normal_folder_keys_not_in_root = [
131 'client_modified', 'parent_node_id', 'server_modified', 'version',
132 'user_id'
133 ]
134 for key in normal_folder_keys_not_in_root:
135 self.assertFalse(
136 key in response_json,
137 "The root folder shouldn't include the {} key".format(key))
138
122 def _assert_content_path(self, node_id, content_path):139 def _assert_content_path(self, node_id, content_path):
123 path = urlparse.urlparse(self.service_root).path140 path = urlparse.urlparse(self.service_root).path
124 self.assertEqual(141 self.assertEqual(
125142
=== 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 @@
72_VALID_METADATA = {72_VALID_METADATA = {
73 'client_modified': '2013-11-20T05:45:13Z',73 'client_modified': '2013-11-20T05:45:13Z',
74 'content_path': '/api/content/538757639',74 'content_path': '/api/content/538757639',
75 'contents': [],
75 'is_deleted': False,76 'is_deleted': False,
76 'is_folder': True,77 'is_folder': True,
77 'is_root': False,78 'is_root': False,
79 'filename': 'Documents',
78 'node_id': '538757639',80 'node_id': '538757639',
79 'parent_node_id': '0',81 'parent_node_id': '0',
80 'path': '/Documents',82 'path': '/Documents',
@@ -91,19 +93,23 @@
91 'valid_object': _VALID_METADATA,93 'valid_object': _VALID_METADATA,
92 'required_properties': {94 'required_properties': {
93 'content_path': 'string',95 'content_path': 'string',
96 'filename': 'string',
94 'is_deleted': 'boolean',97 'is_deleted': 'boolean',
95 'is_folder': 'boolean',98 'is_folder': 'boolean',
96 'is_root': 'boolean',99 'is_root': 'boolean',
97 'node_id': 'string',100 'node_id': 'string',
98 'parent_node_id': 'string null',
99 'path': 'string',101 'path': 'string',
100 'resource_path': 'string',102 'resource_path': 'string',
103 'volume_id': 'string'
104 },
105 'optional_properties': {
106 'contents': 'array',
107 'client_modified': 'string',
108 'parent_node_id': 'string',
101 'server_modified': 'string',109 'server_modified': 'string',
110 'version': 'integer',
102 'user_id': 'string',111 'user_id': 'string',
103 'version': 'integer',
104 'volume_id': 'string'
105 },112 },
106 'optional_properties': {},
107 'allows_x_attributes': True,113 'allows_x_attributes': True,
108 'allows_extra_attributes': False114 'allows_extra_attributes': False
109}115}
@@ -160,6 +166,27 @@
160 self.assert_raises_validation_error(self.json, self.schema)166 self.assert_raises_validation_error(self.json, self.schema)
161167
162168
169class ObjectOptionalPropertiesTestCase(
170 testscenarios.TestWithScenarios, ResponseSchemaTestCase):
171
172 scenarios = [
173 ('{0} without {1}'.format(object_.get('name'), property_), dict(
174 schema=object_.get('schema'),
175 valid_object=object_.get('valid_object'),
176 property_=property_))
177 for object_ in _OBJECTS
178 for property_ in object_.get('optional_properties')
179 ]
180
181 def setUp(self):
182 super(ObjectOptionalPropertiesTestCase, self).setUp()
183 self.json = copy.deepcopy(self.valid_object)
184
185 def test_valid_json_without_optional_property(self):
186 del self.json[self.property_]
187 validictory.validate(self.json, self.schema)
188
189
163_TYPES = types = {190_TYPES = types = {
164 'string': 'test string',191 'string': 'test string',
165 'integer': 1,192 'integer': 1,
@@ -195,7 +222,10 @@
195 property_=property_,222 property_=property_,
196 value=wrong_value))223 value=wrong_value))
197 for object_ in _OBJECTS224 for object_ in _OBJECTS
198 for property_, types in object_.get('required_properties').items()225 for property_, types in (
226 object_.get('required_properties').items() +
227 object_.get('optional_properties').items()
228 )
199 for wrong_type, wrong_value in _get_wrong_types(types).items()229 for wrong_type, wrong_value in _get_wrong_types(types).items()
200 ]230 ]
201231
202232
=== 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 @@
130130
131 def get_root_path(self):131 def get_root_path(self):
132 """Return the path to the root folder."""132 """Return the path to the root folder."""
133 # TODO ask why on the root path it has a trailing / and on the volume133 return '~'
134 # names it doesn't. --elopio - 2013-11-23
135 return '~/Ubuntu One/'
136134
137 def get_new_user_volume_names(self):135 def get_new_user_volume_names(self):
138 """Return a list with the names of the volumes of a new user."""136 """Return a list with the names of the volumes of a new user."""

Subscribers

People subscribed via source and target branches

to all changes: