diff --git a/adminapi/dataset.py b/adminapi/dataset.py index 7962a21bb..53c8db1ab 100644 --- a/adminapi/dataset.py +++ b/adminapi/dataset.py @@ -271,7 +271,7 @@ def _fetch_new_object(self, servertype): ) return _format_obj(response['result']) - def commit(self): + def commit(self) -> int: commit = self._build_commit_object() result = send_request(COMMIT_ENDPOINT, post_params=commit) @@ -282,6 +282,8 @@ def commit(self): for obj in self: obj._confirm_changes() + return result['commit_id'] + def _fetch_results(self): request_data = {'filters': self._filters} if self._restrict is not None: @@ -480,7 +482,7 @@ def update(self, other, **kwargs): self[key] = value # XXX: Deprecated - def commit(self): + def commit(self) -> int: commit = self._build_commit_object() result = send_request(COMMIT_ENDPOINT, post_params=commit) @@ -489,6 +491,8 @@ def commit(self): self._confirm_changes() + return result['commit_id'] + class MultiAttr(set): """This class must redefine all mutable methods of the set class diff --git a/serveradmin/access_control/tests/test_acl.py b/serveradmin/access_control/tests/test_acl.py index 3454557dd..a8b9ccee5 100644 --- a/serveradmin/access_control/tests/test_acl.py +++ b/serveradmin/access_control/tests/test_acl.py @@ -27,7 +27,7 @@ def test_can_commit_related_via_attribute(self): vm = Query({"hostname": "vm-1"}, ["hostname", "hv"]) vm.update(hv="hv-2") - self.assertIsNone(vm.commit(app=Application.objects.get(name="test"))) + self.assertIsInstance(vm.commit(app=Application.objects.get(name="test")), int) # See https://github.com/innogames/serveradmin/pull/351 def test_cannot_commit_related_via_attribute_target(self): diff --git a/serveradmin/api/views.py b/serveradmin/api/views.py index d07dcf928..48f01eb5e 100644 --- a/serveradmin/api/views.py +++ b/serveradmin/api/views.py @@ -99,7 +99,7 @@ def dataset_commit(request, app, data): kwargs[key] = value try: - commit_query(app=app, **kwargs) + _, commit_id = commit_query(app=app, **kwargs) except ValidationError as error: return { 'status': 'error', @@ -109,6 +109,7 @@ def dataset_commit(request, app, data): return { 'status': 'success', + 'commit_id': commit_id, } diff --git a/serveradmin/dataset.py b/serveradmin/dataset.py index 6fb35fad0..5add117b7 100644 --- a/serveradmin/dataset.py +++ b/serveradmin/dataset.py @@ -16,17 +16,20 @@ class Query(BaseQuery): def _fetch_new_object(self, servertype): return DatasetObject(get_default_attribute_values(servertype)) - def commit(self, app=None, user=None): + def commit(self, app=None, user=None) -> int: commit_obj = self._build_commit_object() - commit_query(app=app, user=user, **commit_obj) + _, commit_id = commit_query(app=app, user=user, **commit_obj) self._confirm_changes() + return commit_id def _fetch_results(self): return execute_query(self._filters, self._restrict, self._order_by) class DatasetObject(ApiDatasetObject): - def commit(self, app=None, user=None): + # XXX: Deprecated use Query().commit(). + def commit(self, app=None, user=None) -> int: commit_obj = self._build_commit_object() - commit_query(app=app, user=user, **commit_obj) + _, commit_id = commit_query(app=app, user=user, **commit_obj) self._confirm_changes() + return commit_id diff --git a/serveradmin/serverdb/query_committer.py b/serveradmin/serverdb/query_committer.py index 6b73c5e20..80ece7880 100644 --- a/serveradmin/serverdb/query_committer.py +++ b/serveradmin/serverdb/query_committer.py @@ -129,7 +129,7 @@ def commit_query(created=[], changed=[], deleted=[], app=None, user=None): created_objects, changed_objects, deleted_objects ) - _log_changes(user, app, changed, created_objects, deleted_objects) + commit_id = _log_changes(user, app, changed, created_objects, deleted_objects) post_commit.send_robust( commit_query, created=created, changed=changed, deleted=deleted @@ -139,7 +139,7 @@ def commit_query(created=[], changed=[], deleted=[], app=None, user=None): list(created_objects.values()), list(changed_objects.values()), list(deleted_objects.values()), - ) + ), commit_id def _validate(attribute_lookup, changed, changed_objects): @@ -467,7 +467,7 @@ def _acl_violations(touched_objects, pending_changes, acl): return violations or None -def _log_changes(user, app, changed, created_objects, deleted_objects): +def _log_changes(user, app, changed, created_objects, deleted_objects) -> int: changes = list() commit = ChangeCommit(user=user, app=app) @@ -504,6 +504,8 @@ def _log_changes(user, app, changed, created_objects, deleted_objects): commit.save() Change.objects.bulk_create(changes) + return commit.id + def _fetch_servers(object_ids): servers = { diff --git a/serveradmin/serverdb/tests/test_ip_addr_type.py b/serveradmin/serverdb/tests/test_ip_addr_type.py index 856cda922..e014be07c 100644 --- a/serveradmin/serverdb/tests/test_ip_addr_type.py +++ b/serveradmin/serverdb/tests/test_ip_addr_type.py @@ -49,7 +49,7 @@ class TestIpAddrTypeNullForInternIp(TestIpAddrType): def test_server_without_intern_ip(self): server = self._get_server("null") - self.assertIsNone(server.commit(user=User.objects.first())) + self.assertIsInstance(server.commit(user=User.objects.first()), int) def test_server_with_intern_ip(self): server = self._get_server("null") @@ -87,7 +87,7 @@ def test_server_without_value(self): def test_server_with_value(self): server = self._get_server("host") server["intern_ip"] = "10.0.0.1/32" - self.assertIsNone(server.commit(user=User.objects.first())) + self.assertIsInstance(server.commit(user=User.objects.first()), int) def test_server_with_invalid_value(self): server = self._get_server("host") @@ -132,7 +132,7 @@ def test_change_server_hostname(self): to_rename = Query({"hostname": server["hostname"]}, ["hostname"]) to_rename.update(hostname=self.faker.hostname()) - self.assertIsNone(to_rename.commit(user=User.objects.first())) + self.assertIsInstance(to_rename.commit(user=User.objects.first()), int) class TestIpAddrTypeHostForInetAttributes(TestIpAddrType): @@ -141,13 +141,13 @@ class TestIpAddrTypeHostForInetAttributes(TestIpAddrType): def test_server_without_value(self): server = self._get_server("host") server["intern_ip"] = "10.0.0.1/32" - self.assertIsNone(server.commit(user=User.objects.first())) + self.assertIsInstance(server.commit(user=User.objects.first()), int) def test_server_with_value(self): server = self._get_server("host") server["intern_ip"] = "10.0.0.1/32" server["ip_config_ipv4"] = "10.0.0.2/32" - self.assertIsNone(server.commit(user=User.objects.first())) + self.assertIsInstance(server.commit(user=User.objects.first()), int) def test_server_with_invalid_value(self): server = self._get_server("host") @@ -187,7 +187,7 @@ def test_server_overlaps_with_network(self): server = self._get_server("host") server["intern_ip"] = "10.0.0.1/32" server["ip_config_ipv4"] = "10.0.1.5/32" - self.assertIsNone(server.commit(user=User.objects.first())) + self.assertIsInstance(server.commit(user=User.objects.first()), int) def test_server_with_duplicate_intern_ip(self): first = self._get_server("host") @@ -211,7 +211,7 @@ def test_server_with_duplicate_inet_different_attrs(self): other_attribute = self._get_server("host") other_attribute["intern_ip"] = "10.0.0.3/32" other_attribute["ip_config_new"] = "10.0.0.2/32" - self.assertIsNone(other_attribute.commit(user=User.objects.first())) + self.assertIsInstance(other_attribute.commit(user=User.objects.first()), int) def test_server_with_duplicate_inet_ip(self): server = self._get_server("host") @@ -223,7 +223,7 @@ def test_server_with_duplicate_inet_ip(self): duplicate = self._get_server("loadbalancer") duplicate["intern_ip"] = "10.0.0.2/32" duplicate["ip_config_ipv4"] = "10.0.0.1/32" - self.assertIsNone(duplicate.commit(user=User.objects.first())) + self.assertIsInstance(duplicate.commit(user=User.objects.first()), int) def test_change_server_hostname(self): server = self._get_server("host") @@ -233,7 +233,7 @@ def test_change_server_hostname(self): to_rename = Query({"hostname": server["hostname"]}, ["hostname"]) to_rename.update(hostname=self.faker.hostname()) - self.assertIsNone(to_rename.commit(user=User.objects.first())) + self.assertIsInstance(to_rename.commit(user=User.objects.first()), int) class TestIpAddrTypeLoadbalancerForInternIp(TestIpAddrType): @@ -247,7 +247,7 @@ def test_server_without_value(self): def test_server_with_value(self): server = self._get_server("loadbalancer") server["intern_ip"] = "10.0.0.1/32" - self.assertIsNone(server.commit(user=User.objects.first())) + self.assertIsInstance(server.commit(user=User.objects.first()), int) def test_server_with_ip_network(self): server = self._get_server("loadbalancer") @@ -262,7 +262,7 @@ def test_server_with_duplicate_intern_ip(self): second = self._get_server("loadbalancer") second["intern_ip"] = "10.0.0.1/32" - self.assertIsNone(second.commit(user=User.objects.first())) + self.assertIsInstance(second.commit(user=User.objects.first()), int) def test_change_server_hostname(self): server = self._get_server("loadbalancer") @@ -271,7 +271,7 @@ def test_change_server_hostname(self): to_rename = Query({"hostname": server["hostname"]}, ["hostname"]) to_rename.update(hostname=self.faker.hostname()) - self.assertIsNone(to_rename.commit(user=User.objects.first())) + self.assertIsInstance(to_rename.commit(user=User.objects.first()), int) class TestIpAddrTypeLoadbalancerForInetAttributes(TestIpAddrType): @@ -280,13 +280,13 @@ class TestIpAddrTypeLoadbalancerForInetAttributes(TestIpAddrType): def test_server_without_value(self): server = self._get_server("loadbalancer") server["intern_ip"] = "10.0.0.1/32" - self.assertIsNone(server.commit(user=User.objects.first())) + self.assertIsInstance(server.commit(user=User.objects.first()), int) def test_server_with_value(self): server = self._get_server("loadbalancer") server["intern_ip"] = "10.0.0.1/32" server["ip_config_ipv4"] = "10.0.0.2/32" - self.assertIsNone(server.commit(user=User.objects.first())) + self.assertIsInstance(server.commit(user=User.objects.first()), int) def test_server_with_ip_network(self): server = self._get_server("loadbalancer") @@ -304,7 +304,7 @@ def test_server_with_duplicate_inet_attribute(self): second = self._get_server("loadbalancer") second["intern_ip"] = "10.0.0.1/32" second["ip_config_ipv4"] = "10.0.0.2/32" - self.assertIsNone(second.commit(user=User.objects.first())) + self.assertIsInstance(second.commit(user=User.objects.first()), int) def test_server_with_duplicate_inet_ip(self): first = self._get_server("loadbalancer") @@ -316,7 +316,7 @@ def test_server_with_duplicate_inet_ip(self): duplicate = self._get_server("host") duplicate["intern_ip"] = "10.0.0.2/32" duplicate["ip_config_ipv4"] = "10.0.0.1/32" - self.assertIsNone(duplicate.commit(user=User.objects.first())) + self.assertIsInstance(duplicate.commit(user=User.objects.first()), int) def test_server_with_duplicate_inet_different_attrs(self): server = self._get_server("loadbalancer") @@ -327,7 +327,7 @@ def test_server_with_duplicate_inet_different_attrs(self): duplicate = self._get_server("loadbalancer") duplicate["intern_ip"] = "10.0.0.3/32" duplicate["ip_config_new"] = "10.0.0.2/32" - self.assertIsNone(duplicate.commit(user=User.objects.first())) + self.assertIsInstance(duplicate.commit(user=User.objects.first()), int) def test_change_server_hostname(self): server = self._get_server("loadbalancer") @@ -337,7 +337,7 @@ def test_change_server_hostname(self): to_rename = Query({"hostname": server["hostname"]}, ["hostname"]) to_rename.update(hostname=self.faker.hostname()) - self.assertIsNone(to_rename.commit(user=User.objects.first())) + self.assertIsInstance(to_rename.commit(user=User.objects.first()), int) class TestIpAddrTypeNetworkForInternIp(TestIpAddrType): @@ -351,7 +351,7 @@ def test_server_without_value(self): def test_server_with_value(self): server = self._get_server("route_network") server["intern_ip"] = "10.0.0.0/16" - self.assertIsNone(server.commit(user=User.objects.first())) + self.assertIsInstance(server.commit(user=User.objects.first()), int) def test_server_with_invalid_value(self): server = self._get_server("host") @@ -369,7 +369,7 @@ def test_server_with_invalid_network(self): def test_server_with_ip_address(self): server = self._get_server("route_network") server["intern_ip"] = "10.0.0.1/32" # Just a very small network - self.assertIsNone(server.commit(user=User.objects.first())) + self.assertIsInstance(server.commit(user=User.objects.first()), int) def test_server_network_overlaps(self): first = self._get_server("route_network") @@ -389,7 +389,7 @@ def test_change_server_network_overlaps(self): host = Query({"hostname": first["hostname"]}, ["intern_ip"]) host.update(intern_ip=IPv4Network("10.0.0.0/28")) - self.assertIsNone(host.commit(user=User.objects.first())) + self.assertIsInstance(host.commit(user=User.objects.first()), int) def test_server_network_overlaps_inet(self): first = self._get_server("route_network") @@ -410,7 +410,7 @@ def test_server_network_overlaps_other_servertype(self): # A network can overlap with networks of other servertypes overlaps = self._get_server("provider_network") overlaps["intern_ip"] = "10.0.0.0/28" - self.assertIsNone(overlaps.commit(user=User.objects.first())) + self.assertIsInstance(overlaps.commit(user=User.objects.first()), int) def test_change_server_hostname(self): server = self._get_server("route_network") @@ -419,7 +419,7 @@ def test_change_server_hostname(self): to_rename = Query({"hostname": server["hostname"]}, ["hostname"]) to_rename.update(hostname=self.faker.hostname()) - self.assertIsNone(to_rename.commit(user=User.objects.first())) + self.assertIsInstance(to_rename.commit(user=User.objects.first()), int) class TestIpAddrTypeNetworkForInetAttributes(TestIpAddrType): @@ -428,13 +428,13 @@ class TestIpAddrTypeNetworkForInetAttributes(TestIpAddrType): def test_server_without_value(self): server = self._get_server("route_network") server["intern_ip"] = "10.0.0.0/16" - self.assertIsNone(server.commit(user=User.objects.first())) + self.assertIsInstance(server.commit(user=User.objects.first()), int) def test_server_with_value(self): server = self._get_server("route_network") server["intern_ip"] = "10.0.0.0/30" server["ip_config_ipv4"] = "10.0.1.0/30" - self.assertIsNone(server.commit(user=User.objects.first())) + self.assertIsInstance(server.commit(user=User.objects.first()), int) def test_server_with_invalid_value(self): server = self._get_server("host") @@ -455,7 +455,7 @@ def test_server_with_ip_address(self): server = self._get_server("route_network") server["intern_ip"] = "10.0.0.1/32" # Just a very small network server["ip_config_ipv4"] = "10.0.1.0/32" # Just a very small network - self.assertIsNone(server.commit(user=User.objects.first())) + self.assertIsInstance(server.commit(user=User.objects.first()), int) def test_server_network_overlaps(self): first = self._get_server("route_network") @@ -477,7 +477,7 @@ def test_change_server_network_overlaps(self): host = Query({"hostname": first["hostname"]}, ["ip_config_ipv4"]) host.update(ip_config_ipv4=IPv4Network("10.0.1.0/28")) - self.assertIsNone(host.commit(user=User.objects.first())) + self.assertIsInstance(host.commit(user=User.objects.first()), int) def test_server_network_overlaps_intern_ip(self): first = self._get_server("route_network") @@ -512,7 +512,7 @@ def test_server_network_overlaps_other_servertype(self): overlaps = self._get_server("provider_network") overlaps["intern_ip"] = "10.0.0.0/28" overlaps["ip_config_ipv4"] = "10.0.1.0/30" - self.assertIsNone(overlaps.commit(user=User.objects.first())) + self.assertIsInstance(overlaps.commit(user=User.objects.first()), int) def test_change_server_hostname(self): server = self._get_server("route_network") @@ -522,7 +522,7 @@ def test_change_server_hostname(self): to_rename = Query({"hostname": server["hostname"]}, ["hostname"]) to_rename.update(hostname=self.faker.hostname()) - self.assertIsNone(to_rename.commit(user=User.objects.first())) + self.assertIsInstance(to_rename.commit(user=User.objects.first()), int) class TestIpAddrTypeHostForSupernetAttr(TestIpAddrType): diff --git a/serveradmin/serverdb/views.py b/serveradmin/serverdb/views.py index 3825ec9d0..1e46d462e 100644 --- a/serveradmin/serverdb/views.py +++ b/serveradmin/serverdb/views.py @@ -183,8 +183,8 @@ def recreate(request, change_id): server_object.pop(attribute_id) try: - commit = commit_query([server_object], user=request.user) - object_id = str(commit.created[0]['object_id']) + changes_obj, _ = commit_query([server_object], user=request.user) + object_id = str(changes_obj.created[0]['object_id']) except (CommitError, ValidationError) as error: messages.error(request, str(error)) return redirect(reverse('serverdb_changes')) diff --git a/serveradmin/servershell/views.py b/serveradmin/servershell/views.py index 55f3bf665..e1706f70e 100644 --- a/serveradmin/servershell/views.py +++ b/serveradmin/servershell/views.py @@ -315,8 +315,7 @@ def _edit(request: HttpRequest, server, edit_mode=False, template='edit'): # NO messages.info(request, str('Nothing has changed.')) else: try: - commit_obj = commit_query(created, changed, - user=request.user) + commit_obj, _ = commit_query(created, changed, user=request.user) except (PermissionDenied, ValidationError) as err: messages.error(request, str(err)) else: