From 2c2898f8679f6d4f77e387ae4803efa9c969234b Mon Sep 17 00:00:00 2001
From: Terje Holene <terje.holene@gmail.com>
Date: Mon, 8 May 2023 16:31:50 +0200
Subject: [PATCH] Improve handling of non existing source instance (#236)

* Improve handling of non existing source instance
* Add test to improve coverage
---
 .../Controllers/InstancesController.cs        | 21 ++++-
 .../InstancesController_CopyInstanceTests.cs  | 78 +++++++++++++++++++
 2 files changed, 98 insertions(+), 1 deletion(-)

diff --git a/src/Altinn.App.Api/Controllers/InstancesController.cs b/src/Altinn.App.Api/Controllers/InstancesController.cs
index d7e7a1b17..aec29404d 100644
--- a/src/Altinn.App.Api/Controllers/InstancesController.cs
+++ b/src/Altinn.App.Api/Controllers/InstancesController.cs
@@ -508,7 +508,7 @@ public async Task<ActionResult> CopyInstance(
                 return Forbidden(readAccess);
             }
 
-            Instance sourceInstance = await _instanceClient.GetInstance(app, org, instanceOwnerPartyId, instanceGuid);
+            Instance? sourceInstance = await GetInstance(org, app, instanceOwnerPartyId, instanceGuid);
 
             if (sourceInstance?.Status?.IsArchived is null or false)
             {
@@ -724,6 +724,25 @@ public async Task<ActionResult<List<SimpleInstance>>> GetActiveInstances([FromRo
             return Ok(SimpleInstanceMapper.MapInstanceListToSimpleInstanceList(activeInstances, userAndOrgLookup));
         }
 
+        private async Task<Instance?> GetInstance(string org, string app, int instanceOwnerPartyId, Guid instanceGuid)
+        {
+            try
+            {
+                return await _instanceClient.GetInstance(app, org, instanceOwnerPartyId, instanceGuid);
+            }
+            catch (PlatformHttpException platformHttpException)
+            {
+                switch (platformHttpException.Response.StatusCode)
+                {
+                    case HttpStatusCode.Forbidden: // Storage returns 403 for non-existing instances
+                    case HttpStatusCode.NotFound:
+                        return null;
+                    default:
+                        throw;
+                }
+            }
+        }
+
         private void ConditionallySetReadStatus(Instance instance)
         {
             string? orgClaimValue = User.GetOrg();
diff --git a/test/Altinn.App.Api.Tests/Controllers/InstancesController_CopyInstanceTests.cs b/test/Altinn.App.Api.Tests/Controllers/InstancesController_CopyInstanceTests.cs
index e6d9f3265..a26bb125a 100644
--- a/test/Altinn.App.Api.Tests/Controllers/InstancesController_CopyInstanceTests.cs
+++ b/test/Altinn.App.Api.Tests/Controllers/InstancesController_CopyInstanceTests.cs
@@ -2,6 +2,7 @@
 using Altinn.App.Api.Tests.Utils;
 using Altinn.App.Core.Configuration;
 using Altinn.App.Core.Features;
+using Altinn.App.Core.Helpers;
 using Altinn.App.Core.Interface;
 using Altinn.App.Core.Internal.App;
 using Altinn.App.Core.Internal.AppModel;
@@ -198,6 +199,83 @@ public async Task CopyInstance_InstanceNotArchived_ReturnsBadRequest()
         VerifyNoOtherCalls();
     }
 
+    [Fact]
+    public async Task CopyInstance_InstanceDoesNotExists_ReturnsBadRequest()
+    {
+        // Arrange
+        const string Org = "ttd";
+        const string AppName = "copy-instance";
+        int instanceOwnerPartyId = 343234;
+        Guid instanceGuid = Guid.NewGuid();
+
+        // Storage returns Forbidden if the given instance id is wrong.
+        PlatformHttpException platformHttpException = 
+            await PlatformHttpException.CreateAsync(new HttpResponseMessage(System.Net.HttpStatusCode.Forbidden));
+
+        _httpContextMock.Setup(httpContext => httpContext.User).Returns(PrincipalUtil.GetUserPrincipal(1337));
+        _appMetadata.Setup(a => a.GetApplicationMetadata())
+            .ReturnsAsync(CreateApplicationMetadata($"{Org}/{AppName}", true));
+        _pdp.Setup<Task<XacmlJsonResponse>>(p => p.GetDecisionForRequest(It.IsAny<XacmlJsonRequestRoot>()))
+            .ReturnsAsync(CreateXacmlResponse("Permit"));
+        _instanceClient.Setup(i => i.GetInstance(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<int>(), It.IsAny<Guid>()))
+            .ThrowsAsync(platformHttpException);
+
+        // Act
+        ActionResult actual = await SUT.CopyInstance("ttd", "copy-instance", instanceOwnerPartyId, instanceGuid);
+
+        // Assert
+        Assert.IsType<BadRequestObjectResult>(actual);
+        BadRequestObjectResult badRequest = (BadRequestObjectResult)actual;
+        Assert.Contains("instance being copied must be archived", badRequest!.Value!.ToString());
+
+        _appMetadata.VerifyAll();
+        _pdp.VerifyAll();
+        _instanceClient.VerifyAll();
+        VerifyNoOtherCalls();
+    }
+
+    [Fact]
+    public async Task CopyInstance_PlatformReturnsError_ThrowsException()
+    {
+        // Arrange
+        const string Org = "ttd";
+        const string AppName = "copy-instance";
+        int instanceOwnerPartyId = 343234;
+        Guid instanceGuid = Guid.NewGuid();
+
+        // Simulate a BadGateway respons from Platform
+        PlatformHttpException platformHttpException =
+            await PlatformHttpException.CreateAsync(new HttpResponseMessage(System.Net.HttpStatusCode.BadGateway));
+
+        _httpContextMock.Setup(httpContext => httpContext.User).Returns(PrincipalUtil.GetUserPrincipal(1337));
+        _appMetadata.Setup(a => a.GetApplicationMetadata())
+            .ReturnsAsync(CreateApplicationMetadata($"{Org}/{AppName}", true));
+        _pdp.Setup<Task<XacmlJsonResponse>>(p => p.GetDecisionForRequest(It.IsAny<XacmlJsonRequestRoot>()))
+            .ReturnsAsync(CreateXacmlResponse("Permit"));
+        _instanceClient.Setup(i => i.GetInstance(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<int>(), It.IsAny<Guid>()))
+            .ThrowsAsync(platformHttpException);
+
+        PlatformHttpException? actual = null;
+
+        // Act
+        try
+        {
+            await SUT.CopyInstance("ttd", "copy-instance", instanceOwnerPartyId, instanceGuid);
+        }
+        catch (PlatformHttpException phe)
+        {
+            actual = phe;
+        }
+
+        // Assert
+        Assert.NotNull(actual);
+
+        _appMetadata.VerifyAll();
+        _pdp.VerifyAll();
+        _instanceClient.VerifyAll();
+        VerifyNoOtherCalls();
+    }
+
     [Fact]
     public async Task CopyInstance_InstantiationValidationFails_ReturnsForbidden()
     {