From 8dd1ec641118eaf1255527f2bbb849933da02e10 Mon Sep 17 00:00:00 2001 From: "tyge.folke.nielsen" Date: Tue, 22 Feb 2022 14:59:23 +0100 Subject: [PATCH] FUT1-5892 revert "3156 multiple tag race condition (#3297)" - Issue https://github.com/hapifhir/hapi-fhir/issues/3412 --- .../ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java | 167 ++++-------------- 1 file changed, 39 insertions(+), 128 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java index e117fe997ff..fa8eca6724a 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java @@ -1,6 +1,5 @@ package ca.uhn.fhir.jpa.dao; -import ca.uhn.fhir.i18n.Msg; import ca.uhn.fhir.context.BaseRuntimeChildDefinition; import ca.uhn.fhir.context.BaseRuntimeElementCompositeDefinition; import ca.uhn.fhir.context.BaseRuntimeElementDefinition; @@ -80,7 +79,6 @@ import ca.uhn.fhir.rest.api.server.IBundleProvider; import ca.uhn.fhir.rest.api.server.RequestDetails; import ca.uhn.fhir.rest.api.server.storage.TransactionDetails; -import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException; import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException; import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException; @@ -120,12 +118,8 @@ import org.springframework.context.ApplicationContextAware; import org.springframework.stereotype.Repository; import org.springframework.transaction.PlatformTransactionManager; -import org.springframework.transaction.TransactionDefinition; -import org.springframework.transaction.TransactionStatus; -import org.springframework.transaction.support.TransactionCallback; import org.springframework.transaction.support.TransactionSynchronization; import org.springframework.transaction.support.TransactionSynchronizationManager; -import org.springframework.transaction.support.TransactionTemplate; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -154,7 +148,6 @@ import java.util.Set; import java.util.StringTokenizer; import java.util.UUID; -import java.util.stream.Collectors; import static org.apache.commons.lang3.StringUtils.defaultIfBlank; import static org.apache.commons.lang3.StringUtils.defaultString; @@ -187,9 +180,6 @@ @Repository public abstract class BaseHapiFhirDao extends BaseStorageDao implements IDao, IJpaDao, ApplicationContextAware { - // total attempts to do a tag transaction - private static final int TOTAL_TAG_READ_ATTEMPTS = 10; - public static final long INDEX_STATUS_INDEXED = 1L; public static final long INDEX_STATUS_INDEXING_FAILED = 2L; public static final String NS_JPA_PROFILE = "https://github.com/hapifhir/hapi-fhir/ns/jpa/profile"; @@ -257,9 +247,6 @@ public abstract class BaseHapiFhirDao extends BaseStora @Autowired(required = false) private IFulltextSearchSvc myFulltextSearchSvc; - @Autowired - private PlatformTransactionManager myTransactionManager; - @VisibleForTesting public void setSearchParamPresenceSvc(ISearchParamPresenceSvc theSearchParamPresenceSvc) { mySearchParamPresenceSvc = theSearchParamPresenceSvc; @@ -411,15 +398,40 @@ protected TagDefinition getTagOrNull(TransactionDetails theTransactionDetails, T MemoryCacheService.TagDefinitionCacheKey key = toTagDefinitionMemoryCacheKey(theTagType, theScheme, theTerm); + TagDefinition retVal = myMemoryCacheService.getIfPresent(MemoryCacheService.CacheEnum.TAG_DEFINITION, key); + if (retVal == null) { HashMap resolvedTagDefinitions = theTransactionDetails.getOrCreateUserData(HapiTransactionService.XACT_USERDATA_KEY_RESOLVED_TAG_DEFINITIONS, () -> new HashMap<>()); retVal = resolvedTagDefinitions.get(key); if (retVal == null) { - // actual DB hit(s) happen here - retVal = getOrCreateTag(theTagType, theScheme, theTerm, theLabel); + CriteriaBuilder builder = myEntityManager.getCriteriaBuilder(); + CriteriaQuery cq = builder.createQuery(TagDefinition.class); + Root from = cq.from(TagDefinition.class); + + if (isNotBlank(theScheme)) { + cq.where( + builder.and( + builder.equal(from.get("myTagType"), theTagType), + builder.equal(from.get("mySystem"), theScheme), + builder.equal(from.get("myCode"), theTerm))); + } else { + cq.where( + builder.and( + builder.equal(from.get("myTagType"), theTagType), + builder.isNull(from.get("mySystem")), + builder.equal(from.get("myCode"), theTerm))); + } + + TypedQuery q = myEntityManager.createQuery(cq); + try { + retVal = q.getSingleResult(); + } catch (NoResultException e) { + retVal = new TagDefinition(theTagType, theScheme, theTerm, theLabel); + myEntityManager.persist(retVal); + } TransactionSynchronization sync = new AddTagDefinitionToCacheAfterCommitSynchronization(key, retVal); TransactionSynchronizationManager.registerSynchronization(sync); @@ -431,110 +443,6 @@ protected TagDefinition getTagOrNull(TransactionDetails theTransactionDetails, T return retVal; } - /** - * Gets the tag defined by the fed in values, or saves it if it does not - * exist. - * - * Can also throw an InternalErrorException if something bad happens. - */ - private TagDefinition getOrCreateTag(TagTypeEnum theTagType, String theScheme, String theTerm, String theLabel) { - CriteriaBuilder builder = myEntityManager.getCriteriaBuilder(); - CriteriaQuery cq = builder.createQuery(TagDefinition.class); - Root from = cq.from(TagDefinition.class); - - if (isNotBlank(theScheme)) { - cq.where( - builder.and( - builder.equal(from.get("myTagType"), theTagType), - builder.equal(from.get("mySystem"), theScheme), - builder.equal(from.get("myCode"), theTerm))); - } else { - cq.where( - builder.and( - builder.equal(from.get("myTagType"), theTagType), - builder.isNull(from.get("mySystem")), - builder.equal(from.get("myCode"), theTerm))); - } - - TypedQuery q = myEntityManager.createQuery(cq); - - TransactionTemplate template = new TransactionTemplate(myTransactionManager); - template.setPropagationBehavior(TransactionDefinition.PROPAGATION_REQUIRES_NEW); - - // this transaction will attempt to get or create the tag, - // repeating (on any failure) 10 times. - // if it fails more than this, we will throw exceptions - TagDefinition retVal; - int count = 0; - HashSet throwables = new HashSet<>(); - do { - try { - retVal = template.execute(new TransactionCallback() { - - // do the actual DB call(s) to read and/or write the values - private TagDefinition readOrCreate() { - TagDefinition val; - try { - val = q.getSingleResult(); - } catch (NoResultException e) { - val = new TagDefinition(theTagType, theScheme, theTerm, theLabel); - myEntityManager.persist(val); - } - return val; - } - - @Override - public TagDefinition doInTransaction(TransactionStatus status) { - TagDefinition tag = null; - - try { - tag = readOrCreate(); - } catch (Exception ex) { - // log any exceptions - just in case - // they may be signs of things to come... - ourLog.warn( - "Tag read/write failed: " - + ex.getMessage() + ". " - + "This is not a failure on its own, " - + "but could be useful information in the result of an actual failure." - ); - throwables.add(ex); - } - - return tag; - } - }); - } catch (Exception ex) { - // transaction template can fail if connections to db are exhausted - // and/or timeout - ourLog.warn("Transaction failed with: " - + ex.getMessage() + ". " - + "Transaction will rollback and be reattempted." - ); - retVal = null; - } - count++; - } while (retVal == null && count < TOTAL_TAG_READ_ATTEMPTS); - - if (retVal == null) { - // if tag is still null, - // something bad must be happening - // - throw - String msg = throwables.stream() - .map(Throwable::getMessage) - .collect(Collectors.joining(", ")); - throw new InternalErrorException( - Msg.code(2023) - + "Tag get/create failed after " - + TOTAL_TAG_READ_ATTEMPTS - + " attempts with error(s): " - + msg - ); - } - - return retVal; - } - protected IBundleProvider history(RequestDetails theRequest, String theResourceType, Long theResourcePid, Date theRangeStartInclusive, Date theRangeEndInclusive, Integer theOffset) { String resourceName = defaultIfBlank(theResourceType, null); @@ -943,7 +851,7 @@ protected void postUpdate(ResourceTable theEntity, T theResource) { @CoverageIgnore public BaseHasResource readEntity(IIdType theValueId, RequestDetails theRequest) { - throw new NotImplementedException(Msg.code(927) + ""); + throw new NotImplementedException(""); } /** @@ -1151,7 +1059,7 @@ public R toResource(Class theResourceType, IBaseRes b.append(e.getMessage()); String msg = b.toString(); ourLog.error(msg, e); - throw new DataFormatException(Msg.code(928) + msg, e); + throw new DataFormatException(msg, e); } } else { @@ -1230,7 +1138,7 @@ private void verifyMatchUrlForConditionalCreate(IBaseResource theResource, Strin // Make sure that the match URL was actually appropriate for the supplied resource InMemoryMatchResult outcome = myInMemoryResourceMatcher.match(theIfNoneExist, theResource, theParams); if (outcome.supported() && !outcome.matched()) { - throw new InvalidRequestException(Msg.code(929) + "Failed to process conditional create. The supplied resource did not satisfy the conditional URL."); + throw new InvalidRequestException("Failed to process conditional create. The supplied resource did not satisfy the conditional URL."); } } @@ -1516,7 +1424,8 @@ private void createHistoryEntry(RequestDetails theRequest, IBaseResource theReso private void validateIncomingResourceTypeMatchesExisting(IBaseResource theResource, ResourceTable entity) { String resourceType = myContext.getResourceType(theResource); if (!resourceType.equals(entity.getResourceType())) { - throw new UnprocessableEntityException(Msg.code(930) + "Existing resource ID[" + entity.getIdDt().toUnqualifiedVersionless() + "] is of type[" + entity.getResourceType() + "] - Cannot update with [" + resourceType + "]"); + throw new UnprocessableEntityException( + "Existing resource ID[" + entity.getIdDt().toUnqualifiedVersionless() + "] is of type[" + entity.getResourceType() + "] - Cannot update with [" + resourceType + "]"); } } @@ -1658,7 +1567,8 @@ private void validateChildReferenceTargetTypes(IBase theElement, String thePath) if (!isLogicalReference(referencedId)) { if (!referencedId.getValue().contains("?")) { if (!validTypes.contains(referencedId.getResourceType())) { - throw new UnprocessableEntityException(Msg.code(931) + "Invalid reference found at path '" + newPath + "'. Resource type '" + referencedId.getResourceType() + "' is not valid for this path"); + throw new UnprocessableEntityException( + "Invalid reference found at path '" + newPath + "'. Resource type '" + referencedId.getResourceType() + "' is not valid for this path"); } } } @@ -1673,7 +1583,7 @@ private void validateChildReferenceTargetTypes(IBase theElement, String thePath) protected void validateMetaCount(int theMetaCount) { if (myConfig.getResourceMetaCountHardLimit() != null) { if (theMetaCount > myConfig.getResourceMetaCountHardLimit()) { - throw new UnprocessableEntityException(Msg.code(932) + "Resource contains " + theMetaCount + " meta entries (tag/profile/security label), maximum is " + myConfig.getResourceMetaCountHardLimit()); + throw new UnprocessableEntityException("Resource contains " + theMetaCount + " meta entries (tag/profile/security label), maximum is " + myConfig.getResourceMetaCountHardLimit()); } } } @@ -1710,7 +1620,7 @@ protected void validateResourceForStorage(T theResource, ResourceTable theEntity } if (tag != null) { - throw new UnprocessableEntityException(Msg.code(933) + "Resource contains the 'subsetted' tag, and must not be stored as it may contain a subset of available data"); + throw new UnprocessableEntityException("Resource contains the 'subsetted' tag, and must not be stored as it may contain a subset of available data"); } if (getConfig().isEnforceReferenceTargetTypes()) { @@ -1834,7 +1744,7 @@ private static String parseNarrativeTextIntoWords(IBaseResource theResource) { } } } catch (Exception e) { - throw new DataFormatException(Msg.code(934) + "Unable to convert DIV to string", e); + throw new DataFormatException("Unable to convert DIV to string", e); } } @@ -1864,7 +1774,8 @@ private static List toBaseCodingList(List theSecurity public static void validateResourceType(BaseHasResource theEntity, String theResourceName) { if (!theResourceName.equals(theEntity.getResourceType())) { - throw new ResourceNotFoundException(Msg.code(935) + "Resource with ID " + theEntity.getIdDt().getIdPart() + " exists but it is not of type " + theResourceName + ", found resource of type " + theEntity.getResourceType()); + throw new ResourceNotFoundException( + "Resource with ID " + theEntity.getIdDt().getIdPart() + " exists but it is not of type " + theResourceName + ", found resource of type " + theEntity.getResourceType()); } }