Fixes after TT02 rollout (#305)

* Added validation of date query parameter

* - Removed adaptive sampling
- Added unit test for appid

* Simplified catch

* Updated readdeletedinstances to reflect existing functionality

* Added retry loop in dataelement to address concurrent access.

* Removed unused parameter

---------

Co-authored-by: Henning Normann <h.normann@accenture.com>

src/Storage/Migration/v0.00/01-setup-tables.sql

@@ -68,7 +68,11 @@ TABLESPACE pg_default;
 
 CREATE INDEX IF NOT EXISTS dataelements_instanceinternalId ON storage.dataelements(instanceInternalId);
 CREATE INDEX IF NOT EXISTS dataelements_instanceguid ON storage.dataelements(instanceGuid);
-CREATE INDEX IF NOT EXISTS dataelements_deletestatus_harddeleted ON storage.dataelements (id) WHERE ((element -> 'DeleteStatus'::text) -> 'IsHardDeleted'::text)::boolean;
+CREATE INDEX IF NOT EXISTS instances_isharddeleted_and_more ON storage.instances(id)
+	WHERE (instance -> 'Status' -> 'IsHardDeleted')::BOOLEAN AND
+	(
+		NOT (instance -> 'Status' -> 'IsArchived')::BOOLEAN OR (instance -> 'CompleteConfirmations') IS NOT NULL
+	);
 
 CREATE INDEX IF NOT EXISTS instanceevents_instance ON storage.instanceevents(instance);
 

src/Storage/Migration/v0.00/03-setup-functions.sql

@@ -121,11 +121,13 @@ CREATE OR REPLACE FUNCTION storage.readdeletedinstances()
 AS $BODY$
 BEGIN
 RETURN QUERY
-    -- Make sure that part of the where clause is exactly as in filtered index instances_isharddeleted_confirmed
+    -- Make sure that part of the where clause is exactly as in filtered index instances_isharddeleted_and_more
     SELECT i.instance FROM storage.instances i
-    WHERE (i.instance -> 'Status' -> 'IsHardDeleted')::BOOLEAN
-        AND (i.instance -> 'CompleteConfirmations') IS NOT NULL
-        AND (i.instance -> 'Status' ->> 'HardDeleted')::TIMESTAMPTZ <= (NOW() - (7 ||' days')::INTERVAL);
+    WHERE (i.instance -> 'Status' -> 'IsHardDeleted')::BOOLEAN AND
+    (
+        NOT (i.instance -> 'Status' -> 'IsArchived')::BOOLEAN
+        OR (i.instance -> 'CompleteConfirmations') IS NOT NULL AND (i.instance -> 'Status' ->> 'HardDeleted')::TIMESTAMPTZ <= (NOW() - (7 ||' days')::INTERVAL)
+    );
 END;
 $BODY$;
 

src/Storage/Repository/PgDataRepository.cs

@@ -180,6 +180,34 @@ public async Task<Dictionary<string, List<DataElement>>> ReadAllForMultiple(List
 
         /// <inheritdoc/>
         public async Task<DataElement> Update(Guid instanceGuid, Guid dataElementId, Dictionary<string, object> propertylist)
+        {
+            // Retry loop below because Postgres doesn't lock the element between the read and the following update
+            // even if it's in a transaction with repeatable read.
+            PostgresException exceptionToRethrow = null;
+            for (int i = 0; i < 10; i++)
+            {
+                try
+                {
+                    return await UpdateInternal(dataElementId, propertylist);
+                }
+                catch (PostgresException e)
+                {
+                    if (e.Message == "40001: could not serialize access due to concurrent update")
+                    {
+                        exceptionToRethrow = e;
+                        await Task.Delay(100);
+                    }
+                    else
+                    {
+                        throw;
+                    }
+                }
+            }
+
+            throw exceptionToRethrow;
+        }
+
+        private async Task<DataElement> UpdateInternal(Guid dataElementId, Dictionary<string, object> propertylist)
         {
             if (propertylist.Count > 10)
             {

src/Storage/Repository/PgInstanceRepository.cs

@@ -506,10 +506,17 @@ private static void AddDateParam(string dateParam, StringValues queryValues, Dic
         {
             foreach (string value in queryValues)
             {
-                string @operator = value.Split(':')[0];
-                string dateValue = value[(@operator.Length + 1)..];
-                string postgresParamName = GetPgParamName($"{dateParam}_{@operator}");
-                postgresParams.Add(postgresParamName, valueAsString ? dateValue : DateTimeHelper.ParseAndConvertToUniversalTime(dateValue));
+                try
+                {
+                    string @operator = value.Split(':')[0];
+                    string dateValue = value[(@operator.Length + 1)..];
+                    string postgresParamName = GetPgParamName($"{dateParam}_{@operator}");
+                    postgresParams.Add(postgresParamName, valueAsString ? dateValue : DateTimeHelper.ParseAndConvertToUniversalTime(dateValue));
+                }
+                catch
+                {
+                    throw new ArgumentException($"Invalid date expression: {value} for query key: {dateParam}");
+                }
             }
         }
 

src/Storage/appsettings.json

@@ -51,6 +51,7 @@
       "Altinn.Platform.Storage.Controllers.CleanupController": "Information"
     },
     "ApplicationInsights": {
+      "EnableAdaptiveSampling": false,
       "LogLevel": {
         "Altinn.Platform.Storage.Controllers.CleanupController": "Information"
       }

test/UnitTest/TestingRepositories/InstanceTests.cs

@@ -167,6 +167,48 @@ public async Task Instance_GetInstancesFromQuery_Ok()
             Assert.Equal(1, instances1.Count);
         }
 
+        /// <summary>
+        /// Test GetInstancesFromQuery
+        /// </summary>
+        [Fact]
+        public async Task Instance_GetInstancesFromQuery_AppId_Ok()
+        {
+            // Arrange
+            await _instanceFixture.InstanceRepo.Create(TestData.Instance_1_1.Clone());
+
+            Dictionary<string, StringValues> queryParams = new();
+
+            // Act
+            var instances = await _instanceFixture.InstanceRepo.GetInstancesFromQuery(queryParams, null, 100);
+            queryParams.Add("appId", new StringValues("test-applikasjon-1"));
+
+            // Assert
+            Assert.Equal(1, instances.Count);
+        }
+
+        /// <summary>
+        /// Test GetInstancesFromQuery with bad date
+        /// </summary>
+        [Fact]
+        public async Task Instance_GetInstancesFromQuery_InvalidDate()
+        {
+            // Arrange
+            await _instanceFixture.InstanceRepo.Create(TestData.Instance_1_1.Clone());
+            await _instanceFixture.InstanceRepo.Create(TestData.Instance_1_2.Clone());
+            await _instanceFixture.InstanceRepo.Create(TestData.Instance_1_3.Clone());
+
+            Dictionary<string, StringValues> queryParams = new();
+
+            // Act
+            queryParams.Add("instanceOwner.partyId", new StringValues(TestData.Instance_1_3.InstanceOwner.PartyId));
+            queryParams.Add("process.ended", new StringValues("true"));
+            var instances = await _instanceFixture.InstanceRepo.GetInstancesFromQuery(queryParams, null, 100);
+
+            // Assert
+            Assert.Equal(0, instances.Count);
+            Assert.NotNull(instances.Exception);
+        }
+
         private async Task<Instance> InsertInstanceAndDataHardDelete(Instance instance, DataElement dataelement)
         {
             dataelement.DeleteStatus = new() { IsHardDeleted = true, HardDeleted = DateTime.Now.AddDays(-8).ToUniversalTime() };