Transaction with conditional update fails if SearchNarrowingInterceptor is registered and Enabled Partitioning (#5389)

* Transaction with conditional update fails if SearchNarrowingInterceptor is registered and Enabled Partitioning - Implementation
This commit is contained in:
volodymyr-korzh 2023-10-23 14:26:25 -06:00 committed by GitHub
parent edb01a891a
commit 52bdc2693c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 92 additions and 6 deletions

View File

@ -0,0 +1,6 @@
---
type: fix
issue: 5388
title: "Previously, with partitioning enabled and `UrlBaseTenantIdentificationStrategy` used, registering
`SearchNarrowingInterceptor` would cause to incorrect resolution of `entry.request.url` parameter during
transaction bundle processing. This has been fixed."

View File

@ -23,6 +23,7 @@ import ca.uhn.fhir.rest.server.RestfulServer;
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
import ca.uhn.fhir.rest.server.exceptions.MethodNotAllowedException;
import ca.uhn.fhir.rest.server.exceptions.ResourceNotFoundException;
import ca.uhn.fhir.rest.server.interceptor.auth.SearchNarrowingInterceptor;
import ca.uhn.fhir.rest.server.provider.ProviderConstants;
import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails;
import ca.uhn.fhir.test.utilities.ITestDataBuilder;
@ -40,11 +41,14 @@ import org.hl7.fhir.r4.model.Condition;
import org.hl7.fhir.r4.model.IdType;
import org.hl7.fhir.r4.model.Organization;
import org.hl7.fhir.r4.model.Patient;
import org.hl7.fhir.r4.model.Resource;
import org.hl7.fhir.r4.model.StringType;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.Spy;
@ -65,6 +69,7 @@ import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.hasSize;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
@ -424,6 +429,55 @@ public class MultitenantServerR4Test extends BaseMultitenantResourceProviderR4Te
}
@Test
public void testTransactionPut_withSearchNarrowingInterceptor_createsPatient() {
// setup
IBaseResource patientA = buildPatient(withTenant(TENANT_B), withActiveTrue(), withId("1234a"),
withFamily("Family"), withGiven("Given"));
Bundle transactioBundle = new Bundle();
transactioBundle.setType(Bundle.BundleType.TRANSACTION);
transactioBundle.addEntry()
.setFullUrl("http://localhost:8000/TENANT-A/Patient/1234a")
.setResource((Resource) patientA)
.getRequest().setUrl("Patient/1234a").setMethod(Bundle.HTTPVerb.PUT);
myServer.registerInterceptor(new SearchNarrowingInterceptor());
// execute
myClient.transaction().withBundle(transactioBundle).execute();
// verify - read back using DAO
SystemRequestDetails requestDetails = new SystemRequestDetails();
requestDetails.setTenantId(TENANT_B);
Patient patient1 = myPatientDao.read(new IdType("Patient/1234a"), requestDetails);
assertEquals("Family", patient1.getName().get(0).getFamily());
}
@ParameterizedTest
@ValueSource(strings = {"Patient/1234a", "TENANT-B/Patient/1234a"})
public void testTransactionGet_withSearchNarrowingInterceptor_retrievesPatient(String theEntryUrl) {
// setup
createPatient(withTenant(TENANT_B), withActiveTrue(), withId("1234a"),
withFamily("Family"), withGiven("Given"));
Bundle transactioBundle = new Bundle();
transactioBundle.setType(Bundle.BundleType.TRANSACTION);
transactioBundle.addEntry()
.getRequest().setUrl(theEntryUrl).setMethod(Bundle.HTTPVerb.GET);
myServer.registerInterceptor(new SearchNarrowingInterceptor());
// execute
Bundle result = myClient.transaction().withBundle(transactioBundle).execute();
// verify
assertEquals(1, result.getEntry().size());
Patient retrievedPatient = (Patient) result.getEntry().get(0).getResource();
assertNotNull(retrievedPatient);
assertEquals("Family", retrievedPatient.getName().get(0).getFamily());
}
@Test
public void testDirectDaoAccess_PartitionInRequestDetails_Create() {

View File

@ -1598,9 +1598,16 @@ public class RestfulServer extends HttpServlet implements IRestfulServer<Servlet
myUncompressIncomingContents = theUncompressIncomingContents;
}
private String resolveRequestPath(RequestDetails theRequestDetails, String theRequestPath) {
if (myTenantIdentificationStrategy != null) {
theRequestPath = myTenantIdentificationStrategy.resolveRelativeUrl(theRequestPath, theRequestDetails);
}
return theRequestPath;
}
public void populateRequestDetailsFromRequestPath(RequestDetails theRequestDetails, String theRequestPath) {
UrlPathTokenizer tok = new UrlPathTokenizer(theRequestPath);
String resourceName = null;
String resolvedRequestPath = resolveRequestPath(theRequestDetails, theRequestPath);
UrlPathTokenizer tok = new UrlPathTokenizer(resolvedRequestPath);
if (myTenantIdentificationStrategy != null) {
myTenantIdentificationStrategy.extractTenant(tok, theRequestDetails);
@ -1609,6 +1616,7 @@ public class RestfulServer extends HttpServlet implements IRestfulServer<Servlet
IIdType id = null;
String operation = null;
String compartment = null;
String resourceName = null;
if (tok.hasMoreTokens()) {
resourceName = tok.nextTokenUnescapedAndSanitized();
if (partIsOperation(resourceName)) {
@ -1635,7 +1643,7 @@ public class RestfulServer extends HttpServlet implements IRestfulServer<Servlet
String versionString = tok.nextTokenUnescapedAndSanitized();
if (id == null) {
throw new InvalidRequestException(
Msg.code(298) + "Don't know how to handle request path: " + theRequestPath);
Msg.code(298) + "Don't know how to handle request path: " + resolvedRequestPath);
}
id.setParts(null, resourceName, id.getIdPart(), UrlUtil.unescape(versionString));
} else {
@ -1644,7 +1652,7 @@ public class RestfulServer extends HttpServlet implements IRestfulServer<Servlet
} else if (partIsOperation(nextString)) {
if (operation != null) {
throw new InvalidRequestException(
Msg.code(299) + "URL Path contains two operations: " + theRequestPath);
Msg.code(299) + "URL Path contains two operations: " + resolvedRequestPath);
}
operation = nextString;
} else {
@ -1663,7 +1671,7 @@ public class RestfulServer extends HttpServlet implements IRestfulServer<Servlet
secondaryOperation = nextString;
} else {
throw new InvalidRequestException(Msg.code(300) + "URL path has unexpected token '" + nextString
+ "' at the end: " + theRequestPath);
+ "' at the end: " + resolvedRequestPath);
}
}

