From bf501b0198805c3109409b5fa22bfb00a6a96583 Mon Sep 17 00:00:00 2001
From: Jeroen Castelein <jeroen.castelein@kpn.com>
Date: Thu, 5 Dec 2019 11:00:40 +0100
Subject: [PATCH] FIX: Create PipelineRun return 409 if already exists,
 separate different 409s with details in the response (#TIPD-2118)

---
 katka/exceptions.py                           | 13 +++++++++++
 katka/releases.py                             |  2 +-
 katka/views.py                                | 11 +++++++--
 tests/integration/test_scmpipelinerun_view.py | 23 +++++++++++++++++++
 4 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/katka/exceptions.py b/katka/exceptions.py
index 1b29293..6d01020 100644
--- a/katka/exceptions.py
+++ b/katka/exceptions.py
@@ -12,3 +12,16 @@ class Conflict(APIException):
     status_code = status.HTTP_409_CONFLICT
     default_detail = _("Conflict.")
     default_code = "conflict"
+
+
+class AlreadyExists(Conflict):
+    default_detail = {"detail": "Object already exists.", "code": "already_exists"}
+    default_code = "already_exists"
+
+
+class ParentCommitMissing(Conflict):
+    default_detail = {
+        "detail": "Pipeline Run could not be created, no pipeline run found for parent commit hash.",
+        "code": "parent_commit_missing",
+    }
+    default_code = "parent_commit_missing"
diff --git a/katka/releases.py b/katka/releases.py
index fb6d63e..07ed644 100644
--- a/katka/releases.py
+++ b/katka/releases.py
@@ -103,7 +103,7 @@ def _add_output(pipeline_output: dict, step_output: str) -> None:
     try:
         pipeline_output.update(json.loads(step_output))
     except json.JSONDecodeError:
-        logging.warning("Invalid JSON in step output")
+        log.warning("Invalid JSON in step output")
 
 
 def _get_current_release(pipeline):
diff --git a/katka/views.py b/katka/views.py
index 9b65702..2d1a08f 100644
--- a/katka/views.py
+++ b/katka/views.py
@@ -2,6 +2,7 @@
 from datetime import datetime
 
 from django.conf import settings
+from django.db import IntegrityError
 
 import pytz
 from katka.constants import (
@@ -11,7 +12,7 @@
     PIPELINE_STATUS_QUEUED,
     STEP_FINAL_STATUSES,
 )
-from katka.exceptions import Conflict
+from katka.exceptions import AlreadyExists, ParentCommitMissing
 from katka.models import (
     Application,
     ApplicationMetadata,
@@ -134,11 +135,17 @@ def _can_create(self, application, parent_hash):
 
         return True
 
+    def create(self, request, *args, **kwargs):
+        try:
+            return super().create(request, *args, **kwargs)
+        except IntegrityError:
+            raise AlreadyExists()
+
     def perform_create(self, serializer):
         if not self._can_create(
             serializer.validated_data["application"], serializer.validated_data.get("first_parent_hash")
         ):
-            raise Conflict()
+            raise ParentCommitMissing()
         super().perform_create(serializer)
 
     def _ready_to_run(self, pipeline_run):
diff --git a/tests/integration/test_scmpipelinerun_view.py b/tests/integration/test_scmpipelinerun_view.py
index af8a413..f2a31ec 100644
--- a/tests/integration/test_scmpipelinerun_view.py
+++ b/tests/integration/test_scmpipelinerun_view.py
@@ -1,5 +1,7 @@
 from uuid import UUID
 
+from django.db import transaction
+
 import pytest
 from katka import models
 from katka.constants import PIPELINE_STATUS_FAILED
@@ -220,6 +222,25 @@ def test_partial_update(self, client, logged_in_user, scm_pipeline_run):
         p = models.SCMPipelineRun.objects.get(pk=scm_pipeline_run.public_identifier)
         assert p.steps_completed == 4
 
+    def test_create_already_exists(self, client, logged_in_user, application, scm_pipeline_run):
+        initial_count = models.SCMPipelineRun.objects.count()
+        url = f"/scm-pipeline-runs/"
+        # Provide a different status, should not create a new PLR
+        data = {
+            "commit_hash": scm_pipeline_run.commit_hash,
+            "application": application.public_identifier,
+            "status": "skipped",
+        }
+
+        #  This atomic transaction context allows us to do a count query after the request
+        with transaction.atomic():
+            response = client.post(url, data=data, content_type="application/json")
+
+        assert response.status_code == 409
+        error = response.json()
+        assert error["code"] == "already_exists"
+        assert models.SCMPipelineRun.objects.count() == initial_count
+
     def test_create_first_commit(self, client, logged_in_user, application, scm_pipeline_run):
         initial_count = models.SCMPipelineRun.objects.count()
         url = f"/scm-pipeline-runs/"
@@ -254,6 +275,8 @@ def test_create_parent_commit_not_found(self, client, logged_in_user, applicatio
         }
         response = client.post(url, data=data, content_type="application/json")
         assert response.status_code == 409
+        error = response.json()
+        assert error["code"] == "parent_commit_missing"
         assert models.SCMPipelineRun.objects.count() == initial_count
         assert not models.SCMPipelineRun.objects.filter(commit_hash="874AE57A143AEC5156FD1444A017A32137A3E34A").exists()