Skip to content

Commit

Permalink
Merge pull request #799 from FgForrest/798-constraint-facetgroupsconj…
Browse files Browse the repository at this point in the history
…unction-might-be-legally-empty

fix: cannot use facetXXXGroupsXXX for reference without group type (i.e. with no parameters)
  • Loading branch information
novoj authored Feb 6, 2025
2 parents bb1135e + 7ff76d6 commit 0704f03
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
]
},
require: {
facetGroupsGroupsConjunction: { }
facetGroupsGroupsConjunction: true
}
) {
extraResults {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,6 @@ POST /rest/evita/Product/query
}
}
},
"facetGroupsGroupsConjunction" : { }
"facetGroupsGroupsConjunction" : true
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -191,14 +191,7 @@ public SIMPLE_TYPE build(@Nonnull DataLocator rootDataLocator, @Nonnull Constrai
*/
@Nonnull
protected SIMPLE_TYPE build(@Nonnull ConstraintBuildContext buildContext, @Nonnull ConstraintDescriptor constraintDescriptor) {
final SIMPLE_TYPE simpleType = buildConstraintValue(buildContext, constraintDescriptor, null);
// note: if nullable support is required at root level, the null value can be propagated up, but all cases that
// expect nonnull value must be handled
Assert.isPremiseValid(
simpleType != null,
() -> createSchemaBuildingError("Null constraint value as root constraint value not supported yet.")
);
return simpleType;
return buildConstraintValue(buildContext, constraintDescriptor, null);
}


Expand Down Expand Up @@ -745,12 +738,8 @@ protected OBJECT_FIELD buildFieldFromConstraintDescriptor(@Nonnull ConstraintBui
return null;
}

final SIMPLE_TYPE constraintValue = buildConstraintValue(buildContext, constraintDescriptor, valueTypeSupplier);
if (constraintValue == null) {
// no value (no usable parameter), parent constraint should be omitted as there is nothing to specify
return null;
}
final String constraintKey = keyBuilder.build(buildContext, constraintDescriptor, classifierSupplier);
final SIMPLE_TYPE constraintValue = buildConstraintValue(buildContext, constraintDescriptor, valueTypeSupplier);
return buildFieldFromConstraintDescriptor(constraintDescriptor, constraintKey, constraintValue);
}