View File

@ -26,7 +26,7 @@ public interface ITenantIdentificationStrategy {
/**
* Implementations should use this method to determine the tenant ID
* based on the incoming request andand populate it in the
* based on the incoming request and populate it in the
* {@link RequestDetails#setTenantId(String)}.
*
* @param theUrlPathTokenizer The tokenizer which is used to parse the request path
@ -39,4 +39,13 @@ public interface ITenantIdentificationStrategy {
* if necessary based on the tenant ID
*/
String massageServerBaseUrl(String theFhirServerBase, RequestDetails theRequestDetails);
/**
* Implementations may use this method to resolve relative URL based on the tenant ID from RequestDetails.
*
* @param theRelativeUrl URL that only includes the path, e.g. "Patient/123"
* @param theRequestDetails The request details object which can be used to access tenant ID
* @return Resolved relative URL that starts with tenant ID (if tenant ID present in RequestDetails). Example: "TENANT-A/Patient/123".
*/
String resolveRelativeUrl(String theRelativeUrl, RequestDetails theRequestDetails);
}

View File

@ -102,4 +102,12 @@ public class UrlBaseTenantIdentificationStrategy implements ITenantIdentificatio
}
return result;
}
@Override
public String resolveRelativeUrl(String theRelativeUrl, RequestDetails theRequestDetails) {
if (theRequestDetails.getTenantId() != null && !theRelativeUrl.startsWith(theRequestDetails.getTenantId())) {
return theRequestDetails.getTenantId() + "/" + theRelativeUrl;
}
return theRelativeUrl;
}
}

View File

@ -63,6 +63,7 @@ public class ServletRequestUtil {
requestDetails.setRequestPath(url);
requestDetails.setFhirServerBase(theRequestDetails.getFhirServerBase());
requestDetails.setTenantId(theRequestDetails.getTenantId());
theRequestDetails.getServer().populateRequestDetailsFromRequestPath(requestDetails, url);
return requestDetails;