Merge ~cgrabowski/maas:reverse_unsubscribe_logic into maas:master
- Git
- lp:~cgrabowski/maas
- reverse_unsubscribe_logic
- Merge into master
Status: | Merged |
---|---|
Approved by: | Christian Grabowski |
Approved revision: | 7cd89f4e08129cb5519836200e7d325a6f6cc7cf |
Merge reported by: | MAAS Lander |
Merged at revision: | not available |
Proposed branch: | ~cgrabowski/maas:reverse_unsubscribe_logic |
Merge into: | maas:master |
Diff against target: |
274 lines (+95/-92) 5 files modified
src/maasserver/websockets/base.py (+11/-21) src/maasserver/websockets/handlers/machine.py (+12/-0) src/maasserver/websockets/handlers/tests/test_machine.py (+71/-0) src/maasserver/websockets/tests/test_base.py (+0/-50) src/maasserver/websockets/tests/test_protocol.py (+1/-21) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Alexsander de Souza | Approve | ||
MAAS Lander | Approve | ||
Review via email: mp+426761@code.launchpad.net |
Commit message
add back a set of unsubscribed pks to know when to ignore an update
update units to reflect allow-list behavior
reverse unsubscribe endpoint logic to receive an allow-list
Description of the change
- c36b1b1... by Christian Grabowski
-
remove no longer relevant test
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b reverse_
STATUS: FAILED
LOG: http://
COMMIT: 27d6b8cf2b0a378
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b reverse_
STATUS: SUCCESS
COMMIT: c36b1b185951f71
- 02a30d6... by Christian Grabowski
-
override machine's listen function to only listen for machines in loaded_pks
- 7cd89f4... by Christian Grabowski
-
safely access active_pk
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b reverse_
STATUS: SUCCESS
COMMIT: a704ef6445d66fd
Alexsander de Souza (alexsander-souza) : | # |
Christian Grabowski (cgrabowski) : | # |
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b reverse_
STATUS: SUCCESS
COMMIT: 7cd89f4e08129cb
Alexsander de Souza (alexsander-souza) wrote : | # |
+1
Preview Diff
1 | diff --git a/src/maasserver/websockets/base.py b/src/maasserver/websockets/base.py |
2 | index 3063777..8787174 100644 |
3 | --- a/src/maasserver/websockets/base.py |
4 | +++ b/src/maasserver/websockets/base.py |
5 | @@ -103,7 +103,6 @@ class HandlerOptions: |
6 | pk = "id" |
7 | bulk_pk = "ids" |
8 | pk_type = int |
9 | - unsubscribed_pks = set() |
10 | fields = None |
11 | exclude = None |
12 | list_fields = None |
13 | @@ -340,7 +339,6 @@ class Handler(metaclass=HandlerMetaclass): |
14 | permission = self._meta.view_permission |
15 | if not self.user.has_perm(permission, obj): |
16 | raise HandlerPermissionError() |
17 | - self._meta.unsubscribed_pks.discard(pk) |
18 | return obj |
19 | |
20 | def get_queryset(self, for_list=False): |
21 | @@ -465,7 +463,6 @@ class Handler(metaclass=HandlerMetaclass): |
22 | """Cache all loaded object pks.""" |
23 | getpk = attrgetter(self._meta.pk) |
24 | self.cache["loaded_pks"].update(getpk(obj) for obj in objs) |
25 | - [self._meta.unsubscribed_pks.discard(getpk(obj)) for obj in objs] |
26 | |
27 | def _filter(self, qs, action, params): |
28 | """Return a filtered queryset |
29 | @@ -812,11 +809,7 @@ class Handler(metaclass=HandlerMetaclass): |
30 | :param action: Action that caused this event. |
31 | :param pk: Id of the object. |
32 | """ |
33 | - return ( |
34 | - self.get_object({self._meta.pk: pk}) |
35 | - if pk not in self._meta.unsubscribed_pks |
36 | - else None |
37 | - ) |
38 | + return self.get_object({self._meta.pk: pk}) |
39 | |
40 | def refetch(self, obj): |
41 | """Refetch an object using the handler queryset. |
42 | @@ -826,22 +819,19 @@ class Handler(metaclass=HandlerMetaclass): |
43 | """ |
44 | return self.get_object({self._meta.pk: getattr(obj, self._meta.pk)}) |
45 | |
46 | - def _unsubscribe(self, pk): |
47 | - if pk == self.cache.get("active_pk"): |
48 | - del self.cache["active_pk"] |
49 | - |
50 | - if pk in self.cache.get("loaded_pks", []): |
51 | - self.cache["loaded_pks"].remove(pk) |
52 | - |
53 | - self._meta.unsubscribed_pks.add(pk) |
54 | - |
55 | - return pk |
56 | - |
57 | def unsubscribe(self, params): |
58 | if self._meta.pk in params: |
59 | - return self._unsubscribe(params[self._meta.pk]) |
60 | + pk = params[self._meta.pk] |
61 | + if pk == self.cache.get("active_pk"): |
62 | + del self.cache["active_pk"] |
63 | + self.cache["loaded_pks"] = self.cache["loaded_pks"] - set(pk) |
64 | + return [pk] |
65 | elif self._meta.bulk_pk in params: |
66 | - return [self._unsubscribe(pk) for pk in params[self._meta.bulk_pk]] |
67 | + pks = set(params[self._meta.bulk_pk]) |
68 | + if self.cache.get("active_pk") in pks: |
69 | + del self.cache["active_pk"] |
70 | + self.cache["loaded_pks"] = self.cache["loaded_pks"] - pks |
71 | + return pks |
72 | else: |
73 | raise HandlerValidationError( |
74 | f"'{self._meta.pk}' or '{self._meta.bulk_pk}' must be provided in params for unsubscribe" |
75 | diff --git a/src/maasserver/websockets/handlers/machine.py b/src/maasserver/websockets/handlers/machine.py |
76 | index 93cd2eb..955c350 100644 |
77 | --- a/src/maasserver/websockets/handlers/machine.py |
78 | +++ b/src/maasserver/websockets/handlers/machine.py |
79 | @@ -521,6 +521,18 @@ class MachineHandler(NodeHandler): |
80 | % (op, " or ".join(status_names)) |
81 | ) |
82 | |
83 | + def listen(self, channel, action, pk): |
84 | + """Called when the handler listens for events on channels with |
85 | + `Meta.listen_channels`. |
86 | + |
87 | + :param channel: Channel event occured on. |
88 | + :param action: Action that caused this event. |
89 | + :param pk: Id of the object. |
90 | + """ |
91 | + # if loaded / not unsubscrived, allow listen events |
92 | + if pk in self.cache["loaded_pks"] or pk == self.cache.get("active_pk"): |
93 | + return self.get_object({self._meta.pk: pk}) |
94 | + |
95 | def update_filesystem(self, params): |
96 | node = self._get_node_or_permission_error( |
97 | params, permission=NodePermission.edit |
98 | diff --git a/src/maasserver/websockets/handlers/tests/test_machine.py b/src/maasserver/websockets/handlers/tests/test_machine.py |
99 | index 352d4ea..c4d3c6f 100644 |
100 | --- a/src/maasserver/websockets/handlers/tests/test_machine.py |
101 | +++ b/src/maasserver/websockets/handlers/tests/test_machine.py |
102 | @@ -6271,3 +6271,74 @@ class TestMachineHandlerFilter(MAASServerTestCase): |
103 | _assert_value_in(machine.bmc.power_type, "not_pod_type") |
104 | _assert_value_in(machine.bmc.id, "pod") |
105 | _assert_value_in(machine.bmc.id, "not_pod") |
106 | + |
107 | + def test_unsubscribe_prevents_further_updates_for_pk(self): |
108 | + admin = factory.make_admin() |
109 | + handler = MachineHandler(admin, {}, None) |
110 | + node = factory.make_Node() |
111 | + handler.list({}) |
112 | + listen_result = handler.on_listen("machine", "update", node.system_id) |
113 | + self.assertIsNotNone(listen_result) |
114 | + handler.unsubscribe({"system_ids": [node.system_id]}) |
115 | + self.assertIsNone( |
116 | + handler.on_listen("machine", "update", node.system_id) |
117 | + ) |
118 | + list_result = handler.list({}) |
119 | + self.assertEqual(len(list_result), 1) |
120 | + |
121 | + def test_unsubscribe_raises_validation_error_with_no_pk(self): |
122 | + admin = factory.make_admin() |
123 | + handler = MachineHandler(admin, {}, None) |
124 | + self.assertRaises(HandlerValidationError, handler.unsubscribe, {}) |
125 | + |
126 | + def test_read_an_unsubscribed_object_subscribes(self): |
127 | + admin = factory.make_admin() |
128 | + handler = MachineHandler(admin, {}, None) |
129 | + node1 = factory.make_Node() |
130 | + node2 = factory.make_Node() |
131 | + handler.list({}) |
132 | + self.assertIsNotNone( |
133 | + handler.on_listen("machine", "update", node1.system_id) |
134 | + ) |
135 | + self.assertIsNotNone( |
136 | + handler.on_listen("machine", "update", node2.system_id) |
137 | + ) |
138 | + handler.unsubscribe({"system_ids": [node2.system_id]}) |
139 | + self.assertIsNotNone( |
140 | + handler.on_listen("machine", "update", node1.system_id) |
141 | + ) |
142 | + self.assertIsNone( |
143 | + handler.on_listen("machine", "update", node2.system_id) |
144 | + ) |
145 | + self.assertIsNotNone(handler.get({"system_id": node2.system_id})) |
146 | + self.assertIsNotNone( |
147 | + handler.on_listen("machine", "update", node2.system_id) |
148 | + ) |
149 | + |
150 | + def test_list_an_unsubscribed_object_subscribes(self): |
151 | + admin = factory.make_admin() |
152 | + handler = MachineHandler(admin, {}, None) |
153 | + node1 = factory.make_Node() |
154 | + node2 = factory.make_Node() |
155 | + handler.list({}) |
156 | + self.assertIsNotNone( |
157 | + handler.on_listen("machine", "update", node1.system_id) |
158 | + ) |
159 | + self.assertIsNotNone( |
160 | + handler.on_listen("machine", "update", node2.system_id) |
161 | + ) |
162 | + handler.unsubscribe({"system_ids": [node1.system_id]}) |
163 | + self.assertIsNotNone( |
164 | + handler.on_listen("machine", "update", node2.system_id) |
165 | + ) |
166 | + self.assertIsNone( |
167 | + handler.on_listen("machine", "update", node1.system_id) |
168 | + ) |
169 | + list_result = handler.list({}) |
170 | + self.assertEqual(len(list_result), 2) |
171 | + self.assertIsNotNone( |
172 | + handler.on_listen("machine", "update", node1.system_id) |
173 | + ) |
174 | + self.assertIsNotNone( |
175 | + handler.on_listen("machine", "update", node2.system_id) |
176 | + ) |
177 | diff --git a/src/maasserver/websockets/tests/test_base.py b/src/maasserver/websockets/tests/test_base.py |
178 | index 77f3fd3..e7632b1 100644 |
179 | --- a/src/maasserver/websockets/tests/test_base.py |
180 | +++ b/src/maasserver/websockets/tests/test_base.py |
181 | @@ -986,56 +986,6 @@ class TestHandler(MAASServerTestCase, FakeNodesHandlerMixin): |
182 | for idx in range(3): |
183 | self.assertEqual(f"host-{2-idx}", result[idx]["hostname"]) |
184 | |
185 | - def test_unsubscribe_prevents_further_updates_for_pk(self): |
186 | - handler = self.make_nodes_handler() |
187 | - node = factory.make_Node() |
188 | - handler._meta.queryset = Node.objects.all() |
189 | - handler._meta.listen_channels = ["node"] |
190 | - handler._meta.bulk_pk = "system_ids" |
191 | - handler._meta.pk = "system_id" |
192 | - listen_result = handler.listen("node", "update", node.system_id) |
193 | - self.assertIsNotNone(listen_result) |
194 | - handler.unsubscribe({"system_ids": [node.system_id]}) |
195 | - self.assertIsNone(handler.listen("node", "update", node.system_id)) |
196 | - list_result = handler.list({}) |
197 | - self.assertEqual(len(list_result), 1) |
198 | - |
199 | - def test_unsubscribe_raises_validation_error_with_no_pk(self): |
200 | - handler = self.make_nodes_handler() |
201 | - handler._meta.queryset = Node.objects.all() |
202 | - handler._meta.listen_channels = ["node"] |
203 | - handler._meta.bulk_pk = "system_ids" |
204 | - handler._meta.pk = "system_id" |
205 | - self.assertRaises(HandlerValidationError, handler.unsubscribe, {}) |
206 | - |
207 | - def test_read_an_unsubscribed_object_subscribes(self): |
208 | - handler = self.make_nodes_handler() |
209 | - node = factory.make_Node() |
210 | - handler._meta.queryset = Node.objects.all() |
211 | - handler._meta.listen_channels = ["node"] |
212 | - handler._meta.bulk_pk = "system_ids" |
213 | - handler._meta.pk = "system_id" |
214 | - self.assertIsNotNone(handler.listen("node", "update", node.system_id)) |
215 | - handler.unsubscribe({"system_ids": [node.system_id]}) |
216 | - self.assertIsNone(handler.listen("node", "update", node.system_id)) |
217 | - self.assertIsNotNone(handler.get({"system_id": node.system_id})) |
218 | - self.assertIsNotNone(handler.listen("node", "update", node.system_id)) |
219 | - |
220 | - def test_list_an_unsubscribed_object_subscribes(self): |
221 | - handler = self.make_nodes_handler() |
222 | - node = factory.make_Node() |
223 | - handler._meta.queryset = Node.objects.all() |
224 | - handler._meta.listen_channels = ["node"] |
225 | - handler._meta.bulk_pk = "system_ids" |
226 | - handler._meta.pk = "system_id" |
227 | - listen_result = handler.listen("node", "update", node.system_id) |
228 | - self.assertIsNotNone(listen_result) |
229 | - handler.unsubscribe({"system_ids": [node.system_id]}) |
230 | - self.assertIsNone(handler.listen("node", "update", node.system_id)) |
231 | - list_result = handler.list({}) |
232 | - self.assertEqual(len(list_result), 1) |
233 | - self.assertIsNotNone(handler.listen("node", "update", node.system_id)) |
234 | - |
235 | |
236 | class TestHandlerGrouping(MAASServerTestCase, FakeNodesHandlerMixin): |
237 | def test_group_simple(self): |
238 | diff --git a/src/maasserver/websockets/tests/test_protocol.py b/src/maasserver/websockets/tests/test_protocol.py |
239 | index b5cbdc7..5181adb 100644 |
240 | --- a/src/maasserver/websockets/tests/test_protocol.py |
241 | +++ b/src/maasserver/websockets/tests/test_protocol.py |
242 | @@ -20,10 +20,7 @@ from apiclient.utils import ascii_url |
243 | from maasserver.eventloop import services |
244 | from maasserver.testing.factory import factory as maas_factory |
245 | from maasserver.testing.listener import FakePostgresListenerService |
246 | -from maasserver.testing.testcase import ( |
247 | - MAASServerTestCase, |
248 | - MAASTransactionServerTestCase, |
249 | -) |
250 | +from maasserver.testing.testcase import MAASTransactionServerTestCase |
251 | from maasserver.utils.orm import transactional |
252 | from maasserver.utils.threads import deferToDatabase |
253 | from maasserver.websockets import protocol as protocol_module |
254 | @@ -1003,20 +1000,3 @@ class TestWebSocketFactoryTransactional( |
255 | controller.system_id, |
256 | ), |
257 | ) |
258 | - |
259 | - |
260 | -class TestWebSocketFactoryServer(MAASServerTestCase, MakeProtocolFactoryMixin): |
261 | - def test_processNotify_unsubscribed_object(self): |
262 | - factory = self.make_factory() |
263 | - machine = maas_factory.make_Machine() |
264 | - handler = MachineHandler(maas_factory.make_User(), {}, None) |
265 | - factory.handlers["machine"] = handler |
266 | - result = factory.processNotify( |
267 | - handler, "machine", "update", machine.system_id |
268 | - ) |
269 | - self.assertIsNotNone(result) |
270 | - handler.unsubscribe({"system_ids": [machine.system_id]}) |
271 | - result = factory.processNotify( |
272 | - handler, "machine", "update", machine.system_id |
273 | - ) |
274 | - self.assertIsNone(result) |
UNIT TESTS unsubscribe_ logic lp:~cgrabowski/maas/+git/maas into -b master lp:~maas-committers/maas
-b reverse_
STATUS: FAILED maas-ci. internal: 8080/job/ maas-tester/ 34/consoleText 86d8032e9290965 7784f22bd4
LOG: http://
COMMIT: 93893ed47c2552b