Skip to content

Commit

Permalink
Addressed some comments.
Browse files Browse the repository at this point in the history
  • Loading branch information
@Julio-Oliveira-Encora
Julio-Oliveira-Encora committed Mar 1, 2024
1 parent 1168d59 commit 097d313
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 65 deletions.
1 change: 0 additions & 1 deletion diode-netbox-plugin/netbox_diode_plugin/api/urls.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
from .views import ApplyChangeSetView, ObjectStateView

router = NetBoxRouter()
# router.register("apply-change-set", ApplyChangeSetView, basename="apply-change-set")

urlpatterns = [
path("object-state/", ObjectStateView.as_view()),
Expand Down
54 changes: 25 additions & 29 deletions diode-netbox-plugin/netbox_diode_plugin/api/views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ class ApplyChangeSetView(views.APIView):
permission_classes = [IsAuthenticated, IsDiodePost]

@staticmethod
def _get_object_type_model(object_type):
def _get_object_type_model(object_type: str):
"""Get the object type model from object_type."""
app_label, model_name = object_type.split(".")
object_content_type = ContentType.objects.get_by_natural_key(
Expand All @@ -95,41 +95,34 @@ def _get_object_type_model(object_type):
object_type_model = object_content_type.model_class()
return object_type_model

@staticmethod
@transaction.atomic()
def _perform_save(serializer):
"""Save an object and rollback if any problem."""
serializer.save()

@transaction.atomic(savepoint=True)
def _validate_serializer(
self, change_type, object_id, object_type_model, object_data
def _get_serializer(
self,
change_type: str,
object_id: int,
object_type: str,
object_data: dict,
):
"""Validate the serializer."""
"""Get the serializer for the object type."""
object_type_model = self._get_object_type_model(object_type)
if change_type == "create":
serializer = get_serializer_for_model(object_type_model)(
data=object_data, context={"request": self.request}
)
elif change_type == "update":
if not object_id:
raise ValidationError("object_id parameter is required")

try:
instance = object_type_model.objects.get(id=object_id)
except object_type_model.DoesNotExist:
return "failed", f"Object with id {object_id} does not exist"
raise ValidationError(f"Object with id {object_id} does not exist")

serializer = get_serializer_for_model(object_type_model)(
instance, data=object_data, context={"request": self.request}
)
else:
return "failed", "Invalid change_type"

if not serializer.is_valid():
return "failed", serializer.errors

self.check_object_permissions(self.request, serializer)

return "success", serializer
raise ValidationError("Invalid change_type")
return serializer

def post(self, request, *args, **kwargs):
"""
Expand All @@ -138,6 +131,9 @@ def post(self, request, *args, **kwargs):
The request body should contain a list of changes to be applied.
"""
change_set_id = self.request.data.get("change_set_id", None)
if not change_set_id:
raise ValidationError("change_set_id parameter is required")

change_set = self.request.data.get("change_set", None)
if not change_set:
raise ValidationError("change_set parameter is required")
Expand All @@ -147,38 +143,38 @@ def post(self, request, *args, **kwargs):
for change in change_set:

change_id = change.get("change_id", None)

change_type = change.get("change_type", None)

object_type = change.get("object_type", None)
if not object_type:
raise ValidationError("object_type parameter is required")

object_type_model = self._get_object_type_model(object_type)

object_data = change.get("data", None)
if not object_data:
raise ValidationError("data parameter is required")

object_id = change.get("object_id", None)

result, serializer_result = self._validate_serializer(
change_type, object_id, object_type_model, object_data
serializer = self._get_serializer(
change_type, object_id, object_type, object_data
)

if result == "failed":
if serializer.is_valid():
serializer_list.append(serializer)
else:
return Response(
{
"change_set_id": change_set_id,
"change_id": change_id,
"result": "failed",
"error": serializer_result,
"error": serializer.errors,
},
status=status.HTTP_400_BAD_REQUEST,
)

serializer_list.append(serializer_result)

[self._perform_save(serializer) for serializer in serializer_list]
with transaction.atomic():
[serializer.save() for serializer in serializer_list]

data = {"change_set_id": change_set_id, "result": "success"}
return Response(data, status=status.HTTP_200_OK)
82 changes: 47 additions & 35 deletions diode-netbox-plugin/netbox_diode_plugin/tests/test_apply_change_set.py
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#!/usr/bin/env python
# Copyright 2024 NetBox Labs Inc
"""Diode Netbox Plugin - Tests for ApplyChangeSetView."""
import uuid

from dcim.models import (
Device,
Expand Down Expand Up @@ -130,10 +131,10 @@ class ApplyChangeSetTestCase(BaseApplyChangeSet):
def test_change_type_create_return_200(self):
"""Test create change_type with successful."""
payload = {
"change_set_id": "<UUID-0>",
"change_set_id": str(uuid.uuid4()),
"change_set": [
{
"change_id": "<UUID-0>",
"change_id": str(uuid.uuid4()),
"change_type": "create",
"object_version": None,
"object_type": "dcim.site",
Expand Down Expand Up @@ -161,10 +162,10 @@ def test_change_type_create_return_200(self):
def test_change_type_update_return_200(self):
"""Test update change_type with successful."""
payload = {
"change_set_id": "<UUID-0>",
"change_set_id": str(uuid.uuid4()),
"change_set": [
{
"change_id": "<UUID-0>",
"change_id": str(uuid.uuid4()),
"change_type": "update",
"object_version": None,
"object_type": "dcim.site",
Expand Down Expand Up @@ -196,10 +197,10 @@ def test_change_type_update_return_200(self):
def test_change_type_create_with_error_return_400(self):
"""Test create change_type with wrong payload."""
payload = {
"change_set_id": "<UUID-0>",
"change_set_id": str(uuid.uuid4()),
"change_set": [
{
"change_id": "<UUID-0>",
"change_id": str(uuid.uuid4()),
"change_type": "create",
"object_version": None,
"object_type": "dcim.site",
Expand All @@ -226,16 +227,19 @@ def test_change_type_create_with_error_return_400(self):

self.assertEqual(response.status_code, 400)
self.assertEqual(response.json().get("result"), "failed")
self.assertEqual(response.json().get("change_id"), "<UUID-0>")
self.assertIn(
'Expected a list of items but got type "int".',
response.json().get("error").get("asns"),
)
self.assertFalse(site_created.exists())

def test_change_type_update_with_error_return_400(self):
"""Test update change_type with wrong payload."""
payload = {
"change_set_id": "<UUID-0>",
"change_set_id": str(uuid.uuid4()),
"change_set": [
{
"change_id": "<UUID-0>",
"change_id": str(uuid.uuid4()),
"change_type": "update",
"object_version": None,
"object_type": "dcim.site",
Expand All @@ -262,24 +266,27 @@ def test_change_type_update_with_error_return_400(self):

self.assertEqual(response.status_code, 400)
self.assertEqual(response.json().get("result"), "failed")
self.assertEqual(response.json().get("change_id"), "<UUID-0>")
self.assertIn(
'Expected a list of items but got type "int".',
response.json().get("error").get("asns"),
)
self.assertEqual(site_updated.name, "Site 2")

def test_change_type_create_with_multiples_objects_return_200(self):
"""Test create change type with two objects."""
payload = {
"change_set_id": "<UUID-0>",
"change_set_id": str(uuid.uuid4()),
"change_set": [
{
"change_id": "<UUID-0>",
"change_id": str(uuid.uuid4()),
"change_type": "create",
"object_version": None,
"object_type": "dcim.site",
"object_id": None,
"data": {
"name": "Site A",
"slug": "site-a",
"facility": "Alpha",
"name": "Site Z",
"slug": "site-z",
"facility": "Omega",
"description": "",
"physical_address": "123 Fake St Lincoln NE 68588",
"shipping_address": "123 Fake St Lincoln NE 68588",
Expand All @@ -288,15 +295,15 @@ def test_change_type_create_with_multiples_objects_return_200(self):
},
},
{
"change_id": "<UUID-1>",
"change_id": str(uuid.uuid4()),
"change_type": "create",
"object_version": None,
"object_type": "dcim.device",
"object_id": None,
"data": {
"device_type": self.device_types[1].pk,
"role": self.roles[1].pk,
"name": "Test Device 4",
"name": "Test Device 500",
"site": self.sites[1].pk,
"rack": self.racks[1].pk,
"cluster": self.clusters[1].pk,
Expand All @@ -315,10 +322,10 @@ def test_change_type_create_with_multiples_objects_return_200(self):
def test_change_type_update_with_multiples_objects_return_200(self):
"""Test update change type with two objects."""
payload = {
"change_set_id": "<UUID-0>",
"change_set_id": str(uuid.uuid4()),
"change_set": [
{
"change_id": "<UUID-0>",
"change_id": str(uuid.uuid4()),
"change_type": "update",
"object_version": None,
"object_type": "dcim.site",
Expand All @@ -335,7 +342,7 @@ def test_change_type_update_with_multiples_objects_return_200(self):
},
},
{
"change_id": "<UUID-1>",
"change_id": str(uuid.uuid4()),
"change_type": "update",
"object_version": None,
"object_type": "dcim.device",
Expand All @@ -357,20 +364,20 @@ def test_change_type_update_with_multiples_objects_return_200(self):
)

site_updated = Site.objects.get(id=20)
devide_updated = Device.objects.get(id=10)
device_updated = Device.objects.get(id=10)

self.assertEqual(response.status_code, 200)
self.assertEqual(response.json().get("result"), "success")
self.assertEqual(site_updated.name, "Site A")
self.assertEqual(devide_updated.name, "Test Device 3")
self.assertEqual(device_updated.name, "Test Device 3")

def test_change_type_create_and_update_with_error_in_one_object_return_400(self):
"""Test create and update change type with one object with error."""
payload = {
"change_set_id": "<UUID-0>",
"change_set_id": str(uuid.uuid4()),
"change_set": [
{
"change_id": "<UUID-0>",
"change_id": str(uuid.uuid4()),
"change_type": "create",
"object_version": None,
"object_type": "dcim.site",
Expand All @@ -387,11 +394,11 @@ def test_change_type_create_and_update_with_error_in_one_object_return_400(self)
},
},
{
"change_id": "<UUID-1>",
"change_id": str(uuid.uuid4()),
"change_type": "update",
"object_version": None,
"object_type": "dcim.device",
"object_id": 1,
"object_id": 10,
"data": {
"device_type": 3,
"role": self.roles[1].pk,
Expand All @@ -413,17 +420,20 @@ def test_change_type_create_and_update_with_error_in_one_object_return_400(self)

self.assertEqual(response.status_code, 400)
self.assertEqual(response.json().get("result"), "failed")
self.assertEqual(response.json().get("change_id"), "<UUID-1>")
self.assertIn(
"Related object not found using the provided numeric ID",
response.json().get("error").get("device_type")[0],
)
self.assertFalse(site_created.exists())
self.assertFalse(device_created.exists())

def test_multiples_change_type_create_with_error_in_one_object_return_400(self):
"""Test create change_type with error in one object."""
payload = {
"change_set_id": "<UUID-0>",
"change_set_id": str(uuid.uuid4()),
"change_set": [
{
"change_id": "<UUID-0>",
"change_id": str(uuid.uuid4()),
"change_type": "create",
"object_version": None,
"object_type": "dcim.site",
Expand All @@ -440,7 +450,7 @@ def test_multiples_change_type_create_with_error_in_one_object_return_400(self):
},
},
{
"change_id": "<UUID-1>",
"change_id": str(uuid.uuid4()),
"change_type": "create",
"object_version": None,
"object_type": "dcim.device",
Expand All @@ -466,17 +476,20 @@ def test_multiples_change_type_create_with_error_in_one_object_return_400(self):

self.assertEqual(response.status_code, 400)
self.assertEqual(response.json().get("result"), "failed")
self.assertEqual(response.json().get("change_id"), "<UUID-1>")
self.assertIn(
"Related object not found using the provided numeric ID",
response.json().get("error").get("device_type")[0],
)
self.assertFalse(site_created.exists())
self.assertFalse(device_created.exists())

def test_change_type_update_with_object_id_not_exist_return_400(self):
"""Test update object with nonexistent object_id."""
payload = {
"change_set_id": "<UUID-0>",
"change_set_id": str(uuid.uuid4()),
"change_set": [
{
"change_id": "<UUID-0>",
"change_id": str(uuid.uuid4()),
"change_type": "update",
"object_version": None,
"object_type": "dcim.site",
Expand All @@ -501,6 +514,5 @@ def test_change_type_update_with_object_id_not_exist_return_400(self):

site_updated = Site.objects.get(id=20)
self.assertEqual(response.status_code, 400)
self.assertEqual(response.json().get("result"), "failed")
self.assertEqual(response.json().get("change_id"), "<UUID-0>")
self.assertEqual(response.json()[0], "Object with id 30 does not exist")
self.assertEqual(site_updated.name, "Site 2")

0 comments on commit 097d313

Please sign in to comment.