From 828573b4670d88ec8d2d2520a670e75f24deaf06 Mon Sep 17 00:00:00 2001 From: jamesagnew Date: Wed, 9 Dec 2020 05:53:47 -0500 Subject: [PATCH] Improve synchronization on HashMapResourceProvider --- .../search/cache/SearchCacheStatusEnum.java | 20 +++++++++ .../empi/model/EmpiTransactionContext.java | 20 +++++++++ .../rest/server/TransactionLogMessages.java | 2 +- .../provider/HashMapResourceProvider.java | 42 ++++++++++--------- 4 files changed, 63 insertions(+), 21 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/cache/SearchCacheStatusEnum.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/cache/SearchCacheStatusEnum.java index 8537bbcd00c..3bfec97204c 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/cache/SearchCacheStatusEnum.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/search/cache/SearchCacheStatusEnum.java @@ -1,5 +1,25 @@ package ca.uhn.fhir.jpa.search.cache; +/*- + * #%L + * HAPI FHIR JPA Server + * %% + * Copyright (C) 2014 - 2020 University Health Network + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ + public enum SearchCacheStatusEnum { NOT_TRIED, diff --git a/hapi-fhir-server-empi/src/main/java/ca/uhn/fhir/empi/model/EmpiTransactionContext.java b/hapi-fhir-server-empi/src/main/java/ca/uhn/fhir/empi/model/EmpiTransactionContext.java index bae42966a7a..6e659028105 100644 --- a/hapi-fhir-server-empi/src/main/java/ca/uhn/fhir/empi/model/EmpiTransactionContext.java +++ b/hapi-fhir-server-empi/src/main/java/ca/uhn/fhir/empi/model/EmpiTransactionContext.java @@ -1,5 +1,25 @@ package ca.uhn.fhir.empi.model; +/*- + * #%L + * HAPI FHIR - Enterprise Master Patient Index + * %% + * Copyright (C) 2014 - 2020 University Health Network + * %% + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * #L% + */ + import ca.uhn.fhir.rest.server.TransactionLogMessages; public class EmpiTransactionContext { diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/TransactionLogMessages.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/TransactionLogMessages.java index b7ea82fe6a6..c5419d78cb8 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/TransactionLogMessages.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/TransactionLogMessages.java @@ -2,7 +2,7 @@ package ca.uhn.fhir.rest.server; /*- * #%L - * HAPI FHIR - Enterprise Master Patient Index + * HAPI FHIR - Server Framework * %% * Copyright (C) 2014 - 2020 University Health Network * %% diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/provider/HashMapResourceProvider.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/provider/HashMapResourceProvider.java index ce650300e58..4663fec4cfb 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/provider/HashMapResourceProvider.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/provider/HashMapResourceProvider.java @@ -95,8 +95,8 @@ public class HashMapResourceProvider implements IResour private final Class myResourceType; private final FhirContext myFhirContext; private final String myResourceName; - protected Map> myIdToVersionToResourceMap = Collections.synchronizedMap(new LinkedHashMap<>()); - protected Map> myIdToHistory = Collections.synchronizedMap(new LinkedHashMap<>()); + protected Map> myIdToVersionToResourceMap = new LinkedHashMap<>(); + protected Map> myIdToHistory = new LinkedHashMap<>(); protected LinkedList myTypeHistory = new LinkedList<>(); protected AtomicLong mySearchCount = new AtomicLong(0); private long myNextId; @@ -121,7 +121,7 @@ public class HashMapResourceProvider implements IResour /** * Clear all data held in this resource provider */ - public void clear() { + public synchronized void clear() { myNextId = 1; myIdToVersionToResourceMap.clear(); myIdToHistory.clear(); @@ -131,7 +131,7 @@ public class HashMapResourceProvider implements IResour /** * Clear the counts used by {@link #getCountRead()} and other count methods */ - public void clearCounts() { + public synchronized void clearCounts() { myReadCount.set(0L); myUpdateCount.set(0L); myCreateCount.set(0L); @@ -140,7 +140,7 @@ public class HashMapResourceProvider implements IResour } @Create - public MethodOutcome create(@ResourceParam T theResource, RequestDetails theRequestDetails) { + public synchronized MethodOutcome create(@ResourceParam T theResource, RequestDetails theRequestDetails) { TransactionDetails transactionDetails = new TransactionDetails(); createInternal(theResource, theRequestDetails, transactionDetails); @@ -158,12 +158,14 @@ public class HashMapResourceProvider implements IResour String idPartAsString = Long.toString(idPart); Long versionIdPart = 1L; + assert !myIdToVersionToResourceMap.containsKey(idPartAsString); + IIdType id = store(theResource, idPartAsString, versionIdPart, theRequestDetails, theTransactionDetails); theResource.setId(id); } @Delete - public MethodOutcome delete(@IdParam IIdType theId, RequestDetails theRequestDetails) { + public synchronized MethodOutcome delete(@IdParam IIdType theId, RequestDetails theRequestDetails) { TransactionDetails transactionDetails = new TransactionDetails(); TreeMap versions = myIdToVersionToResourceMap.get(theId.getIdPart()); @@ -185,7 +187,7 @@ public class HashMapResourceProvider implements IResour * This method returns a simple operation count. This is mostly * useful for testing purposes. */ - public long getCountCreate() { + public synchronized long getCountCreate() { return myCreateCount.get(); } @@ -193,7 +195,7 @@ public class HashMapResourceProvider implements IResour * This method returns a simple operation count. This is mostly * useful for testing purposes. */ - public long getCountDelete() { + public synchronized long getCountDelete() { return myDeleteCount.get(); } @@ -201,7 +203,7 @@ public class HashMapResourceProvider implements IResour * This method returns a simple operation count. This is mostly * useful for testing purposes. */ - public long getCountRead() { + public synchronized long getCountRead() { return myReadCount.get(); } @@ -209,7 +211,7 @@ public class HashMapResourceProvider implements IResour * This method returns a simple operation count. This is mostly * useful for testing purposes. */ - public long getCountSearch() { + public synchronized long getCountSearch() { return mySearchCount.get(); } @@ -217,7 +219,7 @@ public class HashMapResourceProvider implements IResour * This method returns a simple operation count. This is mostly * useful for testing purposes. */ - public long getCountUpdate() { + public synchronized long getCountUpdate() { return myUpdateCount.get(); } @@ -226,13 +228,13 @@ public class HashMapResourceProvider implements IResour return myResourceType; } - private synchronized TreeMap getVersionToResource(String theIdPart) { + private TreeMap getVersionToResource(String theIdPart) { myIdToVersionToResourceMap.computeIfAbsent(theIdPart, t -> new TreeMap<>()); return myIdToVersionToResourceMap.get(theIdPart); } @History - public List historyInstance(@IdParam IIdType theId, RequestDetails theRequestDetails) { + public synchronized List historyInstance(@IdParam IIdType theId, RequestDetails theRequestDetails) { LinkedList retVal = myIdToHistory.get(theId.getIdPart()); if (retVal == null) { throw new ResourceNotFoundException(theId); @@ -247,7 +249,7 @@ public class HashMapResourceProvider implements IResour } @Read(version = true) - public T read(@IdParam IIdType theId, RequestDetails theRequestDetails) { + public synchronized T read(@IdParam IIdType theId, RequestDetails theRequestDetails) { TreeMap versions = myIdToVersionToResourceMap.get(theId.getIdPart()); if (versions == null || versions.isEmpty()) { throw new ResourceNotFoundException(theId); @@ -280,14 +282,14 @@ public class HashMapResourceProvider implements IResour } @Search - public List searchAll(RequestDetails theRequestDetails) { + public synchronized List searchAll(RequestDetails theRequestDetails) { mySearchCount.incrementAndGet(); List retVal = getAllResources(); return fireInterceptorsAndFilterAsNeeded(retVal, theRequestDetails); } @Nonnull - protected List getAllResources() { + protected synchronized List getAllResources() { List retVal = new ArrayList<>(); for (TreeMap next : myIdToVersionToResourceMap.values()) { @@ -303,7 +305,7 @@ public class HashMapResourceProvider implements IResour } @Search - public List searchById( + public synchronized List searchById( @RequiredParam(name = "_id") TokenAndListParam theIds, RequestDetails theRequestDetails) { List retVal = new ArrayList<>(); @@ -424,7 +426,7 @@ public class HashMapResourceProvider implements IResour * @param theConditional This is provided only so that subclasses can implement if they want */ @Update - public MethodOutcome update( + public synchronized MethodOutcome update( @ResourceParam T theResource, @ConditionalUrlParam String theConditional, RequestDetails theRequestDetails) { @@ -472,7 +474,7 @@ public class HashMapResourceProvider implements IResour * @param theResource The resource to store. If the resource has an ID, that ID is updated. * @return Return the ID assigned to the stored resource */ - public IIdType store(T theResource) { + public synchronized IIdType store(T theResource) { if (theResource.getIdElement().hasIdPart()) { updateInternal(theResource, null, new TransactionDetails()); } else { @@ -486,7 +488,7 @@ public class HashMapResourceProvider implements IResour * * @since 4.1.0 */ - public List getStoredResources() { + public synchronized List getStoredResources() { List retVal = new ArrayList<>(); for (TreeMap next : myIdToVersionToResourceMap.values()) { retVal.add(next.lastEntry().getValue());