From 126b4e9d663798fa634f6111ebd30b1f72d0e43f Mon Sep 17 00:00:00 2001 From: temi Date: Wed, 1 May 2024 15:38:26 +1000 Subject: [PATCH 1/4] #932 - fix concurrent dataset summary overwrite --- grails-app/services/au/org/ala/ecodata/ParatooService.groovy | 5 ++++- src/test/groovy/au/org/ala/ecodata/ParatooServiceSpec.groovy | 2 ++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/grails-app/services/au/org/ala/ecodata/ParatooService.groovy b/grails-app/services/au/org/ala/ecodata/ParatooService.groovy index c0d745bf9..edf751f69 100644 --- a/grails-app/services/au/org/ala/ecodata/ParatooService.groovy +++ b/grails-app/services/au/org/ala/ecodata/ParatooService.groovy @@ -302,7 +302,10 @@ class ParatooService { dataSet.format = DATASET_DATABASE_TABLE dataSet.sizeUnknown = true - projectService.update([custom: project.project.custom], project.id, false) + Map latestProject = projectService.get(project.project.projectId) + Map latestDataSet = latestProject.custom?.dataSets?.find{it.dataSetId == collection.orgMintedUUID} + latestDataSet.putAll(dataSet) + projectService.update([custom: latestProject.custom], project.id, false) } } } diff --git a/src/test/groovy/au/org/ala/ecodata/ParatooServiceSpec.groovy b/src/test/groovy/au/org/ala/ecodata/ParatooServiceSpec.groovy index a57dc2dfe..cd9424c97 100644 --- a/src/test/groovy/au/org/ala/ecodata/ParatooServiceSpec.groovy +++ b/src/test/groovy/au/org/ala/ecodata/ParatooServiceSpec.groovy @@ -192,6 +192,7 @@ class ParatooServiceSpec extends MongoSpec implements ServiceUnitTest> [resp: [collections: ["coarse-woody-debris-survey": [uuid: "1", createdAt: "2023-09-01T00:00:00.123Z", start_date_time: "2023-09-01T00:00:00.123Z", end_date_time: "2023-09-01T00:00:00.123Z"]]]] 1 * tokenService.getAuthToken(true) >> Mock(AccessToken) + 1 * projectService.get(projectId) >> [projectId: projectId, custom: [dataSets: [dataSet]]] 1 * projectService.update([custom: [dataSets: [expectedDataSetAsync]]], 'p1', false) >> [status: 'ok'] 1 * projectService.update([custom: [dataSets: [expectedDataSetSync]]], 'p1', false) >> [status: 'ok'] 1 * activityService.create(_) >> [activityId: '123'] @@ -302,6 +303,7 @@ class ParatooServiceSpec extends MongoSpec implements ServiceUnitTest> [resp: surveyData] 1 * tokenService.getAuthToken(true) >> Mock(AccessToken) 2 * projectService.update(_, projectId, false) >> [status: 'ok'] + 1 * projectService.get(projectId) >> [projectId: projectId, custom: [dataSets: [dataSet]]] 1 * siteService.create(_) >> { site = it[0]; [siteId: 's1'] } 1 * activityService.create(_) >> [activityId: '123'] 1 * recordService.getAllByActivity('123') >> [] From 45809294224c0c986692aa5f84de1fd0c8d841b0 Mon Sep 17 00:00:00 2001 From: temi Date: Wed, 1 May 2024 15:49:53 +1000 Subject: [PATCH 2/4] #932 - 4.5.1-SNAPSHOT --- build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle b/build.gradle index 93e530559..779ba0bac 100644 --- a/build.gradle +++ b/build.gradle @@ -20,7 +20,7 @@ plugins { id "com.gorylenko.gradle-git-properties" version "2.4.1" } -version "4.5" +version "4.5.1-SNAPSHOT" group "au.org.ala" description "Ecodata" From 616fb3cf6a85843e78e4b9c5cb3d526f3f8cfe4b Mon Sep 17 00:00:00 2001 From: temi Date: Wed, 1 May 2024 16:22:29 +1000 Subject: [PATCH 3/4] #932 - implemented locking before updating dataset summary --- .../au/org/ala/ecodata/ParatooService.groovy | 42 ++++++++++++------- .../org/ala/ecodata/ParatooServiceSpec.groovy | 6 ++- 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/grails-app/services/au/org/ala/ecodata/ParatooService.groovy b/grails-app/services/au/org/ala/ecodata/ParatooService.groovy index edf751f69..7e2ee8013 100644 --- a/grails-app/services/au/org/ala/ecodata/ParatooService.groovy +++ b/grails-app/services/au/org/ala/ecodata/ParatooService.groovy @@ -18,6 +18,7 @@ import static grails.async.Promises.task */ @Slf4j class ParatooService { + static final Object LOCK = new Object() static final String DATASET_DATABASE_TABLE = 'Database Table' static final int PARATOO_MAX_RETRIES = 3 static final String PARATOO_PROTOCOL_PATH = '/protocols' @@ -182,18 +183,21 @@ class ParatooService { dataSet.surveyId = paratooCollectionId.toMap() // No codec to save this to mongo - if (!project.custom) { - project.custom = [:] - } - if (!project.custom.dataSets) { - project.custom.dataSets = [] - } - dataSet.orgMintedIdentifier = paratooCollectionId.encodeAsOrgMintedIdentifier() log.info "Minting identifier for Monitor collection: ${paratooCollectionId}: ${dataSet.orgMintedIdentifier}" - project.custom.dataSets << dataSet - Map result = projectService.update([custom: project.custom], projectId, false) + Map result + synchronized (LOCK) { + Map latestProject = projectService.get(projectId) + if (!latestProject.custom) { + latestProject.custom = [:] + } + if (!latestProject.custom.dataSets) { + latestProject.custom.dataSets = [] + } + latestProject.custom.dataSets << dataSet + result = projectService.update([custom: latestProject.custom], projectId, false) + } if (!result.error) { result.orgMintedIdentifier = dataSet.orgMintedIdentifier @@ -233,7 +237,15 @@ class ParatooService { promise.onError { Throwable e -> log.error("An error occurred feching ${collection.orgMintedUUID}: ${e.message}", e) } - def result = projectService.update([custom: project.project.custom], project.id, false) + + def result + synchronized (LOCK) { + Map latestProject = projectService.get(project.id) + Map latestDataSet = latestProject.custom?.dataSets?.find { it.dataSetId == collection.orgMintedUUID } + latestDataSet.putAll(dataSet) + result = projectService.update([custom: latestProject.custom], project.id, false) + } + [updateResult: result, promise: promise] } @@ -302,10 +314,12 @@ class ParatooService { dataSet.format = DATASET_DATABASE_TABLE dataSet.sizeUnknown = true - Map latestProject = projectService.get(project.project.projectId) - Map latestDataSet = latestProject.custom?.dataSets?.find{it.dataSetId == collection.orgMintedUUID} - latestDataSet.putAll(dataSet) - projectService.update([custom: latestProject.custom], project.id, false) + synchronized (LOCK) { + Map latestProject = projectService.get(project.project.projectId) + Map latestDataSet = latestProject.custom?.dataSets?.find { it.dataSetId == collection.orgMintedUUID } + latestDataSet.putAll(dataSet) + projectService.update([custom: latestProject.custom], project.id, false) + } } } } diff --git a/src/test/groovy/au/org/ala/ecodata/ParatooServiceSpec.groovy b/src/test/groovy/au/org/ala/ecodata/ParatooServiceSpec.groovy index cd9424c97..8a9726d3b 100644 --- a/src/test/groovy/au/org/ala/ecodata/ParatooServiceSpec.groovy +++ b/src/test/groovy/au/org/ala/ecodata/ParatooServiceSpec.groovy @@ -142,11 +142,13 @@ class ParatooServiceSpec extends MongoSpec implements ServiceUnitTest> project 1 * projectService.update(_, projectId, false) >> { data, pId, updateCollectory -> Map dataSet = data.custom.dataSets[1] // The stubbed project already has a dataSet, so the new one will be index=1 assert dataSet.surveyId != null @@ -192,7 +194,7 @@ class ParatooServiceSpec extends MongoSpec implements ServiceUnitTest> [resp: [collections: ["coarse-woody-debris-survey": [uuid: "1", createdAt: "2023-09-01T00:00:00.123Z", start_date_time: "2023-09-01T00:00:00.123Z", end_date_time: "2023-09-01T00:00:00.123Z"]]]] 1 * tokenService.getAuthToken(true) >> Mock(AccessToken) - 1 * projectService.get(projectId) >> [projectId: projectId, custom: [dataSets: [dataSet]]] + 2 * projectService.get(projectId) >> [projectId: projectId, custom: [dataSets: [dataSet]]] 1 * projectService.update([custom: [dataSets: [expectedDataSetAsync]]], 'p1', false) >> [status: 'ok'] 1 * projectService.update([custom: [dataSets: [expectedDataSetSync]]], 'p1', false) >> [status: 'ok'] 1 * activityService.create(_) >> [activityId: '123'] @@ -303,7 +305,7 @@ class ParatooServiceSpec extends MongoSpec implements ServiceUnitTest> [resp: surveyData] 1 * tokenService.getAuthToken(true) >> Mock(AccessToken) 2 * projectService.update(_, projectId, false) >> [status: 'ok'] - 1 * projectService.get(projectId) >> [projectId: projectId, custom: [dataSets: [dataSet]]] + 2 * projectService.get(projectId) >> [projectId: projectId, custom: [dataSets: [dataSet]]] 1 * siteService.create(_) >> { site = it[0]; [siteId: 's1'] } 1 * activityService.create(_) >> [activityId: '123'] 1 * recordService.getAllByActivity('123') >> [] From eb559fc87895cf473b84504f5025963bc800d401 Mon Sep 17 00:00:00 2001 From: temi Date: Wed, 1 May 2024 16:52:10 +1000 Subject: [PATCH 4/4] #932 - fixed broken test --- .../org/ala/ecodata/ParatooServiceSpec.groovy | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/test/groovy/au/org/ala/ecodata/ParatooServiceSpec.groovy b/src/test/groovy/au/org/ala/ecodata/ParatooServiceSpec.groovy index 8a9726d3b..f79449bdf 100644 --- a/src/test/groovy/au/org/ala/ecodata/ParatooServiceSpec.groovy +++ b/src/test/groovy/au/org/ala/ecodata/ParatooServiceSpec.groovy @@ -142,7 +142,7 @@ class ParatooServiceSpec extends MongoSpec implements ServiceUnitTest