remove dojo_model_to/from_id decorator (#13984)

* remove dojo_model_to/from_id decorator

* remove dojo_model_from/to_id

* remove dojo_model_from/to_id

* remove dojo_model_from/to_id

* remove dojo_model_from/to_id

* fix tests

* remove leftover signature methods

* fix test counts

* fix test counts

* fix test counts

* Update dojo/settings/settings.dist.py

Co-authored-by: Cody Maffucci <46459665+Maffooch@users.noreply.github.com>

* fix test

---------

Co-authored-by: Cody Maffucci <46459665+Maffooch@users.noreply.github.com>

Author: valentijnscholten

dojo/api_v2/views.py

@@ -678,13 +678,13 @@ def update_jira_epic(self, request, pk=None):
         try:
 
             if engagement.has_jira_issue:
-                jira_helper.update_epic(engagement, **request.data)
+                jira_helper.update_epic(engagement.id, **request.data)
                 response = Response(
                     {"info": "Jira Epic update query sent"},
                     status=status.HTTP_200_OK,
                 )
             else:
-                jira_helper.add_epic(engagement, **request.data)
+                jira_helper.add_epic(engagement.id, **request.data)
                 response = Response(
                     {"info": "Jira Epic create query sent"},
                     status=status.HTTP_200_OK,

dojo/decorators.py

@@ -3,12 +3,11 @@
 from functools import wraps
 
 from django.conf import settings
-from django.db import models
 from django_ratelimit import UNSAFE
 from django_ratelimit.core import is_ratelimited
 from django_ratelimit.exceptions import Ratelimited
 
-from dojo.models import Dojo_User, Finding
+from dojo.models import Dojo_User
 
 logger = logging.getLogger(__name__)
 
@@ -116,81 +115,6 @@ def __wrapper__(*args, **kwargs):
     return decorator(func)
 
 
-# decorator with parameters needs another wrapper layer
-# example usage: @dojo_model_to_id(parameter=0) but defaults to parameter=0
-def dojo_model_to_id(_func=None, *, parameter=0):
-    # logger.debug('dec_args:' + str(dec_args))
-    # logger.debug('dec_kwargs:' + str(dec_kwargs))
-    # logger.debug('_func:%s', _func)
-
-    def dojo_model_to_id_internal(func, *args, **kwargs):
-        @wraps(func)
-        def __wrapper__(*args, **kwargs):
-            if not settings.CELERY_PASS_MODEL_BY_ID:
-                return func(*args, **kwargs)
-
-            model_or_id = get_parameter_froms_args_kwargs(args, kwargs, parameter)
-
-            if model_or_id:
-                if isinstance(model_or_id, models.Model) and we_want_async(*args, func=func, **kwargs):
-                    logger.debug("converting model_or_id to id: %s", model_or_id)
-                    args = list(args)
-                    args[parameter] = model_or_id.id
-
-            return func(*args, **kwargs)
-
-        return __wrapper__
-
-    if _func is None:
-        # decorator called without parameters
-        return dojo_model_to_id_internal
-    return dojo_model_to_id_internal(_func)
-
-
-# decorator with parameters needs another wrapper layer
-# example usage: @dojo_model_from_id(parameter=0, model=Finding) but defaults to parameter 0 and model Finding
-def dojo_model_from_id(_func=None, *, model=Finding, parameter=0):
-    # logger.debug('dec_args:' + str(dec_args))
-    # logger.debug('dec_kwargs:' + str(dec_kwargs))
-    # logger.debug('_func:%s', _func)
-    # logger.debug('model: %s', model)
-
-    def dojo_model_from_id_internal(func, *args, **kwargs):
-        @wraps(func)
-        def __wrapper__(*args, **kwargs):
-            if not settings.CELERY_PASS_MODEL_BY_ID:
-                return func(*args, **kwargs)
-
-            logger.debug("args:" + str(args))
-            logger.debug("kwargs:" + str(kwargs))
-
-            logger.debug("checking if we need to convert id to model: %s for parameter: %s", model.__name__, parameter)
-
-            model_or_id = get_parameter_froms_args_kwargs(args, kwargs, parameter)
-
-            if model_or_id:
-                if not isinstance(model_or_id, models.Model) and we_want_async(*args, func=func, **kwargs):
-                    logger.debug("instantiating model_or_id: %s for model: %s", model_or_id, model)
-                    try:
-                        instance = model.objects.get(id=model_or_id)
-                    except model.DoesNotExist:
-                        logger.warning("error instantiating model_or_id: %s for model: %s: DoesNotExist", model_or_id, model)
-                        instance = None
-                    args = list(args)
-                    args[parameter] = instance
-                else:
-                    logger.debug("model_or_id already a model instance %s for model: %s", model_or_id, model)
-
-            return func(*args, **kwargs)
-
-        return __wrapper__
-
-    if _func is None:
-        # decorator called without parameters
-        return dojo_model_from_id_internal
-    return dojo_model_from_id_internal(_func)
-
-
 def get_parameter_froms_args_kwargs(args, kwargs, parameter):
     model_or_id = None
     if isinstance(parameter, int):

dojo/endpoint/views.py

@@ -373,7 +373,7 @@ def endpoint_bulk_update_all(request, pid=None):
             product_calc = list(Product.objects.filter(endpoint__id__in=endpoints_to_update).distinct())
             endpoints.delete()
             for prod in product_calc:
-                calculate_grade(prod)
+                calculate_grade(prod.id)
 
             if skipped_endpoint_count > 0:
                 add_error_message_to_response(f"Skipped deletion of {skipped_endpoint_count} endpoints because you are not authorized.")

dojo/engagement/services.py

@@ -16,7 +16,7 @@ def close_engagement(eng):
     eng.save()
 
     if jira_helper.get_jira_project(eng):
-        jira_helper.close_epic(eng, push_to_jira=True)
+        jira_helper.close_epic(eng.id, push_to_jira=True)
 
 
 def reopen_engagement(eng):

dojo/engagement/views.py

@@ -390,7 +390,7 @@ def copy_engagement(request, eid):
         form = DoneForm(request.POST)
         if form.is_valid():
             engagement_copy = engagement.copy()
-            calculate_grade(product)
+            calculate_grade(product.id)
             messages.add_message(
                 request,
                 messages.SUCCESS,

dojo/finding/deduplication.py

@@ -8,7 +8,7 @@
 from django.db.models.query_utils import Q
 
 from dojo.celery import app
-from dojo.decorators import dojo_async_task, dojo_model_from_id, dojo_model_to_id
+from dojo.decorators import dojo_async_task
 from dojo.models import Finding, System_Settings
 
 logger = logging.getLogger(__name__)
@@ -45,12 +45,10 @@ def get_finding_models_for_deduplication(finding_ids):
     )
 
 
-@dojo_model_to_id
 @dojo_async_task
 @app.task
-@dojo_model_from_id
-def do_dedupe_finding_task(new_finding, *args, **kwargs):
-    return do_dedupe_finding(new_finding, *args, **kwargs)
+def do_dedupe_finding_task(new_finding_id, *args, **kwargs):
+    return do_dedupe_finding_task_internal(Finding.objects.get(id=new_finding_id), *args, **kwargs)
 
 
 @dojo_async_task
@@ -71,7 +69,7 @@ def do_dedupe_batch_task(finding_ids, *args, **kwargs):
     dedupe_batch_of_findings(findings)
 
 
-def do_dedupe_finding(new_finding, *args, **kwargs):
+def do_dedupe_finding_task_internal(new_finding, *args, **kwargs):
     from dojo.utils import get_custom_method  # noqa: PLC0415 -- circular import
     if dedupe_method := get_custom_method("FINDING_DEDUPE_METHOD"):
         return dedupe_method(new_finding, *args, **kwargs)

dojo/finding/helper.py

@@ -16,12 +16,12 @@
 import dojo.jira_link.helper as jira_helper
 import dojo.risk_acceptance.helper as ra_helper
 from dojo.celery import app
-from dojo.decorators import dojo_async_task, dojo_model_from_id, dojo_model_to_id
+from dojo.decorators import dojo_async_task
 from dojo.endpoint.utils import endpoint_get_or_create, save_endpoints_to_add
 from dojo.file_uploads.helper import delete_related_files
 from dojo.finding.deduplication import (
     dedupe_batch_of_findings,
-    do_dedupe_finding,
+    do_dedupe_finding_task_internal,
     get_finding_models_for_deduplication,
 )
 from dojo.models import (
@@ -43,6 +43,7 @@
     close_external_issue,
     do_false_positive_history,
     get_current_user,
+    get_object_or_none,
     mass_model_updater,
     to_str_typed,
 )
@@ -390,27 +391,14 @@ def add_findings_to_auto_group(name, findings, group_by, *, create_finding_group
                     finding_group.findings.add(*findings)
 
 
-@dojo_model_to_id
-@dojo_async_task(signature=True)
-@app.task
-@dojo_model_from_id
-def post_process_finding_save_signature(finding, dedupe_option=True, rules_option=True, product_grading_option=True,  # noqa: FBT002
-             issue_updater_option=True, push_to_jira=False, user=None, *args, **kwargs):  # noqa: FBT002 - this is bit hard to fix nice have this universally fixed
-    """
-    Returns a task signature for post-processing a finding. This is useful for creating task signatures
-    that can be used in chords or groups or to await results. We need this extra method because of our dojo_async decorator.
-    If we use more of these celery features, we should probably move away from that decorator.
-    """
-    return post_process_finding_save_internal(finding, dedupe_option, rules_option, product_grading_option,
-                                   issue_updater_option, push_to_jira, user, *args, **kwargs)
-
-
-@dojo_model_to_id
 @dojo_async_task
 @app.task
-@dojo_model_from_id
-def post_process_finding_save(finding, dedupe_option=True, rules_option=True, product_grading_option=True,  # noqa: FBT002
+def post_process_finding_save(finding_id, dedupe_option=True, rules_option=True, product_grading_option=True,  # noqa: FBT002
              issue_updater_option=True, push_to_jira=False, user=None, *args, **kwargs):  # noqa: FBT002 - this is bit hard to fix nice have this universally fixed
+    finding = get_object_or_none(Finding, id=finding_id)
+    if not finding:
+        logger.warning("Finding with id %s does not exist, skipping post_process_finding_save", finding_id)
+        return None
 
     return post_process_finding_save_internal(finding, dedupe_option, rules_option, product_grading_option,
                                    issue_updater_option, push_to_jira, user, *args, **kwargs)
@@ -429,7 +417,7 @@ def post_process_finding_save_internal(finding, dedupe_option=True, rules_option
     if dedupe_option:
         if finding.hash_code is not None:
             if system_settings.enable_deduplication:
-                do_dedupe_finding(finding, *args, **kwargs)
+                do_dedupe_finding_task_internal(finding, *args, **kwargs)
             else:
                 deduplicationLogger.debug("skipping dedupe because it's disabled in system settings")
         else:
@@ -448,7 +436,7 @@ def post_process_finding_save_internal(finding, dedupe_option=True, rules_option
 
     if product_grading_option:
         if system_settings.enable_product_grade:
-            calculate_grade(finding.test.engagement.product)
+            calculate_grade(finding.test.engagement.product.id)
         else:
             deduplicationLogger.debug("skipping product grading because it's disabled in system settings")
 
@@ -465,14 +453,6 @@ def post_process_finding_save_internal(finding, dedupe_option=True, rules_option
             jira_helper.push_to_jira(finding.finding_group)
 
 
-@dojo_async_task(signature=True)
-@app.task
-def post_process_findings_batch_signature(finding_ids, *args, dedupe_option=True, rules_option=True, product_grading_option=True,
-             issue_updater_option=True, push_to_jira=False, user=None, **kwargs):
-    return post_process_findings_batch(finding_ids, *args, dedupe_option=dedupe_option, rules_option=rules_option, product_grading_option=product_grading_option, issue_updater_option=issue_updater_option, push_to_jira=push_to_jira, user=user, **kwargs)
-    # Pass arguments as keyword arguments to ensure Celery properly serializes them
-
-
 @dojo_async_task
 @app.task
 def post_process_findings_batch(finding_ids, *args, dedupe_option=True, rules_option=True, product_grading_option=True,
@@ -516,7 +496,7 @@ def post_process_findings_batch(finding_ids, *args, dedupe_option=True, rules_op
             tool_issue_updater.async_tool_issue_update(finding)
 
     if product_grading_option and system_settings.enable_product_grade:
-        calculate_grade(findings[0].test.engagement.product)
+        calculate_grade(findings[0].test.engagement.product.id)
 
     if push_to_jira:
         for finding in findings:
@@ -1038,7 +1018,7 @@ def close_finding(
     ra_helper.risk_unaccept(user, finding, perform_save=False)
 
     # External issues (best effort)
-    close_external_issue(finding, "Closed by defectdojo", "github")
+    close_external_issue(finding.id, "Closed by defectdojo", "github")
 
     # JIRA sync
     push_to_jira = False

dojo/finding/views.py

@@ -1003,9 +1003,9 @@ def process_github_form(self, request: HttpRequest, finding: Finding, context: d
 
         if context["gform"].is_valid():
             if GITHUB_Issue.objects.filter(finding=finding).exists():
-                update_external_issue(finding, old_status, "github")
+                update_external_issue(finding.id, old_status, "github")
             else:
-                add_external_issue(finding, "github")
+                add_external_issue(finding.id, "github")
 
             return request, True
         add_field_errors_to_response(context["gform"])
@@ -1082,7 +1082,7 @@ def process_form(self, request: HttpRequest, finding: Finding, context: dict):
             product = finding.test.engagement.product
             finding.delete()
             # Update the grade of the product async
-            calculate_grade(product)
+            calculate_grade(product.id)
             # Add a message to the request that the finding was successfully deleted
             messages.add_message(
                 request,
@@ -1318,7 +1318,7 @@ def reopen_finding(request, fid):
     if jira_helper.is_push_all_issues(finding) or jira_helper.is_keep_in_sync_with_jira(finding):
         jira_helper.push_to_jira(finding)
 
-    reopen_external_issue(finding, "re-opened by defectdojo", "github")
+    reopen_external_issue(finding.id, "re-opened by defectdojo", "github")
 
     messages.add_message(
         request, messages.SUCCESS, "Finding Reopened.", extra_tags="alert-success",
@@ -1353,7 +1353,7 @@ def copy_finding(request, fid):
             test = form.cleaned_data.get("test")
             product = finding.test.engagement.product
             finding_copy = finding.copy(test=test)
-            calculate_grade(product)
+            calculate_grade(product.id)
             messages.add_message(
                 request,
                 messages.SUCCESS,
@@ -2101,7 +2101,7 @@ def promote_to_finding(request, fid):
                     ).push_all_issues,
                 )
                 if gform.is_valid():
-                    add_external_issue(new_finding, "github")
+                    add_external_issue(new_finding.id, "github")
 
             messages.add_message(
                 request,
@@ -2733,7 +2733,7 @@ def _bulk_update_finding_status_and_severity(finds, form, request, system_settin
                             fp.save_no_options()
 
         for prod in prods:
-            calculate_grade(prod)
+            calculate_grade(prod.id)
 
     if skipped_duplicate_count > 0:
         messages.add_message(
@@ -2789,7 +2789,7 @@ def _bulk_update_risk_acceptance(finds, form, request, prods):
                 ra_helper.risk_unaccept(request.user, finding)
 
         for prod in prods:
-            calculate_grade(prod)
+            calculate_grade(prod.id)
 
     if skipped_risk_accept_count > 0:
         messages.add_message(
@@ -3084,9 +3084,9 @@ def finding_bulk_update_all(request, pid=None):
                     old_status = finding.status()
                     if form.cleaned_data["push_to_github"]:
                         if GITHUB_Issue.objects.filter(finding=finding).exists():
-                            update_external_issue(finding, old_status, "github")
+                            update_external_issue(finding.id, old_status, "github")
                         else:
-                            add_external_issue(finding, "github")
+                            add_external_issue(finding.id, "github")
 
             if form.cleaned_data["notes"]:
                 logger.debug("Setting bulk notes")

dojo/importers/base_importer.py

@@ -668,7 +668,7 @@ def maybe_launch_post_processing_chord(
             product = self.test.engagement.product
             system_settings = System_Settings.objects.get()
             if system_settings.enable_product_grade:
-                calculate_grade_signature = utils.calculate_grade_signature(product)
+                calculate_grade_signature = utils.calculate_grade.si(product.id)
                 chord(post_processing_task_signatures)(calculate_grade_signature)
             else:
                 group(post_processing_task_signatures).apply_async()