Better error messages for ValueSet $expand contextDirection=existing (#3446)
* Better error messages for ValueSet $expand contextDirection=existing * Apply to $lastn too - it uses aggregations
This commit is contained in:
parent
d11a312abf
commit
20e092ba6d
|
@ -25,7 +25,7 @@ public final class Msg {
|
|||
|
||||
/**
|
||||
* IMPORTANT: Please update the following comment after you add a new code
|
||||
* Last code value: 2068
|
||||
* Last code value: 2070
|
||||
*/
|
||||
|
||||
private Msg() {}
|
||||
|
|
|
@ -83,6 +83,14 @@ public enum TokenParamModifier {
|
|||
return myValue;
|
||||
}
|
||||
|
||||
/**
|
||||
* The modifier without the :
|
||||
* @return the string after the leading :
|
||||
*/
|
||||
public String getBareModifier() {
|
||||
return myValue.substring(1);
|
||||
}
|
||||
|
||||
public static TokenParamModifier forValue(String theValue) {
|
||||
return VALUE_TO_ENUM.get(theValue);
|
||||
}
|
||||
|
|
|
@ -22,6 +22,7 @@ package ca.uhn.fhir.jpa.dao;
|
|||
|
||||
import ca.uhn.fhir.context.FhirContext;
|
||||
import ca.uhn.fhir.context.RuntimeSearchParam;
|
||||
import ca.uhn.fhir.i18n.Msg;
|
||||
import ca.uhn.fhir.jpa.api.config.DaoConfig;
|
||||
import ca.uhn.fhir.jpa.dao.data.IForcedIdDao;
|
||||
import ca.uhn.fhir.jpa.dao.search.ExtendedLuceneClauseBuilder;
|
||||
|
@ -42,9 +43,11 @@ import ca.uhn.fhir.rest.api.server.storage.ResourcePersistentId;
|
|||
import ca.uhn.fhir.rest.param.StringParam;
|
||||
import ca.uhn.fhir.rest.param.TokenParam;
|
||||
import ca.uhn.fhir.rest.server.util.ISearchParamRegistry;
|
||||
import org.hibernate.search.backend.elasticsearch.ElasticsearchExtension;
|
||||
import org.hibernate.search.mapper.orm.Search;
|
||||
import org.hibernate.search.mapper.orm.session.SearchSession;
|
||||
import org.hibernate.search.mapper.orm.work.SearchIndexingPlan;
|
||||
import org.hibernate.search.util.common.SearchException;
|
||||
import org.hl7.fhir.instance.model.api.IAnyResource;
|
||||
import org.hl7.fhir.instance.model.api.IBaseResource;
|
||||
import org.springframework.beans.factory.annotation.Autowired;
|
||||
|
@ -235,13 +238,35 @@ public class FulltextSearchSvcImpl implements IFulltextSearchSvc {
|
|||
@Transactional()
|
||||
@Override
|
||||
public IBaseResource tokenAutocompleteValueSetSearch(ValueSetAutocompleteOptions theOptions) {
|
||||
ensureElastic();
|
||||
|
||||
ValueSetAutocompleteSearch autocomplete = new ValueSetAutocompleteSearch(myFhirContext, getSearchSession());
|
||||
|
||||
return autocomplete.search(theOptions);
|
||||
}
|
||||
|
||||
/**
|
||||
* Throws an error if configured with Lucene.
|
||||
*
|
||||
* Some features only work with Elasticsearch.
|
||||
* Lastn and the autocomplete search use nested aggregations which are Elasticsearch-only
|
||||
*/
|
||||
private void ensureElastic() {
|
||||
//String hibernateSearchBackend = (String) myEntityManager.g.getJpaPropertyMap().get(BackendSettings.backendKey(BackendSettings.TYPE));
|
||||
try {
|
||||
getSearchSession().scope( ResourceTable.class )
|
||||
.aggregation()
|
||||
.extension(ElasticsearchExtension.get());
|
||||
} catch (SearchException e) {
|
||||
// unsupported. we are probably running Lucene.
|
||||
throw new IllegalStateException(Msg.code(2070) + "This operation requires Elasticsearch. Lucene is not supported.");
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
@Override
|
||||
public List<ResourcePersistentId> lastN(SearchParameterMap theParams, Integer theMaximumResults) {
|
||||
ensureElastic();
|
||||
List<Long> pidList = new LastNOperation(getSearchSession(), myFhirContext, mySearchParamRegistry)
|
||||
.executeLastN(theParams, theMaximumResults);
|
||||
return convertLongsToResourcePersistentIds(pidList);
|
||||
|
|
|
@ -22,22 +22,50 @@ package ca.uhn.fhir.jpa.search.autocomplete;
|
|||
|
||||
import ca.uhn.fhir.i18n.Msg;
|
||||
import ca.uhn.fhir.jpa.api.config.DaoConfig;
|
||||
import ca.uhn.fhir.rest.param.TokenParamModifier;
|
||||
import ca.uhn.fhir.rest.server.exceptions.InvalidRequestException;
|
||||
import org.hl7.fhir.instance.model.api.IBaseResource;
|
||||
import org.hl7.fhir.instance.model.api.IIdType;
|
||||
import org.hl7.fhir.instance.model.api.IPrimitiveType;
|
||||
|
||||
import java.util.Arrays;
|
||||
import java.util.List;
|
||||
import java.util.Optional;
|
||||
|
||||
import static org.apache.commons.lang3.StringUtils.defaultString;
|
||||
import static org.apache.commons.lang3.StringUtils.isNotBlank;
|
||||
|
||||
public class ValueSetAutocompleteOptions {
|
||||
|
||||
private String myResourceType;
|
||||
private String mySearchParamCode;
|
||||
private String mySearchParamModifier;
|
||||
private String myFilter;
|
||||
private Integer myCount;
|
||||
private final String myResourceType;
|
||||
private final String mySearchParamCode;
|
||||
private final String mySearchParamModifier;
|
||||
private final String myFilter;
|
||||
private final Integer myCount;
|
||||
|
||||
static final List<String> ourSupportedModifiers = Arrays.asList("", TokenParamModifier.TEXT.getBareModifier());
|
||||
|
||||
public ValueSetAutocompleteOptions(String theContext, String theFilter, Integer theCount) {
|
||||
myFilter = theFilter;
|
||||
myCount = theCount;
|
||||
int separatorIdx = theContext.indexOf('.');
|
||||
String codeWithPossibleModifier;
|
||||
if (separatorIdx >= 0) {
|
||||
myResourceType = theContext.substring(0, separatorIdx);
|
||||
codeWithPossibleModifier = theContext.substring(separatorIdx + 1);
|
||||
} else {
|
||||
myResourceType = null;
|
||||
codeWithPossibleModifier = theContext;
|
||||
}
|
||||
int modifierIdx = codeWithPossibleModifier.indexOf(':');
|
||||
if (modifierIdx >= 0) {
|
||||
mySearchParamCode = codeWithPossibleModifier.substring(0, modifierIdx);
|
||||
mySearchParamModifier = codeWithPossibleModifier.substring(modifierIdx + 1);
|
||||
} else {
|
||||
mySearchParamCode = codeWithPossibleModifier;
|
||||
mySearchParamModifier = null;
|
||||
}
|
||||
}
|
||||
|
||||
public static ValueSetAutocompleteOptions validateAndParseOptions(
|
||||
DaoConfig theDaoConfig,
|
||||
|
@ -57,38 +85,19 @@ public class ValueSetAutocompleteOptions {
|
|||
if (!theDaoConfig.isAdvancedLuceneIndexing()) {
|
||||
throw new InvalidRequestException(Msg.code(2022) + "$expand with contexDirection='existing' requires Extended Lucene Indexing.");
|
||||
}
|
||||
ValueSetAutocompleteOptions result = new ValueSetAutocompleteOptions();
|
||||
if (theContext == null || theContext.isEmpty()) {
|
||||
throw new InvalidRequestException(Msg.code(2021) + "$expand with contexDirection='existing' requires a context");
|
||||
}
|
||||
String filter = theFilter == null ? null : theFilter.getValue();
|
||||
ValueSetAutocompleteOptions result = new ValueSetAutocompleteOptions(theContext.getValue(), filter, IPrimitiveType.toValueOrNull(theCount));
|
||||
|
||||
result.parseContext(theContext);
|
||||
result.myFilter =
|
||||
theFilter == null ? null : theFilter.getValue();
|
||||
result.myCount = IPrimitiveType.toValueOrNull(theCount);
|
||||
if (!ourSupportedModifiers.contains(defaultString(result.getSearchParamModifier()))) {
|
||||
throw new InvalidRequestException(Msg.code(2069) + "$expand with contexDirection='existing' only supports plain token search, or the :text modifier. Received " + result.getSearchParamModifier());
|
||||
}
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
private void parseContext(IPrimitiveType<String> theContextWrapper) {
|
||||
if (theContextWrapper == null || theContextWrapper.isEmpty()) {
|
||||
throw new InvalidRequestException(Msg.code(2021) + "$expand with contexDirection='existing' requires a context");
|
||||
}
|
||||
String theContext = theContextWrapper.getValue();
|
||||
int separatorIdx = theContext.indexOf('.');
|
||||
String codeWithPossibleModifier;
|
||||
if (separatorIdx >= 0) {
|
||||
myResourceType = theContext.substring(0, separatorIdx);
|
||||
codeWithPossibleModifier = theContext.substring(separatorIdx + 1);
|
||||
} else {
|
||||
codeWithPossibleModifier = theContext;
|
||||
}
|
||||
int modifierIdx = codeWithPossibleModifier.indexOf(':');
|
||||
if (modifierIdx >= 0) {
|
||||
mySearchParamCode = codeWithPossibleModifier.substring(0, modifierIdx);
|
||||
mySearchParamModifier = codeWithPossibleModifier.substring(modifierIdx + 1);
|
||||
} else {
|
||||
mySearchParamCode = codeWithPossibleModifier;
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
public String getResourceType() {
|
||||
return myResourceType;
|
||||
|
|
|
@ -1,24 +1,37 @@
|
|||
package ca.uhn.fhir.jpa.dao.r4;
|
||||
|
||||
import ca.uhn.fhir.i18n.Msg;
|
||||
import ca.uhn.fhir.jpa.dao.BaseHapiFhirDao;
|
||||
import ca.uhn.fhir.jpa.search.autocomplete.ValueSetAutocompleteOptions;
|
||||
import ca.uhn.fhir.jpa.searchparam.SearchParameterMap;
|
||||
import ca.uhn.fhir.rest.api.Constants;
|
||||
import ca.uhn.fhir.rest.api.server.IBundleProvider;
|
||||
import ca.uhn.fhir.rest.param.*;
|
||||
import ca.uhn.fhir.rest.param.StringAndListParam;
|
||||
import ca.uhn.fhir.rest.param.StringOrListParam;
|
||||
import ca.uhn.fhir.rest.param.StringParam;
|
||||
import ca.uhn.fhir.rest.param.TokenParam;
|
||||
import ca.uhn.fhir.rest.param.TokenParamModifier;
|
||||
import ca.uhn.fhir.rest.server.servlet.ServletRequestDetails;
|
||||
import org.hl7.fhir.instance.model.api.IIdType;
|
||||
import org.hl7.fhir.r4.model.*;
|
||||
import org.hl7.fhir.r4.model.Device;
|
||||
import org.hl7.fhir.r4.model.Observation;
|
||||
import org.hl7.fhir.r4.model.Observation.ObservationStatus;
|
||||
import org.hl7.fhir.r4.model.Patient;
|
||||
import org.hl7.fhir.r4.model.Quantity;
|
||||
import org.hl7.fhir.r4.model.StringType;
|
||||
import org.junit.jupiter.api.BeforeEach;
|
||||
import org.junit.jupiter.api.Disabled;
|
||||
import org.junit.jupiter.api.Test;
|
||||
|
||||
import javax.servlet.http.HttpServletRequest;
|
||||
import java.util.List;
|
||||
|
||||
import static org.hamcrest.MatcherAssert.assertThat;
|
||||
import static org.hamcrest.Matchers.*;
|
||||
import static org.junit.jupiter.api.Assertions.*;
|
||||
import static org.hamcrest.Matchers.contains;
|
||||
import static org.hamcrest.Matchers.containsInAnyOrder;
|
||||
import static org.hamcrest.Matchers.empty;
|
||||
import static org.hamcrest.Matchers.not;
|
||||
import static org.hamcrest.Matchers.startsWith;
|
||||
import static org.junit.jupiter.api.Assertions.assertNotEquals;
|
||||
import static org.junit.jupiter.api.Assertions.fail;
|
||||
import static org.mockito.Mockito.mock;
|
||||
|
||||
public class FhirResourceDaoR4SearchFtTest extends BaseJpaR4Test {
|
||||
|
@ -457,5 +470,17 @@ public class FhirResourceDaoR4SearchFtTest extends BaseJpaR4Test {
|
|||
|
||||
}
|
||||
|
||||
/**
|
||||
* make sure we provide a clear error message when a feature requires Elastic
|
||||
*/
|
||||
@Test
|
||||
public void tokenAutocompleteFailsWithLucene() {
|
||||
try {
|
||||
myFulltestSearchSvc.tokenAutocompleteValueSetSearch(new ValueSetAutocompleteOptions("Observation.code", null, null));
|
||||
fail("Expected exception");
|
||||
} catch (IllegalStateException e) {
|
||||
assertThat(e.getMessage(), startsWith(Msg.code(2070)));
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -36,7 +36,7 @@ class ValueSetAutocompleteOptionsTest {
|
|||
private IPrimitiveType<String> myUrl;
|
||||
private ValueSet myValueSet;
|
||||
private ValueSetAutocompleteOptions myOptionsResult;
|
||||
private DaoConfig myDaoConfig = new DaoConfig();
|
||||
final private DaoConfig myDaoConfig = new DaoConfig();
|
||||
|
||||
{
|
||||
myDaoConfig.setAdvancedLuceneIndexing(true);
|
||||
|
@ -159,6 +159,14 @@ class ValueSetAutocompleteOptionsTest {
|
|||
assertParseThrowsInvalidRequestWithErrorCode(ERROR_AUTOCOMPLETE_REQUIRES_CONTEXT);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void withUnsupportedModifier() {
|
||||
myFilter = new StringDt("blood");
|
||||
myContext = new StringDt("Observation.code:exact");
|
||||
|
||||
assertParseThrowsInvalidRequestWithErrorCode(2069);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void whenAdvancedIndexingOff() {
|
||||
// given
|
||||
|
@ -168,7 +176,6 @@ class ValueSetAutocompleteOptionsTest {
|
|||
}
|
||||
|
||||
|
||||
|
||||
private void assertParseThrowsInvalidRequestWithErrorCode(int theErrorCode) {
|
||||
InvalidRequestException e = assertThrows(InvalidRequestException.class, ValueSetAutocompleteOptionsTest.this::parseOptions);
|
||||
assertThat(e.getMessage(), startsWith(Msg.code(theErrorCode)));
|
||||
|
|
Loading…
Reference in New Issue