From 09b1f547d0839ba39494691142a6000573b03082 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Tue, 24 Apr 2018 15:11:00 -0400 Subject: [PATCH] Add expunge --- .../main/java/ca/uhn/fhir/util/StopWatch.java | 131 ++++++++++++++---- .../java/ca/uhn/fhir/util/StopWatchTest.java | 1 + .../jpa/dao/dstu3/FhirSystemDaoDstu3.java | 37 ++++- .../provider/JpaResourceProviderDstu2.java | 4 +- .../jpa/provider/JpaSystemProviderDstu2.java | 4 +- .../dstu3/JpaResourceProviderDstu3.java | 4 +- .../dstu3/JpaSystemProviderDstu3.java | 4 +- .../provider/r4/JpaResourceProviderR4.java | 4 +- .../jpa/provider/r4/JpaSystemProviderR4.java | 4 +- .../ca/uhn/fhir/jpa/util/JpaConstants.java | 2 +- .../r4/ResourceProviderExpungeR4Test.java | 6 +- .../server/interceptor/auth/BaseRule.java | 2 +- .../auth/IAuthRuleBuilderOperationNamed.java | 9 +- .../interceptor/auth/OperationRule.java | 11 +- .../server/interceptor/auth/RuleBuilder.java | 16 ++- .../AuthorizationInterceptorR4Test.java | 113 +++++++++++++++ src/changes/changes.xml | 7 + 17 files changed, 298 insertions(+), 61 deletions(-) diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/StopWatch.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/StopWatch.java index 9ad4acc1908..6a3340474df 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/StopWatch.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/StopWatch.java @@ -6,7 +6,8 @@ import org.apache.commons.lang3.time.DateUtils; import java.text.DecimalFormat; import java.text.NumberFormat; import java.util.Date; -import java.util.LinkedHashMap; +import java.util.LinkedList; +import java.util.List; import java.util.concurrent.TimeUnit; import static org.apache.commons.lang3.StringUtils.isNotBlank; @@ -20,9 +21,9 @@ import static org.apache.commons.lang3.StringUtils.isNotBlank; * 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. @@ -48,9 +49,8 @@ public class StopWatch { private static final NumberFormat TEN_DAY_FORMAT = new DecimalFormat("0"); private static Long ourNowForUnitTest; private long myStarted = now(); - private long myCurrentTaskStarted = -1L; - private LinkedHashMap myTaskTotals; - private String myCurrentTaskName; + private TaskTiming myCurrentTask; + private LinkedList myTasks; /** * Constructor @@ -68,9 +68,9 @@ public class StopWatch { myStarted = theStart.getTime(); } - private void ensureTaskTotalsMapExists() { - if (myTaskTotals == null) { - myTaskTotals = new LinkedHashMap<>(); + private void addNewlineIfContentExists(StringBuilder theB) { + if (theB.length() > 0) { + theB.append("\n"); } } @@ -80,14 +80,17 @@ public class StopWatch { * is currently started so it's ok to call it more than once. */ public void endCurrentTask() { - if (isNotBlank(myCurrentTaskName)) { - ensureTaskTotalsMapExists(); - Long existingTotal = myTaskTotals.get(myCurrentTaskName); - long taskTimeElapsed = now() - myCurrentTaskStarted; - Long newTotal = existingTotal != null ? existingTotal + taskTimeElapsed : taskTimeElapsed; - myTaskTotals.put(myCurrentTaskName, newTotal); + ensureTasksListExists(); + if (myCurrentTask != null) { + myCurrentTask.setEnd(now()); + } + myCurrentTask = null; + } + + private void ensureTasksListExists() { + if (myTasks == null) { + myTasks = new LinkedList<>(); } - myCurrentTaskName = null; } /** @@ -95,23 +98,49 @@ public class StopWatch { */ public String formatTaskDurations() { - // Flush the current task if it's ongoing - String continueTask = myCurrentTaskName; - if (isNotBlank(myCurrentTaskName)) { - endCurrentTask(); - startTask(continueTask); + ensureTasksListExists(); + StringBuilder b = new StringBuilder(); + + if (myTasks.size() > 0) { + long delta = myTasks.getFirst().getStart() - myStarted; + if (delta > 10) { + addNewlineIfContentExists(b); + b.append("Before first task"); + b.append(": "); + b.append(formatMillis(delta)); + } } - ensureTaskTotalsMapExists(); - StringBuilder b = new StringBuilder(); - for (String nextTask : myTaskTotals.keySet()) { - if (b.length() > 0) { - b.append("\n"); + TaskTiming last = null; + for (TaskTiming nextTask : myTasks) { + + if (last != null) { + long delta = nextTask.getStart() - last.getEnd(); + if (delta > 10) { + addNewlineIfContentExists(b); + b.append("Between"); + b.append(": "); + b.append(formatMillis(delta)); + } } - b.append(nextTask); + addNewlineIfContentExists(b); + b.append(nextTask.getTaskName()); b.append(": "); - b.append(formatMillis(myTaskTotals.get(nextTask))); + long delta = nextTask.getMillis(); + b.append(formatMillis(delta)); + + last = nextTask; + } + + if (myTasks.size() > 0) { + long delta = now() - myTasks.getLast().getEnd(); + if (delta > 10) { + addNewlineIfContentExists(b); + b.append("After last task"); + b.append(": "); + b.append(formatMillis(delta)); + } } return b.toString(); @@ -214,9 +243,11 @@ public class StopWatch { public void startTask(String theTaskName) { endCurrentTask(); if (isNotBlank(theTaskName)) { - myCurrentTaskStarted = now(); + myCurrentTask = new TaskTiming() + .setTaskName(theTaskName) + .setStart(now()); + myTasks.add(myCurrentTask); } - myCurrentTaskName = theTaskName; } /** @@ -301,4 +332,44 @@ public class StopWatch { ourNowForUnitTest = theNowForUnitTest; } + private static class TaskTiming { + private long myStart; + private long myEnd; + private String myTaskName; + + public long getEnd() { + if (myEnd == 0) { + return now(); + } + return myEnd; + } + + public TaskTiming setEnd(long theEnd) { + myEnd = theEnd; + return this; + } + + public long getMillis() { + return getEnd() - getStart(); + } + + public long getStart() { + return myStart; + } + + public TaskTiming setStart(long theStart) { + myStart = theStart; + return this; + } + + public String getTaskName() { + return myTaskName; + } + + public TaskTiming setTaskName(String theTaskName) { + myTaskName = theTaskName; + return this; + } + } + } diff --git a/hapi-fhir-base/src/test/java/ca/uhn/fhir/util/StopWatchTest.java b/hapi-fhir-base/src/test/java/ca/uhn/fhir/util/StopWatchTest.java index 45a854094f5..299290c1c39 100644 --- a/hapi-fhir-base/src/test/java/ca/uhn/fhir/util/StopWatchTest.java +++ b/hapi-fhir-base/src/test/java/ca/uhn/fhir/util/StopWatchTest.java @@ -122,6 +122,7 @@ public class StopWatchTest { StopWatch.setNowForUnitTestForUnitTest(1600L); String taskDurations = sw.formatTaskDurations(); + ourLog.info(taskDurations); assertEquals("TASK1: 500ms\nTASK2: 100ms", taskDurations); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3.java index 3bbb31c17e2..d1c4020ca13 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/dao/dstu3/FhirSystemDaoDstu3.java @@ -159,6 +159,7 @@ public class FhirSystemDaoDstu3 extends BaseHapiFhirSystemDao { ourLog.debug("Beginning {} with {} resources", theActionName, theRequest.getEntry().size()); final Date updateTime = new Date(); + final StopWatch transactionStopWatch = new StopWatch(); final Set allIds = new LinkedHashSet<>(); final Map idSubstitutions = new HashMap<>(); @@ -221,9 +222,14 @@ public class FhirSystemDaoDstu3 extends BaseHapiFhirSystemDao { Map entriesToProcess = txManager.execute(new TransactionCallback>() { @Override public Map doInTransaction(TransactionStatus status) { - return doTransactionWriteOperations(theRequestDetails, theRequest, theActionName, updateTime, allIds, idSubstitutions, idToPersistedOutcome, response, originalRequestOrder, entries); + Map retVal = doTransactionWriteOperations(theRequestDetails, theActionName, updateTime, allIds, idSubstitutions, idToPersistedOutcome, response, originalRequestOrder, entries, transactionStopWatch); + + transactionStopWatch.startTask("Commit writes to database"); + return retVal; } }); + transactionStopWatch.endCurrentTask(); + for (Entry nextEntry : entriesToProcess.entrySet()) { String responseLocation = nextEntry.getValue().getIdDt().toUnqualified().getValue(); String responseEtag = nextEntry.getValue().getIdDt().getVersionIdPart(); @@ -234,6 +240,9 @@ public class FhirSystemDaoDstu3 extends BaseHapiFhirSystemDao { /* * Loop through the request and process any entries of type GET */ + if (getEntries.size() > 0) { + transactionStopWatch.startTask("Process " + getEntries.size() + " GET entries"); + } for (BundleEntryComponent nextReqEntry : getEntries) { Integer originalOrder = originalRequestOrder.get(nextReqEntry); BundleEntryComponent nextRespEntry = response.getEntry().get(originalOrder); @@ -247,7 +256,7 @@ public class FhirSystemDaoDstu3 extends BaseHapiFhirSystemDao { int qIndex = url.indexOf('?'); ArrayListMultimap paramValues = ArrayListMultimap.create(); - requestDetails.setParameters(new HashMap()); + requestDetails.setParameters(new HashMap<>()); if (qIndex != -1) { String params = url.substring(qIndex); List parameters = translateMatchUrl(params); @@ -297,13 +306,17 @@ public class FhirSystemDaoDstu3 extends BaseHapiFhirSystemDao { } } + transactionStopWatch.endCurrentTask(); response.setType(BundleType.TRANSACTIONRESPONSE); + + ourLog.info("Transaction timing:\n{}", transactionStopWatch.formatTaskDurations()); + return response; } - private Map doTransactionWriteOperations(ServletRequestDetails theRequestDetails, Bundle theRequest, String theActionName, Date theUpdateTime, Set theAllIds, - Map theIdSubstitutions, Map theIdToPersistedOutcome, Bundle theResponse, IdentityHashMap theOriginalRequestOrder, List theEntries) { + private Map doTransactionWriteOperations(ServletRequestDetails theRequestDetails, String theActionName, Date theUpdateTime, Set theAllIds, + Map theIdSubstitutions, Map theIdToPersistedOutcome, Bundle theResponse, IdentityHashMap theOriginalRequestOrder, List theEntries, StopWatch theTransactionStopWatch) { Set deletedResources = new HashSet<>(); List deleteConflicts = new ArrayList<>(); Map entriesToProcess = new IdentityHashMap<>(); @@ -360,10 +373,11 @@ public class FhirSystemDaoDstu3 extends BaseHapiFhirSystemDao { } HTTPVerb verb = nextReqEntry.getRequest().getMethodElement().getValue(); - String resourceType = res != null ? getContext().getResourceDefinition(res).getName() : null; BundleEntryComponent nextRespEntry = theResponse.getEntry().get(theOriginalRequestOrder.get(nextReqEntry)); + theTransactionStopWatch.startTask("Bundle.entry[" + i + "]: " + verb.name() + " " + defaultString(resourceType)); + switch (verb) { case POST: { // CREATE @@ -466,6 +480,8 @@ public class FhirSystemDaoDstu3 extends BaseHapiFhirSystemDao { break; } + + theTransactionStopWatch.endCurrentTask(); } /* @@ -484,6 +500,7 @@ public class FhirSystemDaoDstu3 extends BaseHapiFhirSystemDao { */ FhirTerser terser = getContext().newTerser(); + theTransactionStopWatch.startTask("Index " + theIdToPersistedOutcome.size() + " resources"); for (DaoMethodOutcome nextOutcome : theIdToPersistedOutcome.values()) { IBaseResource nextResource = nextOutcome.getResource(); if (nextResource == null) { @@ -534,8 +551,16 @@ public class FhirSystemDaoDstu3 extends BaseHapiFhirSystemDao { } } + theTransactionStopWatch.endCurrentTask(); + theTransactionStopWatch.startTask("Flush writes to database"); + flushJpaSession(); + theTransactionStopWatch.endCurrentTask(); + if (conditionalRequestUrls.size() > 0) { + theTransactionStopWatch.startTask("Check for conflicts in conditional resources"); + } + /* * Double check we didn't allow any duplicates we shouldn't have */ @@ -552,6 +577,8 @@ public class FhirSystemDaoDstu3 extends BaseHapiFhirSystemDao { } } + theTransactionStopWatch.endCurrentTask(); + for (IdType next : theAllIds) { IdType replacement = theIdSubstitutions.get(next); if (replacement == null) { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/JpaResourceProviderDstu2.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/JpaResourceProviderDstu2.java index 35b76ab3a6c..000f328426a 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/JpaResourceProviderDstu2.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/JpaResourceProviderDstu2.java @@ -85,7 +85,7 @@ public class JpaResourceProviderDstu2 extends BaseJpaResour @IdParam IIdType theIdParam, @OperationParam(name = JpaConstants.OPERATION_EXPUNGE_PARAM_LIMIT) org.hl7.fhir.r4.model.IntegerType theLimit, @OperationParam(name = JpaConstants.OPERATION_EXPUNGE_PARAM_EXPUNGE_DELETED_RESOURCES) org.hl7.fhir.r4.model.BooleanType theExpungeDeletedResources, - @OperationParam(name = JpaConstants.OPERATION_EXPUNGE_PARAM_EXPUNGE_OLD_VERSIONS) org.hl7.fhir.r4.model.BooleanType theExpungeOldVersions + @OperationParam(name = JpaConstants.OPERATION_EXPUNGE_PARAM_EXPUNGE_PREVIOUS_VERSIONS) org.hl7.fhir.r4.model.BooleanType theExpungeOldVersions ) { org.hl7.fhir.r4.model.Parameters retVal = super.doExpunge(theIdParam, theLimit, theExpungeDeletedResources, theExpungeOldVersions, null); return JpaSystemProviderDstu2.toExpungeResponse(retVal); @@ -97,7 +97,7 @@ public class JpaResourceProviderDstu2 extends BaseJpaResour public Parameters expunge( @OperationParam(name = JpaConstants.OPERATION_EXPUNGE_PARAM_LIMIT) org.hl7.fhir.r4.model.IntegerType theLimit, @OperationParam(name = JpaConstants.OPERATION_EXPUNGE_PARAM_EXPUNGE_DELETED_RESOURCES) org.hl7.fhir.r4.model.BooleanType theExpungeDeletedResources, - @OperationParam(name = JpaConstants.OPERATION_EXPUNGE_PARAM_EXPUNGE_OLD_VERSIONS) org.hl7.fhir.r4.model.BooleanType theExpungeOldVersions + @OperationParam(name = JpaConstants.OPERATION_EXPUNGE_PARAM_EXPUNGE_PREVIOUS_VERSIONS) org.hl7.fhir.r4.model.BooleanType theExpungeOldVersions ) { org.hl7.fhir.r4.model.Parameters retVal = super.doExpunge(null, theLimit, theExpungeDeletedResources, theExpungeOldVersions, null); return JpaSystemProviderDstu2.toExpungeResponse(retVal); diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/JpaSystemProviderDstu2.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/JpaSystemProviderDstu2.java index 922bd083691..4e09c196b85 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/JpaSystemProviderDstu2.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/JpaSystemProviderDstu2.java @@ -66,7 +66,7 @@ public class JpaSystemProviderDstu2 extends BaseJpaSystemProviderDstu2Plus extends BaseJpaRes @IdParam IIdType theIdParam, @OperationParam(name = JpaConstants.OPERATION_EXPUNGE_PARAM_LIMIT) org.hl7.fhir.r4.model.IntegerType theLimit, @OperationParam(name = JpaConstants.OPERATION_EXPUNGE_PARAM_EXPUNGE_DELETED_RESOURCES) org.hl7.fhir.r4.model.BooleanType theExpungeDeletedResources, - @OperationParam(name = JpaConstants.OPERATION_EXPUNGE_PARAM_EXPUNGE_OLD_VERSIONS) org.hl7.fhir.r4.model.BooleanType theExpungeOldVersions + @OperationParam(name = JpaConstants.OPERATION_EXPUNGE_PARAM_EXPUNGE_PREVIOUS_VERSIONS) org.hl7.fhir.r4.model.BooleanType theExpungeOldVersions ) { org.hl7.fhir.r4.model.Parameters retVal = super.doExpunge(theIdParam, theLimit, theExpungeDeletedResources, theExpungeOldVersions, null); try { @@ -105,7 +105,7 @@ public class JpaResourceProviderDstu3 extends BaseJpaRes public Parameters expunge( @OperationParam(name = JpaConstants.OPERATION_EXPUNGE_PARAM_LIMIT) org.hl7.fhir.r4.model.IntegerType theLimit, @OperationParam(name = JpaConstants.OPERATION_EXPUNGE_PARAM_EXPUNGE_DELETED_RESOURCES) org.hl7.fhir.r4.model.BooleanType theExpungeDeletedResources, - @OperationParam(name = JpaConstants.OPERATION_EXPUNGE_PARAM_EXPUNGE_OLD_VERSIONS) org.hl7.fhir.r4.model.BooleanType theExpungeOldVersions + @OperationParam(name = JpaConstants.OPERATION_EXPUNGE_PARAM_EXPUNGE_PREVIOUS_VERSIONS) org.hl7.fhir.r4.model.BooleanType theExpungeOldVersions ) { org.hl7.fhir.r4.model.Parameters retVal = super.doExpunge(null, theLimit, theExpungeDeletedResources, theExpungeOldVersions, null); try { diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/dstu3/JpaSystemProviderDstu3.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/dstu3/JpaSystemProviderDstu3.java index 3d63b14ac4a..693803d2585 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/dstu3/JpaSystemProviderDstu3.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/dstu3/JpaSystemProviderDstu3.java @@ -62,7 +62,7 @@ public class JpaSystemProviderDstu3 extends BaseJpaSystemProviderDstu2Plus extends BaseJpaResour @IdParam IIdType theIdParam, @OperationParam(name = JpaConstants.OPERATION_EXPUNGE_PARAM_LIMIT) IntegerType theLimit, @OperationParam(name = JpaConstants.OPERATION_EXPUNGE_PARAM_EXPUNGE_DELETED_RESOURCES) BooleanType theExpungeDeletedResources, - @OperationParam(name = JpaConstants.OPERATION_EXPUNGE_PARAM_EXPUNGE_OLD_VERSIONS) BooleanType theExpungeOldVersions + @OperationParam(name = JpaConstants.OPERATION_EXPUNGE_PARAM_EXPUNGE_PREVIOUS_VERSIONS) BooleanType theExpungeOldVersions ) { return super.doExpunge(theIdParam, theLimit, theExpungeDeletedResources, theExpungeOldVersions, null); } @@ -95,7 +95,7 @@ public class JpaResourceProviderR4 extends BaseJpaResour public Parameters expunge( @OperationParam(name = JpaConstants.OPERATION_EXPUNGE_PARAM_LIMIT) IntegerType theLimit, @OperationParam(name = JpaConstants.OPERATION_EXPUNGE_PARAM_EXPUNGE_DELETED_RESOURCES) BooleanType theExpungeDeletedResources, - @OperationParam(name = JpaConstants.OPERATION_EXPUNGE_PARAM_EXPUNGE_OLD_VERSIONS) BooleanType theExpungeOldVersions + @OperationParam(name = JpaConstants.OPERATION_EXPUNGE_PARAM_EXPUNGE_PREVIOUS_VERSIONS) BooleanType theExpungeOldVersions ) { return super.doExpunge(null, theLimit, theExpungeDeletedResources, theExpungeOldVersions, null); } diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/r4/JpaSystemProviderR4.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/r4/JpaSystemProviderR4.java index ed82b607e92..6dcf67a0413 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/r4/JpaSystemProviderR4.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/provider/r4/JpaSystemProviderR4.java @@ -59,7 +59,7 @@ public class JpaSystemProviderR4 extends BaseJpaSystemProviderDstu2Plus theTesters) { theTesters.forEach(this::addTester); } - + boolean applyTesters(RestOperationTypeEnum theOperation, RequestDetails theRequestDetails, IIdType theInputResourceId, IBaseResource theInputResource, IBaseResource theOutputResource) { boolean retVal = true; if (theOutputResource == null) { diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderOperationNamed.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderOperationNamed.java index 4baf720be5f..d2ff8349d76 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderOperationNamed.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/IAuthRuleBuilderOperationNamed.java @@ -38,7 +38,7 @@ public interface IAuthRuleBuilderOperationNamed { /** * Rule applies to invocations of this operation at the type level on any type */ - IAuthRuleFinished onAnyType(); + IAuthRuleBuilderRuleOpClassifierFinished onAnyType(); /** * Rule applies to invocations of this operation at the instance level @@ -53,6 +53,11 @@ public interface IAuthRuleBuilderOperationNamed { /** * Rule applies to invocations of this operation at the instance level on any instance */ - IAuthRuleFinished onAnyInstance(); + IAuthRuleBuilderRuleOpClassifierFinished onAnyInstance(); + + /** + * Rule applies to invocations of this operation at any level (server, type or instance) + */ + IAuthRuleBuilderRuleOpClassifierFinished atAnyLevel(); } diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/OperationRule.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/OperationRule.java index 48b075bde8e..f115b6f9322 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/OperationRule.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/OperationRule.java @@ -40,11 +40,16 @@ class OperationRule extends BaseRule implements IAuthRule { private HashSet> myAppliesToInstancesOfType; private boolean myAppliesToAnyType; private boolean myAppliesToAnyInstance; + private boolean myAppliesAtAnyLevel; public OperationRule(String theRuleName) { super(theRuleName); } + public void appliesAtAnyLevel(boolean theAppliesAtAnyLevel) { + myAppliesAtAnyLevel = theAppliesAtAnyLevel; + } + public void appliesToAnyInstance() { myAppliesToAnyInstance = true; } @@ -82,12 +87,12 @@ class OperationRule extends BaseRule implements IAuthRule { boolean applies = false; switch (theOperation) { case EXTENDED_OPERATION_SERVER: - if (myAppliesToServer) { + if (myAppliesToServer || myAppliesAtAnyLevel) { applies = true; } break; case EXTENDED_OPERATION_TYPE: - if (myAppliesToAnyType) { + if (myAppliesToAnyType || myAppliesAtAnyLevel) { applies = true; } else if (myAppliesToTypes != null) { // TODO: Convert to a map of strings and keep the result @@ -101,7 +106,7 @@ class OperationRule extends BaseRule implements IAuthRule { } break; case EXTENDED_OPERATION_INSTANCE: - if (myAppliesToAnyInstance) { + if (myAppliesToAnyInstance || myAppliesAtAnyLevel) { applies = true; } else if (theInputResourceId != null) { if (myAppliesToIds != null) { diff --git a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java index 11eee16b62c..e2f7efe3800 100644 --- a/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java +++ b/hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/interceptor/auth/RuleBuilder.java @@ -451,7 +451,7 @@ public class RuleBuilder implements IAuthRuleBuilder { } @Override - public IAuthRuleFinished onAnyInstance() { + public IAuthRuleBuilderRuleOpClassifierFinished onAnyInstance() { OperationRule rule = createRule(); rule.appliesToAnyInstance(); myRules.add(rule); @@ -459,7 +459,15 @@ public class RuleBuilder implements IAuthRuleBuilder { } @Override - public IAuthRuleFinished onAnyType() { + public IAuthRuleBuilderRuleOpClassifierFinished atAnyLevel() { + OperationRule rule = createRule(); + rule.appliesAtAnyLevel(true); + myRules.add(rule); + return new RuleBuilderFinished(rule); + } + + @Override + public IAuthRuleBuilderRuleOpClassifierFinished onAnyType() { OperationRule rule = createRule(); rule.appliesToAnyType(); myRules.add(rule); @@ -473,7 +481,7 @@ public class RuleBuilder implements IAuthRuleBuilder { Validate.notBlank(theInstanceId.getIdPart(), "theInstanceId does not have an ID part"); OperationRule rule = createRule(); - ArrayList ids = new ArrayList(); + ArrayList ids = new ArrayList<>(); ids.add(theInstanceId); rule.appliesToInstances(ids); myRules.add(rule); @@ -509,7 +517,7 @@ public class RuleBuilder implements IAuthRuleBuilder { } private HashSet> toTypeSet(Class theType) { - HashSet> appliesToTypes = new HashSet>(); + HashSet> appliesToTypes = new HashSet<>(); appliesToTypes.add(theType); return appliesToTypes; } diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/AuthorizationInterceptorR4Test.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/AuthorizationInterceptorR4Test.java index 272d3bd907c..165ef33c6d4 100644 --- a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/AuthorizationInterceptorR4Test.java +++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/rest/server/interceptor/AuthorizationInterceptorR4Test.java @@ -1158,6 +1158,119 @@ public class AuthorizationInterceptorR4Test { assertFalse(ourHitMethod); } + + @Test + public void testOperationAppliesAtAnyLevel() throws Exception { + ourServlet.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { + @Override + public List buildRuleList(RequestDetails theRequestDetails) { + return new RuleBuilder() + .allow("RULE 1").operation().named("opName").atAnyLevel().andThen() + .build(); + } + }); + + HttpGet httpGet; + HttpResponse status; + String response; + + // Server + ourHitMethod = false; + ourReturn = Collections.singletonList(createObservation(10, "Patient/2")); + httpGet = new HttpGet("http://localhost:" + ourPort + "/$opName"); + status = ourClient.execute(httpGet); + response = extractResponseAndClose(status); + assertEquals(200, status.getStatusLine().getStatusCode()); + assertTrue(ourHitMethod); + + // Type + ourHitMethod = false; + ourReturn = Collections.singletonList(createPatient(2)); + httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/$opName"); + status = ourClient.execute(httpGet); + response = extractResponseAndClose(status); + ourLog.info(response); + assertEquals(200, status.getStatusLine().getStatusCode()); + assertTrue(ourHitMethod); + + // Instance + ourHitMethod = false; + ourReturn = Collections.singletonList(createPatient(2)); + httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/1/$opName"); + status = ourClient.execute(httpGet); + response = extractResponseAndClose(status); + ourLog.info(response); + assertEquals(200, status.getStatusLine().getStatusCode()); + assertTrue(ourHitMethod); + + // Instance Version + ourHitMethod = false; + ourReturn = Collections.singletonList(createPatient(2)); + httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/1/_history/2/$opName"); + status = ourClient.execute(httpGet); + response = extractResponseAndClose(status); + ourLog.info(response); + assertEquals(200, status.getStatusLine().getStatusCode()); + assertTrue(ourHitMethod); + + } + + @Test + public void testOperationAppliesAtAnyLevelWrongOpName() throws Exception { + ourServlet.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { + @Override + public List buildRuleList(RequestDetails theRequestDetails) { + return new RuleBuilder() + .allow("RULE 1").operation().named("opNameBadOp").atAnyLevel().andThen() + .build(); + } + }); + + HttpGet httpGet; + HttpResponse status; + String response; + + // Server + ourHitMethod = false; + ourReturn = Collections.singletonList(createObservation(10, "Patient/2")); + httpGet = new HttpGet("http://localhost:" + ourPort + "/$opName"); + status = ourClient.execute(httpGet); + response = extractResponseAndClose(status); + assertEquals(403, status.getStatusLine().getStatusCode()); + assertFalse(ourHitMethod); + + // Type + ourHitMethod = false; + ourReturn = Collections.singletonList(createPatient(2)); + httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/$opName"); + status = ourClient.execute(httpGet); + response = extractResponseAndClose(status); + ourLog.info(response); + assertEquals(403, status.getStatusLine().getStatusCode()); + assertFalse(ourHitMethod); + + // Instance + ourHitMethod = false; + ourReturn = Collections.singletonList(createPatient(2)); + httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/1/$opName"); + status = ourClient.execute(httpGet); + response = extractResponseAndClose(status); + ourLog.info(response); + assertEquals(403, status.getStatusLine().getStatusCode()); + assertFalse(ourHitMethod); + + // Instance Version + ourHitMethod = false; + ourReturn = Collections.singletonList(createPatient(2)); + httpGet = new HttpGet("http://localhost:" + ourPort + "/Patient/1/_history/2/$opName"); + status = ourClient.execute(httpGet); + response = extractResponseAndClose(status); + ourLog.info(response); + assertEquals(403, status.getStatusLine().getStatusCode()); + assertFalse(ourHitMethod); + + } + @Test public void testOperationTypeLevel() throws Exception { ourServlet.registerInterceptor(new AuthorizationInterceptor(PolicyEnum.DENY) { diff --git a/src/changes/changes.xml b/src/changes/changes.xml index f5fa680203f..c5acecfd0bc 100644 --- a/src/changes/changes.xml +++ b/src/changes/changes.xml @@ -71,6 +71,13 @@ source IPs, or to only allow operations with specific parameter values. + + A new qualifier has been added to the AuthorizationInterceptor + RuleBuilder that allows a rule on an operation to match + atAnyLevel()]]>, meaning that the rule + applies to the operation by name whether it is at the + server, type, or instance level. +