Expand All @@ -775,7 +764,7 @@ protected abstract OBJECT_FIELD buildFieldFromConstraintDescriptor(@Nonnull Cons
* @param valueTypeSupplier supplies concrete value type for constraint values if value parameter has generic type
* @return input type representing the field value
*/
@Nullable
@Nonnull
protected SIMPLE_TYPE buildConstraintValue(@Nonnull ConstraintBuildContext buildContext,
@Nonnull ConstraintDescriptor constraintDescriptor,
@Nullable ValueTypeSupplier valueTypeSupplier) {
Expand Down Expand Up @@ -892,7 +881,7 @@ protected Optional<DataLocator> resolveChildDataLocator(@Nonnull ConstraintBuild
*
* If returns null, parent constraint should be omitted, because there are no valid parameters to specify.
*/
@Nullable
@Nonnull
protected SIMPLE_TYPE obtainWrapperObjectConstraintValue(@Nonnull ConstraintBuildContext buildContext,
@Nonnull List<ValueParameterDescriptor> valueParameters,
@Nonnull List<ChildParameterDescriptor> childParameters,
Expand Down Expand Up @@ -941,7 +930,7 @@ protected SIMPLE_TYPE obtainWrapperObjectConstraintValue(@Nonnull ConstraintBuil
*
* <b>Note:</b> this method should not be used directly, instead use {@link #obtainWrapperObjectConstraintValue(ConstraintBuildContext, List, List, List, ValueTypeSupplier)}.
*/
@Nullable
@Nonnull
protected abstract SIMPLE_TYPE buildWrapperObjectConstraintValue(@Nonnull ConstraintBuildContext buildContext,
@Nonnull WrapperObjectKey wrapperObjectKey,
@Nonnull List<ValueParameterDescriptor> valueParameters,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* | __/\ V /| | || (_| | |_| | |_) |
* \___| \_/ |_|\__\__,_|____/|____/
*
* Copyright (c) 2023-2024
* Copyright (c) 2023-2025
*
* Licensed under the Business Source License, Version 1.1 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -379,7 +379,7 @@ private Object extractValueParameterFromValue(@Nonnull ParsedConstraintDescripto
/**
* Should extract raw argument from wrapper object `value`.
*/
@Nonnull
@Nullable
private Object extractValueArgumentFromWrapperObject(@Nonnull ParsedConstraintDescriptor parsedConstraintDescriptor,
@Nonnull Object value,
@Nonnull ValueParameterDescriptor parameterDescriptor) {
Expand All @@ -402,17 +402,22 @@ private Object extractToArgumentFromWrapperRange(@Nonnull ParsedConstraintDescri
return extractRangeFromWrapperRange(parsedConstraintDescriptor, value).get(1);
}

@Nonnull
@Nullable
private Object extractArgumentFromWrapperObject(@Nonnull ParsedConstraintDescriptor parsedConstraintDescriptor,
@Nonnull Object value,
@Nonnull String parameterName) {
if (value instanceof Boolean) {
// wrapper object is empty (no parameters)
return null;
}

final Map<String, Object> wrapperObject;
try {
//noinspection unchecked
wrapperObject = (Map<String, Object>) value;
} catch (ClassCastException e) {
throw createQueryResolvingInternalError(
"Constraint `" + parsedConstraintDescriptor + "` expected to be wrapper object but found `" + value + "`."
"Constraint `" + parsedConstraintDescriptor.originalKey() + "` expected to be wrapper object but found `" + value + "`."
);
}
return wrapperObject.get(parameterName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@ protected Map<String, GraphQLInputType> buildChildConstraintValue(@Nonnull Const
return childTypes;
}

@Nullable
@Nonnull
@Override
protected GraphQLInputType buildWrapperObjectConstraintValue(@Nonnull ConstraintBuildContext buildContext,
@Nonnull WrapperObjectKey wrapperObjectKey,
Expand Down Expand Up @@ -332,8 +332,11 @@ protected GraphQLInputType buildWrapperObjectConstraintValue(@Nonnull Constraint
});

if (wrapperObjectFields.isEmpty()) {
// no fields, parent constraint should be omitted
return null;
// no fields (no usable parameters), there is nothing specific to specify, but we still want to be able to use
// the constraint without parameters
final GraphQLInputType emptyWrapperObject = buildNoneConstraintValue();
sharedContext.cacheWrapperObject(wrapperObjectKey, emptyWrapperObject);
return emptyWrapperObject;
}

final String wrapperObjectName = constructWrapperObjectName(wrapperObjectKey);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ protected Map<String, OpenApiSimpleType> buildChildConstraintValue(@Nonnull Cons

}

@Nullable
@Nonnull
@Override
protected OpenApiSimpleType buildWrapperObjectConstraintValue(@Nonnull ConstraintBuildContext buildContext,
@Nonnull WrapperObjectKey wrapperObjectKey,
Expand Down Expand Up @@ -335,8 +335,11 @@ protected OpenApiSimpleType buildWrapperObjectConstraintValue(@Nonnull Constrain
});

if (wrapperObjectFields.isEmpty()) {
// no fields, parent constraint should be omitted
return null;
// no fields (no usable parameters), there is nothing specific to specify, but we still want to be able to use
// the constraint without parameters
final OpenApiSimpleType emptyWrapperObject = buildNoneConstraintValue();
sharedContext.cacheWrapperObject(wrapperObjectKey, emptyWrapperObject);
return emptyWrapperObject;
}

final String wrapperObjectName = constructWrapperObjectName(wrapperObjectKey);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* | __/\ V /| | || (_| | |_| | |_) |
* \___| \_/ |_|\__\__,_|____/|____/
*
* Copyright (c) 2023
* Copyright (c) 2023-2025
*
* Licensed under the Business Source License, Version 1.1 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -409,6 +409,11 @@ private JsonNode convertWrapperObjectStructure(@Nonnull ConstraintToJsonConvertC
));
});

if (wrapperObject.isEmpty()) {
// there are no usable parameters, but the constraint is still valid
return convertNoneStructure();
}

return wrapperObject;
}

Expand Down

0 comments on commit 0704f03

Please sign in to comment.