Skip to content

Commit

Permalink
Fix new logic for embedded params. No cleanup yet.
Browse files Browse the repository at this point in the history
  • Loading branch information
@lukedegruchy
lukedegruchy committed Jan 24, 2025
1 parent c42da79 commit 92d40ae
Show file tree
Hide file tree
Showing 7 changed files with 354 additions and 77 deletions.
50 changes: 35 additions & 15 deletions ...src/main/java/ca/uhn/fhir/rest/server/method/BaseMethodBindingMethodParameterBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import ca.uhn.fhir.rest.api.server.RequestDetails;
import ca.uhn.fhir.rest.api.server.SystemRequestDetails;
import ca.uhn.fhir.rest.server.exceptions.InternalErrorException;
import ca.uhn.fhir.util.ReflectionUtil;
import jakarta.annotation.Nonnull;
import jakarta.annotation.Nullable;
import org.slf4j.Logger;
Expand Down Expand Up @@ -99,17 +98,27 @@ private Object[] tryBuildMethodParams()
Msg.code(234198927), myMethod, Arrays.toString(myInputMethodParams)));
}

final List<Class<?>> parameterTypesWithOperationEmbeddedParam =
ReflectionUtil.getMethodParamsWithClassesWithFieldsWithAnnotation(
myMethod, EmbeddedOperationParam.class);
ourLog.info(
"1234: START building for method: {}, requestDetails: {}, inputMethodParams: {}",
myMethod.getName(),
myRequestDetails,
Arrays.toString(myInputMethodParams));

final List<Class<?>> parameterTypesWithOperationEmbeddedParams =
EmbeddedOperationUtils.getMethodParamsAnnotatedWithEmbeddableOperationParams(myMethod);

// final List<Class<?>> parameterTypesWithOperationEmbeddedParam =
// ReflectionUtil.getMethodParamsWithClassesWithFieldsWithAnnotation(
// myMethod, EmbeddedOperationParams.class);

if (parameterTypesWithOperationEmbeddedParam.size() > 1) {
// if (parameterTypesWithOperationEmbeddedParam.size() > 1) {
if (parameterTypesWithOperationEmbeddedParams.size() > 1) {
throw new InternalErrorException(String.format(
"%sInvalid operation embedded parameters. More than a single such class is part of method definition: %s",
Msg.code(924469634), myMethod.getName()));
}

if (parameterTypesWithOperationEmbeddedParam.isEmpty()) {
if (parameterTypesWithOperationEmbeddedParams.isEmpty()) {
return myInputMethodParams;
}

Expand Down Expand Up @@ -141,39 +150,50 @@ private Object[] tryBuildMethodParams()
Msg.code(924469634), methodName));
}

final Class<?> parameterTypeWithOperationEmbeddedParam = parameterTypesWithOperationEmbeddedParam.get(0);
final Class<?> parameterTypeWithOperationEmbeddedParams = parameterTypesWithOperationEmbeddedParams.get(0);

return determineMethodParamsForOperationEmbeddedParams(parameterTypeWithOperationEmbeddedParam);
return determineMethodParamsForOperationEmbeddedParams(parameterTypeWithOperationEmbeddedParams);
}

private Object[] determineMethodParamsForOperationEmbeddedParams(
Class<?> theParameterTypeWithOperationEmbeddedParam)
Class<?> theParameterTypeWithOperationEmbeddedParams)
throws InvocationTargetException, IllegalAccessException, InstantiationException {

final String methodName = myMethod.getName();

ourLog.info(
"1234: invoking parameterTypeWithOperationEmbeddedParam: {} and theMethod: {}",
theParameterTypeWithOperationEmbeddedParam,
"1234: invoking parameterTypeWithOperationEmbeddedParams: {} and theMethod: {}",
theParameterTypeWithOperationEmbeddedParams,
methodName);

final Object operationEmbeddedType = buildOperationEmbeddedObject(
methodName, theParameterTypeWithOperationEmbeddedParam, myInputMethodParams);
final Object operationEmbeddedType =
buildOperationEmbeddedObject(theParameterTypeWithOperationEmbeddedParams, myInputMethodParams);

ourLog.info(
"1234: build method params with embedded object and requestDetails (if applicable) for: {}",
operationEmbeddedType);

return buildMethodParamsInCorrectPositions(operationEmbeddedType);
final Object[] params = buildMethodParamsInCorrectPositions(operationEmbeddedType);

ourLog.info(
"1234: END: method: {}, requestDetails: {}, inputMethodParams: {}, outputMethodParams: {}",
myMethod.getName(),
myRequestDetails,
Arrays.toString(myInputMethodParams),
Arrays.toString(params));

return params;
}

