Merge lp:~elopio/cloudspacesclient/remove_extra_root_fields into lp:cloudspacesclient
- remove_extra_root_fields
- Merge into trunk
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 |
Related bugs: |
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.
Description of the change
To post a comment you must log in.
Revision history for this message
Łukasz Czyżykowski (lukasz-czyzykowski) wrote : | # |
review:
Needs Fixing
Revision history for this message
Leo Arias (elopio) wrote : | # |
Updated. I'm going to merge, thank you!
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'src/cloudspacesclient/schemas.py' | |||
2 | --- src/cloudspacesclient/schemas.py 2013-11-22 09:22:15 +0000 | |||
3 | +++ src/cloudspacesclient/schemas.py 2013-11-26 16:41:32 +0000 | |||
4 | @@ -55,23 +55,39 @@ | |||
5 | 55 | 'client_modified': { | 55 | 'client_modified': { |
6 | 56 | 'type': 'string', | 56 | 'type': 'string', |
7 | 57 | # TODO ask what should be the date format. --elopio - 2013-11-21 | 57 | # TODO ask what should be the date format. --elopio - 2013-11-21 |
9 | 58 | 'format': 'date-time' | 58 | 'format': 'date-time', |
10 | 59 | 'required': False | ||
11 | 59 | }, | 60 | }, |
12 | 60 | 'content_path': {'type': 'string'}, | 61 | 'content_path': {'type': 'string'}, |
13 | 62 | 'contents': { | ||
14 | 63 | 'type': 'array', | ||
15 | 64 | 'required': False | ||
16 | 65 | }, | ||
17 | 66 | 'filename': {'type': 'string'}, | ||
18 | 61 | 'is_deleted': {'type': 'boolean'}, | 67 | 'is_deleted': {'type': 'boolean'}, |
19 | 62 | 'is_folder': {'type': 'boolean'}, | 68 | 'is_folder': {'type': 'boolean'}, |
20 | 63 | 'is_root': {'type': 'boolean'}, | 69 | 'is_root': {'type': 'boolean'}, |
21 | 64 | 'node_id': {'type': 'string'}, | 70 | 'node_id': {'type': 'string'}, |
23 | 65 | 'parent_node_id': {'type': ['string', 'null']}, | 71 | 'parent_node_id': { |
24 | 72 | 'type': 'string', | ||
25 | 73 | 'required': False | ||
26 | 74 | }, | ||
27 | 66 | 'path': {'type': 'string'}, | 75 | 'path': {'type': 'string'}, |
28 | 67 | 'resource_path': {'type': 'string'}, | 76 | 'resource_path': {'type': 'string'}, |
29 | 68 | 'server_modified': { | 77 | 'server_modified': { |
30 | 69 | 'type': 'string', | 78 | 'type': 'string', |
31 | 70 | # TODO ask what should be the date format. --elopio - 2013-11-21 | 79 | # TODO ask what should be the date format. --elopio - 2013-11-21 |
36 | 71 | 'format': 'date-time' | 80 | 'format': 'date-time', |
37 | 72 | }, | 81 | 'required': False |
38 | 73 | 'user_id': {'type': 'string'}, | 82 | }, |
39 | 74 | 'version': {'type': 'integer'}, | 83 | 'user_id': { |
40 | 84 | 'type': 'string', | ||
41 | 85 | 'required': False | ||
42 | 86 | }, | ||
43 | 87 | 'version': { | ||
44 | 88 | 'type': 'integer', | ||
45 | 89 | 'required': False | ||
46 | 90 | }, | ||
47 | 75 | 'volume_id': {'type': 'string'}, | 91 | 'volume_id': {'type': 'string'}, |
48 | 76 | 'x_attributes': { | 92 | 'x_attributes': { |
49 | 77 | 'type': 'object', | 93 | 'type': 'object', |
50 | 78 | 94 | ||
51 | === modified file 'src/cloudspacesclient/tests/conformance/test_cloudspaces_api.py' | |||
52 | --- src/cloudspacesclient/tests/conformance/test_cloudspaces_api.py 2013-11-25 14:53:15 +0000 | |||
53 | +++ src/cloudspacesclient/tests/conformance/test_cloudspaces_api.py 2013-11-26 16:41:32 +0000 | |||
54 | @@ -17,6 +17,7 @@ | |||
55 | 17 | 17 | ||
56 | 18 | """Conformance tests for Cloudspaces API.""" | 18 | """Conformance tests for Cloudspaces API.""" |
57 | 19 | 19 | ||
58 | 20 | import os | ||
59 | 20 | import unittest | 21 | import unittest |
60 | 21 | import urlparse | 22 | import urlparse |
61 | 22 | from datetime import datetime | 23 | from datetime import datetime |
62 | @@ -93,19 +94,24 @@ | |||
63 | 93 | 94 | ||
64 | 94 | def test_get_root_metadata(self): | 95 | def test_get_root_metadata(self): |
65 | 95 | user_rep = self.cloudspaces_api_client.get_user_representation().json() | 96 | user_rep = self.cloudspaces_api_client.get_user_representation().json() |
67 | 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')) |
68 | 98 | node_id = root_volume.get('node_id') | ||
69 | 99 | filename = os.path.split(root_volume.get('name'))[-1] | ||
70 | 97 | response = self.cloudspaces_api_client.get_metadata(node_id) | 100 | response = self.cloudspaces_api_client.get_metadata(node_id) |
71 | 98 | self.assertTrue(response.ok) | 101 | self.assertTrue(response.ok) |
72 | 99 | response_json = response.json() | 102 | response_json = response.json() |
73 | 100 | validictory.validate( | 103 | validictory.validate( |
74 | 101 | response_json, schemas.GET_METADATA_RESPONSE_SCHEMA) | 104 | response_json, schemas.GET_METADATA_RESPONSE_SCHEMA) |
75 | 105 | self._assert_normal_folder_keys_not_in_root(response_json) | ||
76 | 102 | self._assert_content_path(node_id, response_json.get('content_path')) | 106 | self._assert_content_path(node_id, response_json.get('content_path')) |
77 | 103 | # TODO Check that the contents are not present because the folder is | 107 | # TODO Check that the contents are not present because the folder is |
78 | 104 | # empty. --elopio - 2013-11-25 | 108 | # empty. --elopio - 2013-11-25 |
79 | 105 | self.assertTrue(response_json.get('is_folder')) | 109 | self.assertTrue(response_json.get('is_folder')) |
80 | 106 | self.assertFalse(response_json.get('is_deleted')) | 110 | self.assertFalse(response_json.get('is_deleted')) |
81 | 107 | self.assertTrue(response_json.get('is_root')) | 111 | self.assertTrue(response_json.get('is_root')) |
83 | 108 | # TODO Check the filename field. --elopio - 2013-11-25 | 112 | # TODO The spec doesn't include it in the filename in the examples. |
84 | 113 | # --elopio - 2013-11-25 | ||
85 | 114 | self.assertEqual(response_json.get('filename'), filename) | ||
86 | 109 | self.assertEqual(response_json.get('node_id'), node_id) | 115 | self.assertEqual(response_json.get('node_id'), node_id) |
87 | 110 | # TODO The spec says the path shouldn't be returned on the root | 116 | # TODO The spec says the path shouldn't be returned on the root |
88 | 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? |
89 | @@ -113,12 +119,23 @@ | |||
90 | 113 | self.assertEqual( | 119 | self.assertEqual( |
91 | 114 | response_json.get('path'), self.test_server.get_root_path()) | 120 | response_json.get('path'), self.test_server.get_root_path()) |
92 | 115 | self._assert_resource_path(node_id, response_json.get('resource_path')) | 121 | self._assert_resource_path(node_id, response_json.get('resource_path')) |
93 | 122 | # TODO Check the volume_id field. --elopio - 2013-11-25 | ||
94 | 116 | 123 | ||
95 | 117 | def _get_root_volume(self, volumes): | 124 | def _get_root_volume(self, volumes): |
96 | 118 | # FIXME The first volume returned is not necessarily the root. | 125 | # FIXME The first volume returned is not necessarily the root. |
97 | 119 | # --elopio - 2013-11-23 | 126 | # --elopio - 2013-11-23 |
98 | 120 | return volumes[0] | 127 | return volumes[0] |
99 | 121 | 128 | ||
100 | 129 | def _assert_normal_folder_keys_not_in_root(self, response_json): | ||
101 | 130 | normal_folder_keys_not_in_root = [ | ||
102 | 131 | 'client_modified', 'parent_node_id', 'server_modified', 'version', | ||
103 | 132 | 'user_id' | ||
104 | 133 | ] | ||
105 | 134 | for key in normal_folder_keys_not_in_root: | ||
106 | 135 | self.assertFalse( | ||
107 | 136 | key in response_json, | ||
108 | 137 | "The root folder shouldn't include the {} key".format(key)) | ||
109 | 138 | |||
110 | 122 | def _assert_content_path(self, node_id, content_path): | 139 | def _assert_content_path(self, node_id, content_path): |
111 | 123 | path = urlparse.urlparse(self.service_root).path | 140 | path = urlparse.urlparse(self.service_root).path |
112 | 124 | self.assertEqual( | 141 | self.assertEqual( |
113 | 125 | 142 | ||
114 | === modified file 'src/cloudspacesclient/tests/unit/test_schemas.py' | |||
115 | --- src/cloudspacesclient/tests/unit/test_schemas.py 2013-11-22 09:22:15 +0000 | |||
116 | +++ src/cloudspacesclient/tests/unit/test_schemas.py 2013-11-26 16:41:32 +0000 | |||
117 | @@ -72,9 +72,11 @@ | |||
118 | 72 | _VALID_METADATA = { | 72 | _VALID_METADATA = { |
119 | 73 | 'client_modified': '2013-11-20T05:45:13Z', | 73 | 'client_modified': '2013-11-20T05:45:13Z', |
120 | 74 | 'content_path': '/api/content/538757639', | 74 | 'content_path': '/api/content/538757639', |
121 | 75 | 'contents': [], | ||
122 | 75 | 'is_deleted': False, | 76 | 'is_deleted': False, |
123 | 76 | 'is_folder': True, | 77 | 'is_folder': True, |
124 | 77 | 'is_root': False, | 78 | 'is_root': False, |
125 | 79 | 'filename': 'Documents', | ||
126 | 78 | 'node_id': '538757639', | 80 | 'node_id': '538757639', |
127 | 79 | 'parent_node_id': '0', | 81 | 'parent_node_id': '0', |
128 | 80 | 'path': '/Documents', | 82 | 'path': '/Documents', |
129 | @@ -91,19 +93,23 @@ | |||
130 | 91 | 'valid_object': _VALID_METADATA, | 93 | 'valid_object': _VALID_METADATA, |
131 | 92 | 'required_properties': { | 94 | 'required_properties': { |
132 | 93 | 'content_path': 'string', | 95 | 'content_path': 'string', |
133 | 96 | 'filename': 'string', | ||
134 | 94 | 'is_deleted': 'boolean', | 97 | 'is_deleted': 'boolean', |
135 | 95 | 'is_folder': 'boolean', | 98 | 'is_folder': 'boolean', |
136 | 96 | 'is_root': 'boolean', | 99 | 'is_root': 'boolean', |
137 | 97 | 'node_id': 'string', | 100 | 'node_id': 'string', |
138 | 98 | 'parent_node_id': 'string null', | ||
139 | 99 | 'path': 'string', | 101 | 'path': 'string', |
140 | 100 | 'resource_path': 'string', | 102 | 'resource_path': 'string', |
141 | 103 | 'volume_id': 'string' | ||
142 | 104 | }, | ||
143 | 105 | 'optional_properties': { | ||
144 | 106 | 'contents': 'array', | ||
145 | 107 | 'client_modified': 'string', | ||
146 | 108 | 'parent_node_id': 'string', | ||
147 | 101 | 'server_modified': 'string', | 109 | 'server_modified': 'string', |
148 | 110 | 'version': 'integer', | ||
149 | 102 | 'user_id': 'string', | 111 | 'user_id': 'string', |
150 | 103 | 'version': 'integer', | ||
151 | 104 | 'volume_id': 'string' | ||
152 | 105 | }, | 112 | }, |
153 | 106 | 'optional_properties': {}, | ||
154 | 107 | 'allows_x_attributes': True, | 113 | 'allows_x_attributes': True, |
155 | 108 | 'allows_extra_attributes': False | 114 | 'allows_extra_attributes': False |
156 | 109 | } | 115 | } |
157 | @@ -160,6 +166,27 @@ | |||
158 | 160 | self.assert_raises_validation_error(self.json, self.schema) | 166 | self.assert_raises_validation_error(self.json, self.schema) |
159 | 161 | 167 | ||
160 | 162 | 168 | ||
161 | 169 | class ObjectOptionalPropertiesTestCase( | ||
162 | 170 | testscenarios.TestWithScenarios, ResponseSchemaTestCase): | ||
163 | 171 | |||
164 | 172 | scenarios = [ | ||
165 | 173 | ('{0} without {1}'.format(object_.get('name'), property_), dict( | ||
166 | 174 | schema=object_.get('schema'), | ||
167 | 175 | valid_object=object_.get('valid_object'), | ||
168 | 176 | property_=property_)) | ||
169 | 177 | for object_ in _OBJECTS | ||
170 | 178 | for property_ in object_.get('optional_properties') | ||
171 | 179 | ] | ||
172 | 180 | |||
173 | 181 | def setUp(self): | ||
174 | 182 | super(ObjectOptionalPropertiesTestCase, self).setUp() | ||
175 | 183 | self.json = copy.deepcopy(self.valid_object) | ||
176 | 184 | |||
177 | 185 | def test_valid_json_without_optional_property(self): | ||
178 | 186 | del self.json[self.property_] | ||
179 | 187 | validictory.validate(self.json, self.schema) | ||
180 | 188 | |||
181 | 189 | |||
182 | 163 | _TYPES = types = { | 190 | _TYPES = types = { |
183 | 164 | 'string': 'test string', | 191 | 'string': 'test string', |
184 | 165 | 'integer': 1, | 192 | 'integer': 1, |
185 | @@ -195,7 +222,10 @@ | |||
186 | 195 | property_=property_, | 222 | property_=property_, |
187 | 196 | value=wrong_value)) | 223 | value=wrong_value)) |
188 | 197 | for object_ in _OBJECTS | 224 | for object_ in _OBJECTS |
190 | 198 | for property_, types in object_.get('required_properties').items() | 225 | for property_, types in ( |
191 | 226 | object_.get('required_properties').items() + | ||
192 | 227 | object_.get('optional_properties').items() | ||
193 | 228 | ) | ||
194 | 199 | for wrong_type, wrong_value in _get_wrong_types(types).items() | 229 | for wrong_type, wrong_value in _get_wrong_types(types).items() |
195 | 200 | ] | 230 | ] |
196 | 201 | 231 | ||
197 | 202 | 232 | ||
198 | === modified file 'src/cloudspacesclient/ubuntuone.py' | |||
199 | --- src/cloudspacesclient/ubuntuone.py 2013-11-24 05:49:47 +0000 | |||
200 | +++ src/cloudspacesclient/ubuntuone.py 2013-11-26 16:41:32 +0000 | |||
201 | @@ -130,9 +130,7 @@ | |||
202 | 130 | 130 | ||
203 | 131 | def get_root_path(self): | 131 | def get_root_path(self): |
204 | 132 | """Return the path to the root folder.""" | 132 | """Return the path to the root folder.""" |
208 | 133 | # TODO ask why on the root path it has a trailing / and on the volume | 133 | return '~' |
206 | 134 | # names it doesn't. --elopio - 2013-11-23 | ||
207 | 135 | return '~/Ubuntu One/' | ||
209 | 136 | 134 | ||
210 | 137 | def get_new_user_volume_names(self): | 135 | def get_new_user_volume_names(self): |
211 | 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.""" |
Small change needed, now that we have filename field too: http:// paste.ubuntu. com/6478476/