Improve docs for NO-OP update pointcuts (#5294)

This commit is contained in:
James Agnew 2023-09-12 16:40:48 -04:00 committed by GitHub
parent fe8c6c066e
commit d4717a08d6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 105 additions and 0 deletions

View File

@ -1484,6 +1484,15 @@ public enum Pointcut implements IPointcut {
* to the new contents of the resource. These changes will be reflected in
* permanent storage.
* </p>
* <p>
* <b>NO-OPS:</b> If the client has submitted an update that does not actually make any changes
* (i.e. the resource they include in the PUT body is identical to the content that
* was already stored) the server may choose to ignore the update and perform
* a "NO-OP". In this case, this pointcut is still invoked, but {@link #STORAGE_PRECOMMIT_RESOURCE_UPDATED}
* will not be. Hook methods for this pointcut may make changes to the new contents of the
* resource being updated, and in this case the NO-OP will be cancelled and
* {@link #STORAGE_PRECOMMIT_RESOURCE_UPDATED} will also be invoked.
* </p>
* Hooks may accept the following parameters:
* <ul>
* <li>org.hl7.fhir.instance.model.api.IBaseResource - The previous contents of the resource being updated</li>
@ -1617,6 +1626,10 @@ public enum Pointcut implements IPointcut {
* changes as storage has already occurred. Changes will not be reflected
* in storage, but may be reflected in the HTTP response.
* </p>
* <p>
* NO-OP note: See {@link #STORAGE_PRESTORAGE_RESOURCE_UPDATED} for a note on
* no-op updates when no changes are detected.
* </p>
* Hooks may accept the following parameters:
* <ul>
* <li>org.hl7.fhir.instance.model.api.IBaseResource - The previous contents of the resource</li>

View File

@ -1,6 +1,10 @@
package ca.uhn.fhir.jpa.dao.r4;
import ca.uhn.fhir.i18n.Msg;
import ca.uhn.fhir.interceptor.api.HookParams;
import ca.uhn.fhir.interceptor.api.IAnonymousInterceptor;
import ca.uhn.fhir.interceptor.api.IPointcut;
import ca.uhn.fhir.interceptor.api.Pointcut;
import ca.uhn.fhir.jpa.api.config.JpaStorageSettings;
import ca.uhn.fhir.jpa.model.dao.JpaPid;
import ca.uhn.fhir.jpa.model.entity.ResourceHistoryTable;
@ -16,11 +20,13 @@ import ca.uhn.fhir.rest.api.MethodOutcome;
import ca.uhn.fhir.rest.api.server.IBundleProvider;
import ca.uhn.fhir.rest.api.server.SystemRequestDetails;
import ca.uhn.fhir.rest.param.StringParam;
import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import ca.uhn.fhir.rest.server.exceptions.ResourceGoneException;
import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException;
import ca.uhn.fhir.rest.server.exceptions.ResourceVersionConflictException;
import ca.uhn.fhir.rest.server.exceptions.UnprocessableEntityException;
import ca.uhn.fhir.util.BundleBuilder;
import org.hl7.fhir.instance.model.api.IBaseResource;
import org.hl7.fhir.instance.model.api.IIdType;
import org.hl7.fhir.r4.model.CanonicalType;
@ -37,6 +43,7 @@ import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import javax.persistence.Id;
import java.util.ArrayList;
import java.util.Date;
import java.util.HashSet;
@ -71,6 +78,7 @@ public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test {
myStorageSettings.setResourceServerIdStrategy(new JpaStorageSettings().getResourceServerIdStrategy());
myStorageSettings.setResourceClientIdStrategy(new JpaStorageSettings().getResourceClientIdStrategy());
myStorageSettings.setResourceServerIdStrategy(JpaStorageSettings.IdStrategyEnum.SEQUENTIAL_NUMERIC);
myInterceptorRegistry.unregisterAllAnonymousInterceptors();
}
@ -1172,4 +1180,88 @@ public class FhirResourceDaoR4UpdateTest extends BaseJpaR4Test {
}
}
@Test
public void testUpdateNoChange_ChangeForcedInPreStorageInterceptor() {
// Add interceptor which forces a change
myInterceptorRegistry.registerAnonymousInterceptor(Pointcut.STORAGE_PRESTORAGE_RESOURCE_UPDATED, new IAnonymousInterceptor() {
@Override
public void invoke(IPointcut thePointcut, HookParams theArgs) {
Patient newResource = (Patient) theArgs.get(IBaseResource.class, 1);
newResource.addIdentifier().setValue("foo");
}
});
Patient p = new Patient();
p.setId("Patient/A");
p.setActive(true);
assertEquals("1", myPatientDao.update(p, mySrd).getId().getVersionIdPart());
p = new Patient();
p.setId("Patient/A");
p.setActive(true);
assertEquals("2", myPatientDao.update(p, mySrd).getId().getVersionIdPart());
p = myPatientDao.read(new IdType("Patient/A"), mySrd);
assertTrue(p.getActive());
assertEquals("foo", p.getIdentifierFirstRep().getValue());
}
/**
* The precommit interceptor should not actually be invoked since this is a NO-OP
* (only prestorage is)
*/
@Test
public void testUpdateNoChange_ChangeForcedInPreCommitInterceptor() {
// Add interceptor which forces a change
myInterceptorRegistry.registerAnonymousInterceptor(Pointcut.STORAGE_PRECOMMIT_RESOURCE_UPDATED, new IAnonymousInterceptor() {
@Override
public void invoke(IPointcut thePointcut, HookParams theArgs) {
throw new InternalErrorException("failed intentionally");
}
});
Patient p = new Patient();
p.setId("Patient/A");
p.setActive(true);
assertEquals("1", myPatientDao.update(p, mySrd).getId().getVersionIdPart());
p = new Patient();
p.setId("Patient/A");
p.setActive(true);
assertEquals("1", myPatientDao.update(p, mySrd).getId().getVersionIdPart());
p = myPatientDao.read(new IdType("Patient/A"), mySrd);
assertTrue(p.getActive());
assertEquals(0, p.getIdentifier().size());
}
@Test
public void testUpdateNoChange_ChangeForcedInPreStorageInterceptor_InTransaction() {
// Add interceptor which forces a change
myInterceptorRegistry.registerAnonymousInterceptor(Pointcut.STORAGE_PRESTORAGE_RESOURCE_UPDATED, new IAnonymousInterceptor() {
@Override
public void invoke(IPointcut thePointcut, HookParams theArgs) {
Patient newResource = (Patient) theArgs.get(IBaseResource.class, 1);
newResource.addIdentifier().setValue("foo");
}
});
Patient p = new Patient();
p.setId("Patient/A");
p.setActive(true);
BundleBuilder bb = new BundleBuilder(myFhirContext);
bb.addTransactionUpdateEntry(p);
assertThat(mySystemDao.transaction(mySrd, bb.getBundleTyped()).getEntryFirstRep().getResponse().getLocation(), endsWith("/_history/1"));
p = new Patient();
p.setId("Patient/A");
p.setActive(true);
bb = new BundleBuilder(myFhirContext);
bb.addTransactionUpdateEntry(p);
assertThat(mySystemDao.transaction(mySrd, bb.getBundleTyped()).getEntryFirstRep().getResponse().getLocation(), endsWith("/_history/2"));
p = myPatientDao.read(new IdType("Patient/A"), mySrd);
assertTrue(p.getActive());
assertEquals("foo", p.getIdentifierFirstRep().getValue());
}
}