@Nonnull
private Object buildOperationEmbeddedObject(
String theMethodName, Class<?> theParameterTypeWithOperationEmbeddedParam, Object[] theMethodParams)
Class<?> theParameterTypeWithOperationEmbeddedParam, Object[] theMethodParams)
throws InstantiationException, IllegalAccessException, InvocationTargetException {
final Constructor<?> constructor =
EmbeddedOperationUtils.validateAndGetConstructor(theParameterTypeWithOperationEmbeddedParam);

// LUKETODO: redo this method in the new constructor param world
// LUKETODO: off by one error
final Object[] methodParamsWithoutRequestDetails = cloneWithRemovedRequestDetails(theMethodParams);

final Annotation[] annotations = Arrays.stream(theParameterTypeWithOperationEmbeddedParam.getDeclaredFields())
Expand Down
10 changes: 5 additions & 5 deletions hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/EmbeddedOperationUtils.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@

import ca.uhn.fhir.context.ConfigurationException;
import ca.uhn.fhir.i18n.Msg;
import ca.uhn.fhir.rest.annotation.EmbeddableOperationParams;
import ca.uhn.fhir.rest.annotation.EmbeddedOperationParam;
import ca.uhn.fhir.rest.annotation.EmbeddedOperationParams;
import ca.uhn.fhir.rest.annotation.OperationParameterRangeType;
import ca.uhn.fhir.util.ReflectionUtil;
import jakarta.annotation.Nonnull;
Expand Down Expand Up @@ -150,13 +150,13 @@ static boolean isValidSourceTypeConversion(
&& OperationParameterRangeType.NOT_APPLICABLE != theOperationParameterRangeType;
}

public static List<Class<?>> getMethodParamsAnnotatedWithEmbeddedOperationParams(Method theMethod) {
public static List<Class<?>> getMethodParamsAnnotatedWithEmbeddableOperationParams(Method theMethod) {
return Arrays.stream(theMethod.getParameterTypes())
.filter(EmbeddedOperationUtils::hasEmbeddedOperationParamsAnnotation)
.filter(EmbeddedOperationUtils::hasEmbeddableOperationParamsAnnotation)
.collect(Collectors.toUnmodifiableList());
}

private static boolean hasEmbeddedOperationParamsAnnotation(Class<?> theMethodParameterType) {
private static boolean hasEmbeddableOperationParamsAnnotation(Class<?> theMethodParameterType) {
final Annotation[] annotations = theMethodParameterType.getAnnotations();

if (annotations.length == 0) {
Expand All @@ -169,7 +169,7 @@ private static boolean hasEmbeddedOperationParamsAnnotation(Class<?> theMethodPa
Msg.code(9132164), theMethodParameterType));
}

return EmbeddedOperationParams.class == annotations[0].annotationType();
return EmbeddableOperationParams.class == annotations[0].annotationType();
}

private static void validateConstructorArgs(Constructor<?> theConstructor, Field[] theDeclaredFields) {
Expand Down
194 changes: 184 additions & 10 deletions hapi-fhir-server/src/main/java/ca/uhn/fhir/rest/server/method/MethodUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
import ca.uhn.fhir.rest.annotation.ConditionalUrlParam;
import ca.uhn.fhir.rest.annotation.Count;
import ca.uhn.fhir.rest.annotation.Elements;
import ca.uhn.fhir.rest.annotation.EmbeddedOperationParam;
import ca.uhn.fhir.rest.annotation.EmbeddableOperationParams;
import ca.uhn.fhir.rest.annotation.EmbeddedOperationParams;
import ca.uhn.fhir.rest.annotation.GraphQLQueryBody;
import ca.uhn.fhir.rest.annotation.GraphQLQueryUrl;
Expand Down Expand Up @@ -71,12 +71,16 @@
import org.hl7.fhir.instance.model.api.IPrimitiveType;

import java.lang.annotation.Annotation;
import java.lang.reflect.Constructor;
import java.lang.reflect.Method;
import java.lang.reflect.Parameter;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Date;
import java.util.List;
import java.util.Optional;
import java.util.stream.Collectors;

import static org.apache.commons.lang3.StringUtils.isNotBlank;

Expand Down Expand Up @@ -350,9 +354,117 @@ public static List<IParameter> getResourceParameters(
parameterType = newParameterType;
}
} else if (nextAnnotation instanceof EmbeddedOperationParams) {
final List<Class<?>> operationEmbeddedTypes =
ReflectionUtil.getMethodParamsWithClassesWithFieldsWithAnnotation(
methodToUse, EmbeddedOperationParam.class);
// LUKETODO: cleanup
// NEW
if (op == null) {
throw new ConfigurationException(Msg.code(846192641)
+ "@OperationParam or OperationEmbeddedParam detected on method that is not annotated with @Operation: "
+ methodToUse.toGenericString());
}

final List<Class<?>> embeddedParamsClasses = Arrays.stream(methodToUse.getParameterTypes())
.filter(paramType -> paramType.isAnnotationPresent(EmbeddableOperationParams.class))
.collect(Collectors.toUnmodifiableList());

// LUKETODO; better error?
if (embeddedParamsClasses.isEmpty()) {
throw new ConfigurationException(String.format(
"%sThere is no param with @EmbeddableOperationParams is supported for now for method: %s",
Msg.code(9999924), methodToUse.getName()));
}

// LUKETODO; better error?
if (embeddedParamsClasses.size() > 1) {
throw new ConfigurationException(String.format(
"%sMore than one param with with @EmbeddableOperationParams for method: %s",
Msg.code(9999927), methodToUse.getName()));
}

final Class<?> soleEmbeddedParamClass = embeddedParamsClasses.get(0);

final Constructor<?>[] constructorsForEmbeddableOperationParams =
soleEmbeddedParamClass.getConstructors();

// LUKETODO; better error?
if (constructorsForEmbeddableOperationParams.length == 0) {
throw new ConfigurationException(String.format(
"%sThere is no constructor with @EmbeddableOperationParams is supported for now for method: %s",
Msg.code(9999924), methodToUse.getName()));
}

// LUKETODO; better error?
if (constructorsForEmbeddableOperationParams.length > 1) {
throw new ConfigurationException(String.format(
"%sOnly one constructor with @EmbeddableOperationParams is supported but there is mulitple for method: %s",
Msg.code(9999927), methodToUse.getName()));
}

final Constructor<?> constructor = constructorsForEmbeddableOperationParams[0];

final Parameter[] constructorParams = constructor.getParameters();

for (Parameter constructorParam : constructorParams) {
final Annotation[] annotations = constructorParam.getAnnotations();

// LUKETODO: test
if (annotations.length == 0) {
throw new ConfigurationException(String.format(
"%s Constructor params have no annotation for embedded params class: %s and method: %s",
Msg.code(9999937),
constructor.getDeclaringClass().getName(),
methodToUse.getName()));
}

// LUKETODO: test
if (annotations.length > 1) {
throw new ConfigurationException(String.format(
"%s Constructor params have more than one annotation for embedded params: %s and method: %s",
Msg.code(9999947),
constructor.getDeclaringClass().getName(),
methodToUse.getName()));
}

final Annotation soleAnnotation = annotations[0];

IParameter innerParam = null;
if (soleAnnotation instanceof IdParam) {
// LUKETODO: we're missing this parameter when we build the method binding
innerParam = new NullParameter();
// Need to add this explicitly
parameters.add(innerParam);
} else if (soleAnnotation instanceof OperationParam) {
final OperationParam operationParam = (OperationParam) soleAnnotation;
final String description = ParametersUtil.extractDescription(annotations);
final List<String> examples = ParametersUtil.extractExamples(annotations);
final OperationParameter operationParameter = new OperationParameter(
theContext,
op.name(),
operationParam.name(),
operationParam.min(),
operationParam.max(),
description,
examples,
operationParam.sourceType(),
operationParam.rangeType());

final ParamInitializationContext paramContext =
buildParamContext(constructor, constructorParam, operationParameter);

innerParam = operationParameter;
paramContexts.add(paramContext);
}

param = innerParam;
}

// LUKETODO: need to disable the below code

// OLD
// final List<Class<?>> operationEmbeddedTypes =
// ReflectionUtil.getMethodParamsWithClassesWithFieldsWithAnnotation(
// methodToUse, EmbeddedOperationParam.class);

final List<Class<?>> operationEmbeddedTypes = List.of();

if (op == null) {
throw new ConfigurationException(Msg.code(846192641)
Expand Down Expand Up @@ -468,12 +580,6 @@ public Object outgoingClient(Object theObject) {
}
}

if (paramContexts.isEmpty() || !(param instanceof EmbeddedOperationParameter)) {
// RequestDetails if it's last
paramContexts.add(
new ParamInitializationContext(param, parameterType, outerCollectionType, innerCollectionType));
}

if (param == null) {
throw new ConfigurationException(
Msg.code(408) + "Parameter #" + (paramIndex + 1) + "/" + (parameterTypes.length)
Expand All @@ -482,6 +588,21 @@ public Object outgoingClient(Object theObject) {
+ "' has no recognized FHIR interface parameter nextParameterAnnotations. Don't know how to handle this parameter");
}

// LUKETODO: refactor into some sort of static method
// LUKETODO: comment that this is a guard against adding the entire embeddable parameter as an
// OperationParameter
// LUKETODO: find a better guard for this?

// LUKETODO: this doesn't work because the contexts are not empty

if (paramContexts.isEmpty()
|| Arrays.stream(parameterType.getAnnotations())
.noneMatch(annotation -> annotation instanceof EmbeddableOperationParams)) {
// RequestDetails if it's last
paramContexts.add(
new ParamInitializationContext(param, parameterType, outerCollectionType, innerCollectionType));
}

for (ParamInitializationContext paramContext : paramContexts) {
paramContext.initialize(methodToUse);
parameters.add(paramContext.getParam());
Expand All @@ -491,4 +612,57 @@ public Object outgoingClient(Object theObject) {
}
return parameters;
}

private static ParamInitializationContext buildParamContext(
Constructor<?> theConstructor, Parameter theConstructorParameter, OperationParameter theOperationParam) {

final Class<?> genericParameter =
ReflectionUtil.getGenericCollectionTypeOfConstructorParameter(theConstructor, theConstructorParameter);

Class<?> parameterType = theConstructorParameter.getType();
Class<? extends java.util.Collection<?>> outerCollectionType = null;
Class<? extends java.util.Collection<?>> innerCollectionType = null;

// Flat collection
if (Collection.class.isAssignableFrom(parameterType)) {
innerCollectionType = unsafeCast(parameterType);
parameterType = genericParameter;
if (parameterType == null) {
final String error = String.format(
"%s Cannot find generic type for field: %s in class: %s for constructor: %s",
Msg.code(724612469),
theConstructorParameter.getName(),
theConstructorParameter.getClass().getCanonicalName(),
theConstructor.getName());
throw new ConfigurationException(error);
}

// Collection of a Collection: Permitted
if (Collection.class.isAssignableFrom(parameterType)) {
outerCollectionType = innerCollectionType;
innerCollectionType = unsafeCast(parameterType);
}

// Collection of a Collection of a Collection: Prohibited
if (Collection.class.isAssignableFrom(parameterType)) {
final String error = String.format(
"%sInvalid generic type (a collection of a collection of a collection) for field: %s in class: %s for constructor: %s",
Msg.code(724612469),
theConstructorParameter.getName(),
theConstructorParameter.getClass().getCanonicalName(),
theConstructor.getName());
throw new ConfigurationException(error);
}
}

// TODO: LD: Don't worry about the OperationEmbeddedParam.type() for now until we chose to implement it later

return new ParamInitializationContext(
theOperationParam, parameterType, outerCollectionType, innerCollectionType);
}

@SuppressWarnings("unchecked")
private static <T> T unsafeCast(Object theObject) {
return (T) theObject;
}
}
Loading

0 comments on commit 92d40ae

Please sign in to comment.