honeymoose
/
hapi-fhir
mirror of https://github.com/hapifhir/hapi-fhir.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix expunge transaction boundaries

This commit is contained in:
James Agnew 2018-06-25 10:22:49 -04:00
parent e5cb609f4d
commit f0da7a33de
7 changed files with 217 additions and 160 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

158
hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirDao.java
View File

@ -58,7 +58,6 @@ import ca.uhn.fhir.util.*;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Charsets;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import com.google.common.hash.HashFunction;
import com.google.common.hash.Hashing;
@ -227,6 +226,7 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> implements IDao,
}
protected ExpungeOutcome doExpunge(String theResourceName, Long theResourceId, Long theVersion, ExpungeOptions theExpungeOptions) {
TransactionTemplate txTemplate = new TransactionTemplate(myPlatformTransactionManager);
if (!getConfig().isExpungeEnabled()) {
throw new MethodNotAllowedException("$expunge is not enabled on this server");
@ -245,32 +245,39 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> implements IDao,
/*
* Delete historical versions of deleted resources
*/
Pageable page = new PageRequest(0, remainingCount.get());
Slice<Long> resourceIds;
Pageable page = PageRequest.of(0, remainingCount.get());
Slice<Long> resourceIds = txTemplate.execute(t -> {
if (theResourceId != null) {
resourceIds = myResourceTableDao.findIdsOfDeletedResourcesOfType(page, theResourceId, theResourceName);
return myResourceTableDao.findIdsOfDeletedResourcesOfType(page, theResourceId, theResourceName);
} else {
if (theResourceName != null) {
resourceIds = myResourceTableDao.findIdsOfDeletedResourcesOfType(page, theResourceName);
return myResourceTableDao.findIdsOfDeletedResourcesOfType(page, theResourceName);
} else {
resourceIds = myResourceTableDao.findIdsOfDeletedResources(page);
return myResourceTableDao.findIdsOfDeletedResources(page);
}
}
});
for (Long next : resourceIds) {
txTemplate.execute(t -> {
expungeHistoricalVersionsOfId(next, remainingCount);
if (remainingCount.get() <= 0) {
return toExpungeOutcome(theExpungeOptions, remainingCount);
}
return null;
});
}
/*
* Delete current versions of deleted resources
*/
for (Long next : resourceIds) {
txTemplate.execute(t -> {
expungeCurrentVersionOfResource(next);
if (remainingCount.get() <= 0) {
return toExpungeOutcome(theExpungeOptions, remainingCount);
}
return null;
});
}
}
@ -280,22 +287,26 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> implements IDao,
/*
* Delete historical versions of non-deleted resources
*/
Pageable page = new PageRequest(0, remainingCount.get());
Slice<Long> historicalIds;
Pageable page = PageRequest.of(0, remainingCount.get());
Slice<Long> historicalIds = txTemplate.execute(t -> {
if (theResourceId != null && theVersion != null) {
historicalIds = toSlice(myResourceHistoryTableDao.findForIdAndVersion(theResourceId, theVersion));
return toSlice(myResourceHistoryTableDao.findForIdAndVersion(theResourceId, theVersion));
} else {
if (theResourceName != null) {
historicalIds = myResourceHistoryTableDao.findIdsOfPreviousVersionsOfResources(page, theResourceName);
return myResourceHistoryTableDao.findIdsOfPreviousVersionsOfResources(page, theResourceName);
} else {
historicalIds = myResourceHistoryTableDao.findIdsOfPreviousVersionsOfResources(page);
return myResourceHistoryTableDao.findIdsOfPreviousVersionsOfResources(page);
}
}
});
for (Long next : historicalIds) {
txTemplate.execute(t -> {
expungeHistoricalVersion(next);
if (remainingCount.decrementAndGet() <= 0) {
return toExpungeOutcome(theExpungeOptions, remainingCount);
}
return null;
});
}
}
@ -704,58 +715,6 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> implements IDao,
return retVal;
}
@SuppressWarnings("unchecked")
public <R extends IBaseResource> IFhirResourceDao<R> getDao(Class<R> theType) {
Map<Class<? extends IBaseResource>, IFhirResourceDao<?>> resourceTypeToDao = getDaos();
IFhirResourceDao<R> dao = (IFhirResourceDao<R>) resourceTypeToDao.get(theType);
return dao;
}
protected IFhirResourceDao<?> getDaoOrThrowException(Class<? extends IBaseResource> theClass) {
IFhirResourceDao<? extends IBaseResource> retVal = getDao(theClass);
if (retVal == null) {
List<String> supportedResourceTypes = getDaos()
.keySet()
.stream()
.map(t->myContext.getResourceDefinition(t).getName())
.sorted()
.collect(Collectors.toList());
throw new InvalidRequestException("Unable to process request, this server does not know how to handle resources of type " + getContext().getResourceDefinition(theClass).getName() + " - Can handle: " + supportedResourceTypes);
}
return retVal;
}
private Map<Class<? extends IBaseResource>, IFhirResourceDao<?>> getDaos() {
if (myResourceTypeToDao == null) {
Map<Class<? extends IBaseResource>, IFhirResourceDao<?>> resourceTypeToDao = new HashMap<>();
Map<String, IFhirResourceDao> daos = myApplicationContext.getBeansOfType(IFhirResourceDao.class, false, false);
String[] beanNames = myApplicationContext.getBeanNamesForType(IFhirResourceDao.class);
for (IFhirResourceDao<?> next : daos.values()) {
resourceTypeToDao.put(next.getResourceType(), next);
}
if (this instanceof IFhirResourceDao<?>) {
IFhirResourceDao<?> thiz = (IFhirResourceDao<?>) this;
resourceTypeToDao.put(thiz.getResourceType(), thiz);
}
myResourceTypeToDao = resourceTypeToDao;
}
return Collections.unmodifiableMap(myResourceTypeToDao);
}
@PostConstruct
public void startClearCaches() {
myResourceTypeToDao = null;
}
protected Set<ResourceIndexedSearchParamCoords> extractSearchParamCoords(ResourceTable theEntity, IBaseResource theResource) {
return mySearchParamExtractor.extractSearchParamCoords(theEntity, theResource);
}
@ -958,18 +917,6 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> implements IDao,
return myConfig;
}
@Override
public void setApplicationContext(ApplicationContext theApplicationContext) throws BeansException {
/*
* We do a null check here because Smile's module system tries to
* initialize the application context twice if two modules depend on
* the persistence module. The second time sets the dependency's appctx.
*/
if (myApplicationContext == null) {
myApplicationContext = theApplicationContext;
}
}
public void setConfig(DaoConfig theConfig) {
myConfig = theConfig;
}
@ -996,6 +943,50 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> implements IDao,
}
}
@SuppressWarnings("unchecked")
public <R extends IBaseResource> IFhirResourceDao<R> getDao(Class<R> theType) {
Map<Class<? extends IBaseResource>, IFhirResourceDao<?>> resourceTypeToDao = getDaos();
IFhirResourceDao<R> dao = (IFhirResourceDao<R>) resourceTypeToDao.get(theType);
return dao;
}
protected IFhirResourceDao<?> getDaoOrThrowException(Class<? extends IBaseResource> theClass) {
IFhirResourceDao<? extends IBaseResource> retVal = getDao(theClass);
if (retVal == null) {
List<String> supportedResourceTypes = getDaos()
.keySet()
.stream()
.map(t -> myContext.getResourceDefinition(t).getName())
.sorted()
.collect(Collectors.toList());
throw new InvalidRequestException("Unable to process request, this server does not know how to handle resources of type " + getContext().getResourceDefinition(theClass).getName() + " - Can handle: " + supportedResourceTypes);
}
return retVal;
}
private Map<Class<? extends IBaseResource>, IFhirResourceDao<?>> getDaos() {
if (myResourceTypeToDao == null) {
Map<Class<? extends IBaseResource>, IFhirResourceDao<?>> resourceTypeToDao = new HashMap<>();
Map<String, IFhirResourceDao> daos = myApplicationContext.getBeansOfType(IFhirResourceDao.class, false, false);
String[] beanNames = myApplicationContext.getBeanNamesForType(IFhirResourceDao.class);
for (IFhirResourceDao<?> next : daos.values()) {
resourceTypeToDao.put(next.getResourceType(), next);
}
if (this instanceof IFhirResourceDao<?>) {
IFhirResourceDao<?> thiz = (IFhirResourceDao<?>) this;
resourceTypeToDao.put(thiz.getResourceType(), thiz);
}
myResourceTypeToDao = resourceTypeToDao;
}
return Collections.unmodifiableMap(myResourceTypeToDao);
}
public IResourceIndexedCompositeStringUniqueDao getResourceIndexedCompositeStringUniqueDao() {
return myResourceIndexedCompositeStringUniqueDao;
}
@ -1537,6 +1528,18 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> implements IDao,
return retVal;
}
@Override
public void setApplicationContext(ApplicationContext theApplicationContext) throws BeansException {
/*
* We do a null check here because Smile's module system tries to
* initialize the application context twice if two modules depend on
* the persistence module. The second time sets the dependency's appctx.
*/
if (myApplicationContext == null) {
myApplicationContext = theApplicationContext;
}
}
private void setUpdatedTime(Collection<? extends BaseResourceIndexedSearchParam> theParams, Date theUpdateTime) {
for (BaseResourceIndexedSearchParam nextSearchParam : theParams) {
nextSearchParam.setUpdated(theUpdateTime);
@ -1593,6 +1596,11 @@ public abstract class BaseHapiFhirDao<T extends IBaseResource> implements IDao,
return false;
}
@PostConstruct
public void startClearCaches() {
myResourceTypeToDao = null;
}
private ExpungeOutcome toExpungeOutcome(ExpungeOptions theExpungeOptions, AtomicInteger theRemainingCount) {
return new ExpungeOutcome()
.setDeletedCount(theExpungeOptions.getLimit() - theRemainingCount.get());

13
hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/BaseHapiFhirResourceDao.java
View File

@ -50,6 +50,7 @@ import ca.uhn.fhir.rest.server.method.SearchMethodBinding;
import ca.uhn.fhir.util.*;
import org.apache.commons.lang3.Validate;
import org.hl7.fhir.instance.model.api.*;
import org.hl7.fhir.r4.model.InstantType;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Required;
import org.springframework.lang.NonNull;
@ -517,6 +518,7 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
}
@Override
@Transactional(propagation = Propagation.NEVER)
public ExpungeOutcome expunge(IIdType theId, ExpungeOptions theExpungeOptions) {
BaseHasResource entity = readEntity(theId);
if (theId.hasVersionIdPart()) {
@ -532,6 +534,7 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
}
@Override
@Transactional(propagation = Propagation.NEVER)
public ExpungeOutcome expunge(ExpungeOptions theExpungeOptions) {
ourLog.info("Beginning TYPE[{}] expunge operation", getResourceName());
@ -856,14 +859,8 @@ public abstract class BaseHapiFhirResourceDao<T extends IBaseResource> extends B
T retVal = toResource(myResourceType, entity, null, null, false);
IPrimitiveType<Date> deleted;
if (retVal instanceof IResource) {
deleted = ResourceMetadataKeyEnum.DELETED_AT.get((IResource) retVal);
} else {
deleted = ResourceMetadataKeyEnum.DELETED_AT.get((IAnyResource) retVal);
}
if (deleted != null && !deleted.isEmpty()) {
throw new ResourceGoneException("Resource was deleted at " + deleted.getValueAsString());
if (entity.getDeleted() != null) {
throw new ResourceGoneException("Resource was deleted at " + new InstantType(entity.getDeleted()).getValueAsString());
}
ourLog.debug("Processed read on {} in {}ms", theId.getValue(), w.getMillisAndRestart());

50
hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/entity/ForcedId.java
View File

@ -20,26 +20,16 @@ package ca.uhn.fhir.jpa.entity;
* #L%
*/
import javax.persistence.Column;
import javax.persistence.Entity;
import javax.persistence.ForeignKey;
import javax.persistence.GeneratedValue;
import javax.persistence.GenerationType;
import javax.persistence.Id;
import javax.persistence.Index;
import javax.persistence.JoinColumn;
import javax.persistence.OneToOne;
import javax.persistence.SequenceGenerator;
import javax.persistence.Table;
import javax.persistence.UniqueConstraint;
import org.hibernate.annotations.ColumnDefault;
import javax.persistence.*;
//@formatter:off
@Entity()
@Table(name = "HFJ_FORCED_ID", uniqueConstraints = {
@UniqueConstraint(name = "IDX_FORCEDID_RESID", columnNames = {"RESOURCE_PID"}),
@UniqueConstraint(name = "IDX_FORCEDID_TYPE_RESID", columnNames = {"RESOURCE_TYPE", "RESOURCE_PID"})
@UniqueConstraint(name = "IDX_FORCEDID_TYPE_RESID", columnNames = {"RESOURCE_TYPE", "RESOURCE_PID"}),
@UniqueConstraint(name = "IDX_FORCEDID_TYPE_FID", columnNames = {"RESOURCE_TYPE", "FORCED_ID"})
}, indexes = {
@Index(name = "IDX_FORCEDID_TYPE_FORCEDID", columnList = "RESOURCE_TYPE,FORCED_ID"),
})
@ -81,10 +71,18 @@ public class ForcedId {
return myForcedId;
}
public void setForcedId(String theForcedId) {
myForcedId = theForcedId;
}
public ResourceTable getResource() {
return myResource;
}
public void setResource(ResourceTable theResource) {
myResource = theResource;
}
public Long getResourcePid() {
if (myResourcePid == null) {
return myResource.getId();
@ -92,28 +90,20 @@ public class ForcedId {
return myResourcePid;
}
public String getResourceType() {
return myResourceType;
}
public void setForcedId(String theForcedId) {
myForcedId = theForcedId;
}
public void setResource(ResourceTable theResource) {
myResource = theResource;
}
public void setResourcePid(Long theResourcePid) {
myResourcePid = theResourcePid;
}
public void setResourcePid(ResourceTable theResourcePid) {
myResource = theResourcePid;
}
public String getResourceType() {
return myResourceType;
}
public void setResourceType(String theResourceType) {
myResourceType = theResourceType;
}
public void setResourcePid(Long theResourcePid) {
myResourcePid = theResourcePid;
}
}

20
hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/dao/r4/BaseJpaR4Test.java
View File

@ -170,6 +170,12 @@ public abstract class BaseJpaR4Test extends BaseJpaTest {
@Qualifier("myPatientDaoR4")
protected IFhirResourceDaoPatient<Patient> myPatientDao;
@Autowired
protected IResourceTableDao myResourceTableDao;
@Autowired
protected IResourceHistoryTableDao myResourceHistoryTableDao;
@Autowired
protected IForcedIdDao myForcedIdDao;
@Autowired
@Qualifier("myCoverageDaoR4")
protected IFhirResourceDao<Coverage> myCoverageDao;
@Autowired
@ -188,10 +194,6 @@ public abstract class BaseJpaR4Test extends BaseJpaTest {
@Qualifier("myResourceProvidersR4")
protected Object myResourceProviders;
@Autowired
protected IResourceTableDao myResourceTableDao;
@Autowired
protected IResourceHistoryTableDao myResourceHistoryTableDao;
@Autowired
protected IResourceTagDao myResourceTagDao;
@Autowired
protected ISearchCoordinatorSvc mySearchCoordinatorSvc;
@ -315,6 +317,11 @@ public abstract class BaseJpaR4Test extends BaseJpaTest {
return myFhirCtx;
}
@Override
protected PlatformTransactionManager getTxManager() {
return myTxManager;
}
protected <T extends IBaseResource> T loadResourceFromClasspath(Class<T> type, String resourceName) throws IOException {
InputStream stream = FhirResourceDaoDstu2SearchNoFtTest.class.getResourceAsStream(resourceName);
if (stream == null) {
@ -325,11 +332,6 @@ public abstract class BaseJpaR4Test extends BaseJpaTest {
return newJsonParser.parseResource(type, string);
}
@Override
protected PlatformTransactionManager getTxManager() {
return myTxManager;
}
@AfterClass
public static void afterClassClearContextBaseJpaR4Test() throws Exception {
ourValueSetDao.purgeCaches();

55
hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/provider/r4/ExpungeR4Test.java
View File

@ -2,19 +2,27 @@ package ca.uhn.fhir.jpa.provider.r4;
import ca.uhn.fhir.jpa.dao.DaoConfig;
import ca.uhn.fhir.jpa.dao.IFhirResourceDao;
import ca.uhn.fhir.jpa.dao.data.IForcedIdDao;
import ca.uhn.fhir.jpa.dao.data.IResourceHistoryTableDao;
import ca.uhn.fhir.jpa.dao.data.IResourceTableDao;
import ca.uhn.fhir.jpa.util.ExpungeOptions;
import ca.uhn.fhir.rest.server.exceptions.PreconditionFailedException;
import ca.uhn.fhir.rest.server.exceptions.ResourceGoneException;
import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException;
import ca.uhn.fhir.util.TestUtil;
import org.hamcrest.Matchers;
import org.hl7.fhir.instance.model.api.IIdType;
import org.hl7.fhir.r4.model.IdType;
import org.hl7.fhir.r4.model.Observation;
import org.hl7.fhir.r4.model.Patient;
import org.junit.After;
import org.junit.AfterClass;
import org.junit.Before;
import org.junit.Test;
import org.springframework.beans.factory.annotation.Autowired;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.not;
import static org.junit.Assert.*;
public class ExpungeR4Test extends BaseResourceProviderR4Test {
@ -58,11 +66,8 @@ public class ExpungeR4Test extends BaseResourceProviderR4Test {
getDao(theId).read(theId);
}
@Override
@Before
public void before() throws Exception {
super.before();
public void createStandardPatients() {
Patient p = new Patient();
p.setId("PT-ONEVERSION");
p.getMeta().addTag().setSystem("http://foo").setCode("bar");
@ -105,7 +110,6 @@ public class ExpungeR4Test extends BaseResourceProviderR4Test {
o.setStatus(Observation.ObservationStatus.FINAL);
myDeletedObservationId = myObservationDao.create(o).getId();
myDeletedObservationId = myObservationDao.delete(myDeletedObservationId).getId();
}
private IFhirResourceDao<?> getDao(IIdType theId) {
@ -126,6 +130,8 @@ public class ExpungeR4Test extends BaseResourceProviderR4Test {
@Test
public void testExpungeInstanceOldVersionsAndDeleted() {
createStandardPatients();
Patient p = new Patient();
p.setId("PT-TWOVERSION");
p.getMeta().addTag().setSystem("http://foo").setCode("bar");
@ -151,8 +157,35 @@ public class ExpungeR4Test extends BaseResourceProviderR4Test {
assertGone(myDeletedObservationId);
}
@Test
public void testExpungeAllVersionsDeletesRow() {
// Create then delete
Patient p = new Patient();
p.setId("TEST");
p.getMeta().addTag().setSystem("http://foo").setCode("bar");
p.setActive(true);
p.addName().setFamily("FOO");
myPatientDao.update(p).getId();
myPatientDao.delete(new IdType("Patient/TEST"));
runInTransaction(()-> assertThat(myResourceTableDao.findAll(), not(empty())));
runInTransaction(()-> assertThat(myResourceHistoryTableDao.findAll(), not(empty())));
runInTransaction(()-> assertThat(myForcedIdDao.findAll(), not(empty())));
myPatientDao.expunge(new ExpungeOptions()
.setExpungeDeletedResources(true)
.setExpungeOldVersions(true));
runInTransaction(()-> assertThat(myResourceTableDao.findAll(), empty()));
runInTransaction(()-> assertThat(myResourceHistoryTableDao.findAll(), empty()));
runInTransaction(()-> assertThat(myForcedIdDao.findAll(), empty()));
}
@Test
public void testExpungeInstanceVersionCurrentVersion() {
createStandardPatients();
try {
myPatientDao.expunge(myTwoVersionPatientId.withVersion("2"), new ExpungeOptions()
@ -166,6 +199,8 @@ public class ExpungeR4Test extends BaseResourceProviderR4Test {
@Test
public void testExpungeInstanceVersionOldVersionsAndDeleted() {
createStandardPatients();
Patient p = new Patient();
p.setId("PT-TWOVERSION");
p.getMeta().addTag().setSystem("http://foo").setCode("bar");
@ -193,6 +228,8 @@ public class ExpungeR4Test extends BaseResourceProviderR4Test {
@Test
public void testExpungeSystemOldVersionsAndDeleted() {
createStandardPatients();
mySystemDao.expunge(new ExpungeOptions()
.setExpungeDeletedResources(true)
.setExpungeOldVersions(true));
@ -212,6 +249,8 @@ public class ExpungeR4Test extends BaseResourceProviderR4Test {
@Test
public void testExpungeTypeDeletedResources() {
createStandardPatients();
myPatientDao.expunge(new ExpungeOptions()
.setExpungeDeletedResources(true)
.setExpungeOldVersions(false));
@ -231,6 +270,8 @@ public class ExpungeR4Test extends BaseResourceProviderR4Test {
@Test
public void testExpungeTypeOldVersions() {
createStandardPatients();
myPatientDao.expunge(new ExpungeOptions()
.setExpungeDeletedResources(false)
.setExpungeOldVersions(true));
@ -251,6 +292,8 @@ public class ExpungeR4Test extends BaseResourceProviderR4Test {
@Test
public void testExpungeSystemEverything() {
createStandardPatients();
mySystemDao.expunge(new ExpungeOptions()
.setExpungeEverything(true));
@ -270,6 +313,8 @@ public class ExpungeR4Test extends BaseResourceProviderR4Test {
@Test
public void testExpungeTypeOldVersionsAndDeleted() {
createStandardPatients();
myPatientDao.expunge(new ExpungeOptions()
.setExpungeDeletedResources(true)
.setExpungeOldVersions(true));

2
hapi-fhir-jpaserver-uhnfhirtest/derby_maintenance.txt
View File

@ -144,3 +144,5 @@ drop index IDX_SP_TOKEN_UNQUAL;
16919 | public | hfj_spidx_token | 5.5205e+07 | 24763842560 | 17233928192 | 8192 | 7529906176 | 23 GB | 16 GB | 8192 bytes | 7181 MB
drop index IDX_FORCEDID_TYPE_FORCEDID;
create index IDX_FORCEDID_TYPE_FID;

13
src/changes/changes.xml
View File

@ -95,6 +95,19 @@
performance improvement on servers that are validating or executing FHIRPath repeatedly
under load. This module is used by default in the JPA server.
</action>
<action type="fix">
An index in the JPA server on the HFJ_FORCED_ID table was incorrectly
not marked as unique. This meant that under heavy load it was possible to
create two resources with the same client-assigned ID.
</action>
<action type="fix">
The JPA server
<![CDATA[<code>$expunge</code>]]>
operation deleted components of an individual resource record in
separate database transactions, meaning that if an operation failed
unexpectedly resources could be left in a weird state. This has been
corrected.
</action>
</release>
<release version="3.4.0" date="2018-05-28">
<action type="add">