sts-sponsors team mailing list archive

[Alberto Donato <mp+434475@xxxxxxxxxxxxxxxxxx>]

Alberto Donato has proposed merging ~ack/maas:drop-tags-manager into maas:master.

Commit message:
drop TagsManager, move the only logic to the API as it's specific



Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~ack/maas/+git/maas/+merge/434475
-- 
Your team MAAS Maintainers is requested to review the proposed merge of ~ack/maas:drop-tags-manager into maas:master.

diff --git a/src/maasserver/api/tags.py b/src/maasserver/api/tags.py
index 0b90743..5c2731c 100644
--- a/src/maasserver/api/tags.py
+++ b/src/maasserver/api/tags.py
@@ -10,6 +10,7 @@ from django.conf import settings
 from django.core.exceptions import PermissionDenied
 from django.db.utils import DatabaseError
 from django.http import HttpResponse
+from django.shortcuts import get_object_or_404
 from piston3.utils import rc
 
 from maasserver.api.nodes import NODES_PREFETCH, NODES_SELECT_RELATED
@@ -58,6 +59,13 @@ def check_rack_controller_access(request, rack_controller):
         )
 
 
+def get_tag_or_404(name, user, to_edit=False):
+    """Fetch a Tag by name or raise an Http404 exception."""
+    if to_edit and not user.is_superuser:
+        raise PermissionDenied()
+    return get_object_or_404(Tag, name=name)
+
+
 class TagHandler(OperationsHandler):
     """
     Tags are properties that can be associated with a Node and serve as
@@ -89,7 +97,7 @@ class TagHandler(OperationsHandler):
         @error-example "not-found"
             No Tag matches the given query.
         """
-        return Tag.objects.get_tag_or_404(name=name, user=request.user)
+        return get_tag_or_404(name=name, user=request.user)
 
     def update(self, request, name):
         """@description-title Update a tag
@@ -119,9 +127,7 @@ class TagHandler(OperationsHandler):
         @error-example "not-found"
             No Tag matches the given query.
         """
-        tag = Tag.objects.get_tag_or_404(
-            name=name, user=request.user, to_edit=True
-        )
+        tag = get_tag_or_404(name=name, user=request.user, to_edit=True)
         name = tag.name
         form = TagForm(request.data, instance=tag)
         if not form.is_valid():
@@ -159,9 +165,7 @@ class TagHandler(OperationsHandler):
         @error-example "not-found"
             No Tag matches the given query.
         """
-        tag = Tag.objects.get_tag_or_404(
-            name=name, user=request.user, to_edit=True
-        )
+        tag = get_tag_or_404(name=name, user=request.user, to_edit=True)
         tag.delete()
         create_audit_event(
             EVENT_TYPES.TAG,
@@ -178,7 +182,7 @@ class TagHandler(OperationsHandler):
         # This is done because this operation actually returns a list of nodes
         # and not a list of tags as this handler is defined to return.
         self.fields = None
-        tag = Tag.objects.get_tag_or_404(name=name, user=request.user)
+        tag = get_tag_or_404(name=name, user=request.user)
         nodes = model.objects.get_nodes(
             request.user, NodePermission.view, from_nodes=tag.node_set.all()
         )
@@ -318,9 +322,7 @@ class TagHandler(OperationsHandler):
         @error-example "not-found"
             No Tag matches the given query.
         """
-        tag = Tag.objects.get_tag_or_404(
-            name=name, user=request.user, to_edit=True
-        )
+        tag = get_tag_or_404(name=name, user=request.user, to_edit=True)
         tag.populate_nodes()
         return {"rebuilding": tag.name}
 
@@ -373,7 +375,7 @@ class TagHandler(OperationsHandler):
         @error-example "not-found"
             No Tag matches the given query.
         """
-        tag = Tag.objects.get_tag_or_404(name=name, user=request.user)
+        tag = get_tag_or_404(name=name, user=request.user)
         rack_controller = None
         if not request.user.is_superuser:
             system_id = request.data.get("rack_controller", None)
diff --git a/src/maasserver/models/tag.py b/src/maasserver/models/tag.py
index e179a48..ca9e445 100644
--- a/src/maasserver/models/tag.py
+++ b/src/maasserver/models/tag.py
@@ -4,10 +4,9 @@
 """Node objects."""
 
 
-from django.core.exceptions import PermissionDenied, ValidationError
+from django.core.exceptions import ValidationError
 from django.core.validators import RegexValidator
-from django.db.models import CharField, Manager, TextField
-from django.shortcuts import get_object_or_404
+from django.db.models import CharField, TextField
 from lxml import etree
 from twisted.internet import reactor
 
@@ -17,33 +16,6 @@ from maasserver.utils.orm import post_commit_do
 from maasserver.utils.threads import deferToDatabase
 
 
-class TagManager(Manager):
-    """A utility to manage the collection of Tags."""
-
-    # Everyone can see all tags, but only superusers can edit tags.
-
-    def get_tag_or_404(self, name, user, to_edit=False):
-        """Fetch a `Tag` by name.  Raise exceptions if no `Tag` with
-        this name exist.
-
-        :param name: The Tag.name.
-        :type name: string
-        :param user: The user that should be used in the permission check.
-        :type user: django.contrib.auth.models.User
-        :param to_edit: Are we going to edit this tag, or just view it?
-        :type to_edit: bool
-        :raises: django.http.Http404_,
-            :class:`maasserver.exceptions.PermissionDenied`.
-
-        .. _django.http.Http404: https://
-           docs.djangoproject.com/en/dev/topics/http/views/
-           #the-http404-exception
-        """
-        if to_edit and not user.is_superuser:
-            raise PermissionDenied()
-        return get_object_or_404(Tag, name=name)
-
-
 class Tag(CleanSave, TimestampedModel):
     """A `Tag` is a label applied to a `Node`.
 
@@ -69,8 +41,6 @@ class Tag(CleanSave, TimestampedModel):
     comment = TextField(blank=True)
     kernel_opts = TextField(blank=True)
 
-    objects = TagManager()
-
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
         # Track what the original definition is, so we can detect when it