Transaction fails if SearchNarrowingInterceptor is registered and Partitioning Enabled - fix cross-tenant requests failure (#5408)

* Transaction with conditional update fails if SearchNarrowingInterceptor is registered and Enabled Partitioning - fix and tests added
This commit is contained in:
volodymyr-korzh 2023-10-27 17:17:37 -06:00 committed by GitHub
parent 97fc5ca0dc
commit 1c5fe61b38
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 51 additions and 4 deletions

View File

@ -69,7 +69,7 @@ public class UrlBaseTenantIdentificationStrategy implements ITenantIdentificatio
tenantId = defaultIfBlank(theUrlPathTokenizer.peek(), null);
// If it's "metadata" or starts with "$", use DEFAULT partition and don't consume this token:
if (tenantId != null && (tenantId.equals("metadata") || tenantId.startsWith("$"))) {
if (tenantId != null && (tenantId.equals("metadata") || isOperation(tenantId))) {
tenantId = "DEFAULT";
theRequestDetails.setTenantId(tenantId);
ourLog.trace("No tenant ID found for metadata or system request; using DEFAULT.");
@ -94,6 +94,10 @@ public class UrlBaseTenantIdentificationStrategy implements ITenantIdentificatio
}
}
private boolean isOperation(String theToken) {
return theToken.startsWith("$");
}
@Override
public String massageServerBaseUrl(String theFhirServerBase, RequestDetails theRequestDetails) {
String result = theFhirServerBase;
@ -105,9 +109,26 @@ public class UrlBaseTenantIdentificationStrategy implements ITenantIdentificatio
@Override
public String resolveRelativeUrl(String theRelativeUrl, RequestDetails theRequestDetails) {
if (theRequestDetails.getTenantId() != null && !theRelativeUrl.startsWith(theRequestDetails.getTenantId())) {
return theRequestDetails.getTenantId() + "/" + theRelativeUrl;
UrlPathTokenizer tokenizer = new UrlPathTokenizer(theRelativeUrl);
// there is no more tokens in the URL - skip url resolution
if (!tokenizer.hasMoreTokens() || tokenizer.peek() == null) {
return theRelativeUrl;
}
return theRelativeUrl;
String nextToken = tokenizer.peek();
// there is no tenant ID in parent request details or tenant ID is already present in URL - skip url resolution
if (theRequestDetails.getTenantId() == null || nextToken.equals(theRequestDetails.getTenantId())) {
return theRelativeUrl;
}
// token is Resource type or operation - adding tenant ID from parent request details
if (isResourceType(nextToken, theRequestDetails) || isOperation(nextToken)) {
return theRequestDetails.getTenantId() + "/" + theRelativeUrl;
} else {
return theRelativeUrl;
}
}
private boolean isResourceType(String token, RequestDetails theRequestDetails) {
return theRequestDetails.getFhirContext().getResourceTypes().stream().anyMatch(type -> type.equals(token));
}
}

View File

@ -11,12 +11,17 @@ import ca.uhn.fhir.util.UrlPathTokenizer;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;
import org.mockito.Mock;
import org.mockito.junit.jupiter.MockitoExtension;
import java.util.Collections;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@ -69,6 +74,27 @@ public class UrlBaseTenantIdentificationStrategyTest {
assertEquals(BASE_URL, actual);
}
@CsvSource(value = {
" , , empty input url - empty URL should be returned",
"TENANT1/Patient/123 , TENANT1/Patient/123 , tenant ID already exists - input URL should be returned",
"TENANT1/Patient/$export, TENANT1/Patient/$export , tenant ID already exists - input URL should be returned",
"TENANT2/Patient/123 , TENANT2/Patient/123 , requestDetails contains different tenant ID - input URL should be returned",
"TENANT2/$export , TENANT2/$export , requestDetails contains different tenant ID - input URL should be returned",
"Patient/123 , TENANT1/Patient/123 , url starts with resource type - tenant ID should be added to URL",
"Patient/$export , TENANT1/Patient/$export , url starts with resource type - tenant ID should be added to URL",
"$export , TENANT1/$export , url starts with operation name - tenant ID should be added to URL",
})
@ParameterizedTest
void resolveRelativeUrl_returnsCorrectlyResolvedUrl(String theInputUrl, String theExpectedResolvedUrl, String theMessage) {
lenient().when(myRequestDetails.getTenantId()).thenReturn("TENANT1");
lenient().when(myFHIRContext.getResourceTypes()).thenReturn(Collections.singleton("Patient"));
lenient().when(myRequestDetails.getFhirContext()).thenReturn(myFHIRContext);
String actual = ourTenantStrategy.resolveRelativeUrl(theInputUrl, myRequestDetails);
assertEquals(theExpectedResolvedUrl, actual, theMessage);
}
@Test
void extractTenant_givenNormalRequestAndExplicitTenant_shouldUseTenant() {
//given a Patient request on MYTENANT