Skip to content

Commit ac0c24d

Browse files
novojnovoj
authored
Merge pull request #770 from FgForrest/fix-facetXXXGroupsXXXjunction-constraint-target-reference-entity-type-instead-of-reference-group-type
Fix: facet groups conjunction constraint target reference entity type instead of reference group type
2 parents 5546f21 + 8d9e40c commit ac0c24d

File tree

13 files changed

+99
-77
lines changed

13 files changed

+99
-77
lines changed

‎evita_external_api/evita_external_api_core/src/main/java/io/evitadb/externalApi/api/catalog/dataApi/builder/constraint/ConstraintSchemaBuilder.java

+21-6
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* | __/\ V /| | || (_| | |_| | |_) |
77
* \___| \_/ |_|\__\__,_|____/|____/
88
*
9-
* Copyright (c) 2023-2024
9+
* Copyright (c) 2023-2025
1010
*
1111
* Licensed under the Business Source License, Version 1.1 (the "License");
1212
* you may not use this file except in compliance with the License.
@@ -191,7 +191,14 @@ public SIMPLE_TYPE build(@Nonnull DataLocator rootDataLocator, @Nonnull Constrai
191191
*/
192192
@Nonnull
193193
protected SIMPLE_TYPE build(@Nonnull ConstraintBuildContext buildContext, @Nonnull ConstraintDescriptor constraintDescriptor) {
194-
return buildConstraintValue(buildContext, constraintDescriptor, null);
194+
final SIMPLE_TYPE simpleType = buildConstraintValue(buildContext, constraintDescriptor, null);
195+
// note: if nullable support is required at root level, the null value can be propagated up, but all cases that
196+
// expect nonnull value must be handled
197+
Assert.isPremiseValid(
198+
simpleType != null,
199+
() -> createSchemaBuildingError("Null constraint value as root constraint value not supported yet.")
200+
);
201+
return simpleType;
195202
}
196203

197204

@@ -738,8 +745,12 @@ protected OBJECT_FIELD buildFieldFromConstraintDescriptor(@Nonnull ConstraintBui
738745
return null;
739746
}
740747

741-
final String constraintKey = keyBuilder.build(buildContext, constraintDescriptor, classifierSupplier);
742748
final SIMPLE_TYPE constraintValue = buildConstraintValue(buildContext, constraintDescriptor, valueTypeSupplier);
749+
if (constraintValue == null) {
750+
// no value (no usable parameter), parent constraint should be omitted as there is nothing to specify
751+
return null;
752+
}
753+
final String constraintKey = keyBuilder.build(buildContext, constraintDescriptor, classifierSupplier);
743754
return buildFieldFromConstraintDescriptor(constraintDescriptor, constraintKey, constraintValue);
744755
}
745756

@@ -764,7 +775,7 @@ protected abstract OBJECT_FIELD buildFieldFromConstraintDescriptor(@Nonnull Cons
764775
* @param valueTypeSupplier supplies concrete value type for constraint values if value parameter has generic type
765776
* @return input type representing the field value
766777
*/
767-
@Nonnull
778+
@Nullable
768779
protected SIMPLE_TYPE buildConstraintValue(@Nonnull ConstraintBuildContext buildContext,
769780
@Nonnull ConstraintDescriptor constraintDescriptor,
770781
@Nullable ValueTypeSupplier valueTypeSupplier) {
@@ -878,8 +889,10 @@ protected Optional<DataLocator> resolveChildDataLocator(@Nonnull ConstraintBuild
878889
/**
879890
* Returns field value representing constraint value of constraint with multiple creator value parameters or
880891
* combination of value and child parameters, either from cache or newly built one.
892+
*
893+
* If returns null, parent constraint should be omitted, because there are no valid parameters to specify.
881894
*/
882-
@Nonnull
895+
@Nullable
883896
protected SIMPLE_TYPE obtainWrapperObjectConstraintValue(@Nonnull ConstraintBuildContext buildContext,
884897
@Nonnull List<ValueParameterDescriptor> valueParameters,
885898
@Nonnull List<ChildParameterDescriptor> childParameters,
@@ -924,9 +937,11 @@ protected SIMPLE_TYPE obtainWrapperObjectConstraintValue(@Nonnull ConstraintBuil
924937
* combination of value and child parameters.
925938
* Implementation should cache the built objects for later reuse.
926939
*
940+
* If returns null, parent constraint should be omitted, because there are no valid parameters to specify.
941+
*
927942
* <b>Note:</b> this method should not be used directly, instead use {@link #obtainWrapperObjectConstraintValue(ConstraintBuildContext, List, List, List, ValueTypeSupplier)}.
928943
*/
929-
@Nonnull
944+
@Nullable
930945
protected abstract SIMPLE_TYPE buildWrapperObjectConstraintValue(@Nonnull ConstraintBuildContext buildContext,
931946
@Nonnull WrapperObjectKey wrapperObjectKey,
932947
@Nonnull List<ValueParameterDescriptor> valueParameters,

‎evita_external_api/evita_external_api_core/src/main/java/io/evitadb/externalApi/api/catalog/dataApi/constraint/DataLocatorResolver.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* | __/\ V /| | || (_| | |_| | |_) |
77
* \___| \_/ |_|\__\__,_|____/|____/
88
*
9-
* Copyright (c) 2023-2024
9+
* Copyright (c) 2023-2025
1010
*
1111
* Licensed under the Business Source License, Version 1.1 (the "License");
1212
* you may not use this file except in compliance with the License.
@@ -177,7 +177,7 @@ public Optional<DataLocator> resolveChildParameterDataLocator(@Nonnull DataLocat
177177
@Nonnull
178178
public DataLocator resolveConstraintDataLocator(@Nonnull DataLocator parentDataLocator,
179179
@Nonnull ConstraintDescriptor constraintDescriptor,
180-
@Nonnull Optional<String> classifier) {
180+
@SuppressWarnings("OptionalUsedAsFieldOrParameterType") @Nonnull Optional<String> classifier) {
181181
return switch (constraintDescriptor.propertyType()) {
182182
case GENERIC, ATTRIBUTE, ASSOCIATED_DATA, PRICE -> parentDataLocator; // these property type currently doesn't have any container constraints
183183
case ENTITY -> {

‎evita_external_api/evita_external_api_core/src/main/java/io/evitadb/externalApi/api/catalog/dataApi/resolver/constraint/ConstraintDescriptorResolver.java

+4-2
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* | __/\ V /| | || (_| | |_| | |_) |
77
* \___| \_/ |_|\__\__,_|____/|____/
88
*
9-
* Copyright (c) 2023-2024
9+
* Copyright (c) 2023-2025
1010
*
1111
* Licensed under the Business Source License, Version 1.1 (the "License");
1212
* you may not use this file except in compliance with the License.
@@ -153,6 +153,7 @@ private Optional<String> constructClassifier(@Nonnull Deque<String> classifierWo
153153
if (classifierWords.isEmpty()) {
154154
return Optional.empty();
155155
}
156+
//noinspection DataFlowIssue
156157
return Optional.of(
157158
StringUtils.uncapitalize(String.join("", classifierWords))
158159
);
@@ -164,6 +165,7 @@ private String constructFullName(@Nonnull Deque<String> fullNameWords) {
164165
!fullNameWords.isEmpty(),
165166
() -> new ExternalApiInternalError("Full name cannot be empty.")
166167
);
168+
//noinspection DataFlowIssue
167169
return StringUtils.uncapitalize(String.join("", fullNameWords));
168170
}
169171

@@ -174,7 +176,7 @@ private String constructFullName(@Nonnull Deque<String> fullNameWords) {
174176
@Nonnull
175177
private Optional<String> resolveActualClassifier(@Nonnull ConstraintResolveContext resolveContext,
176178
@Nonnull ConstraintDescriptor constraintDescriptor,
177-
@Nonnull Optional<String> rawClassifier) {
179+
@SuppressWarnings("OptionalUsedAsFieldOrParameterType") @Nonnull Optional<String> rawClassifier) {
178180
return rawClassifier.map(c -> {
179181
final Optional<ImplicitClassifier> implicitClassifier = constraintDescriptor.creator().implicitClassifier();
180182
if (implicitClassifier.isPresent()) {

‎evita_external_api/evita_external_api_graphql/src/main/java/io/evitadb/externalApi/graphql/api/catalog/dataApi/builder/constraint/GraphQLConstraintSchemaBuilder.java

+20-11
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* | __/\ V /| | || (_| | |_| | |_) |
77
* \___| \_/ |_|\__\__,_|____/|____/
88
*
9-
* Copyright (c) 2023-2024
9+
* Copyright (c) 2023-2025
1010
*
1111
* Licensed under the Business Source License, Version 1.1 (the "License");
1212
* you may not use this file except in compliance with the License.
@@ -52,6 +52,7 @@
5252
import javax.annotation.Nonnull;
5353
import javax.annotation.Nullable;
5454
import java.io.Serializable;
55+
import java.util.ArrayList;
5556
import java.util.Collection;
5657
import java.util.Currency;
5758
import java.util.LinkedList;
@@ -257,19 +258,15 @@ protected Map<String, GraphQLInputType> buildChildConstraintValue(@Nonnull Const
257258
return childTypes;
258259
}
259260

260-
@Nonnull
261+
@Nullable
261262
@Override
262263
protected GraphQLInputType buildWrapperObjectConstraintValue(@Nonnull ConstraintBuildContext buildContext,
263264
@Nonnull WrapperObjectKey wrapperObjectKey,
264265
@Nonnull List<ValueParameterDescriptor> valueParameters,
265266
@Nonnull List<ChildParameterDescriptor> childParameters,
266267
@Nonnull List<AdditionalChildParameterDescriptor> additionalChildParameters,
267268
@Nullable ValueTypeSupplier valueTypeSupplier) {
268-
final String wrapperObjectName = constructWrapperObjectName(wrapperObjectKey);
269-
final GraphQLInputObjectType.Builder wrapperObjectBuilder = newInputObject().name(wrapperObjectName);
270-
final GraphQLInputType wrapperObjectPointer = typeRef(wrapperObjectName);
271-
// cache wrapper object for reuse
272-
sharedContext.cacheWrapperObject(wrapperObjectKey, wrapperObjectPointer);
269+
final List<GraphQLInputObjectField.Builder> wrapperObjectFields = new ArrayList<>(valueParameters.size() + childParameters.size() + additionalChildParameters.size());
273270

274271
// build primitive values
275272
for (ValueParameterDescriptor valueParameter : valueParameters) {
@@ -279,7 +276,7 @@ protected GraphQLInputType buildWrapperObjectConstraintValue(@Nonnull Constraint
279276
!valueParameter.type().isArray(), // we want treat missing arrays as empty arrays for more client convenience
280277
valueTypeSupplier
281278
);
282-
wrapperObjectBuilder.field(f -> f
279+
wrapperObjectFields.add(newInputObjectField()
283280
.name(valueParameter.name())
284281
.type(nestedPrimitiveConstraintValue));
285282
}
@@ -298,7 +295,7 @@ protected GraphQLInputType buildWrapperObjectConstraintValue(@Nonnull Constraint
298295
nestedChildConstraintValue = nonNull(nestedChildConstraintValue);
299296
}
300297

301-
wrapperObjectBuilder.field(newInputObjectField()
298+
wrapperObjectFields.add(newInputObjectField()
302299
.name(childParameter.name())
303300
.type(nestedChildConstraintValue));
304301
} else {
@@ -316,7 +313,7 @@ protected GraphQLInputType buildWrapperObjectConstraintValue(@Nonnull Constraint
316313
) {
317314
nestedChildConstraint.setValue(nonNull(nestedChildConstraint.getValue()));
318315
}
319-
wrapperObjectBuilder.field(newInputObjectField()
316+
wrapperObjectFields.add(newInputObjectField()
320317
.name(key)
321318
.type(nestedChildConstraint.getValue()));
322319
});
@@ -332,12 +329,24 @@ protected GraphQLInputType buildWrapperObjectConstraintValue(@Nonnull Constraint
332329
nestedAdditionalChildConstraintValue = nonNull(nestedAdditionalChildConstraintValue);
333330
}
334331

335-
wrapperObjectBuilder.field(newInputObjectField()
332+
wrapperObjectFields.add(newInputObjectField()
336333
.name(additionalChildParameter.name())
337334
.type(nestedAdditionalChildConstraintValue));
338335
});
339336
});
340337

338+
if (wrapperObjectFields.isEmpty()) {
339+
// no fields, parent constraint should be omitted
340+
return null;
341+
}
342+
343+
final String wrapperObjectName = constructWrapperObjectName(wrapperObjectKey);
344+
final GraphQLInputObjectType.Builder wrapperObjectBuilder = newInputObject().name(wrapperObjectName);
345+
wrapperObjectFields.forEach(wrapperObjectBuilder::field);
346+
final GraphQLInputType wrapperObjectPointer = typeRef(wrapperObjectName);
347+
// cache wrapper object for reuse
348+
sharedContext.cacheWrapperObject(wrapperObjectKey, wrapperObjectPointer);
349+
341350
sharedContext.addNewType(wrapperObjectBuilder.build());
342351
return wrapperObjectPointer;
343352
}

‎evita_external_api/evita_external_api_rest/src/main/java/io/evitadb/externalApi/rest/api/catalog/dataApi/builder/constraint/OpenApiConstraintSchemaBuilder.java

+20-11
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* | __/\ V /| | || (_| | |_| | |_) |
77
* \___| \_/ |_|\__\__,_|____/|____/
88
*
9-
* Copyright (c) 2023-2024
9+
* Copyright (c) 2023-2025
1010
*
1111
* Licensed under the Business Source License, Version 1.1 (the "License");
1212
* you may not use this file except in compliance with the License.
@@ -53,6 +53,7 @@
5353
import javax.annotation.Nonnull;
5454
import javax.annotation.Nullable;
5555
import java.io.Serializable;
56+
import java.util.ArrayList;
5657
import java.util.Collection;
5758
import java.util.Currency;
5859
import java.util.LinkedList;
@@ -260,19 +261,15 @@ protected Map<String, OpenApiSimpleType> buildChildConstraintValue(@Nonnull Cons
260261

261262
}
262263

263-
@Nonnull
264+
@Nullable
264265
@Override
265266
protected OpenApiSimpleType buildWrapperObjectConstraintValue(@Nonnull ConstraintBuildContext buildContext,
266267
@Nonnull WrapperObjectKey wrapperObjectKey,
267268
@Nonnull List<ValueParameterDescriptor> valueParameters,
268269
@Nonnull List<ChildParameterDescriptor> childParameters,
269270
@Nonnull List<AdditionalChildParameterDescriptor> additionalChildParameters,
270271
@Nullable ValueTypeSupplier valueTypeSupplier) {
271-
final String wrapperObjectName = constructWrapperObjectName(wrapperObjectKey);
272-
final OpenApiObject.Builder wrapperObjectBuilder = newObject().name(wrapperObjectName);
273-
final OpenApiTypeReference wrapperObjectPointer = typeRefTo(wrapperObjectName);
274-
// cache wrapper object for reuse
275-
sharedContext.cacheWrapperObject(wrapperObjectKey, wrapperObjectPointer);
272+
final List<OpenApiProperty.Builder> wrapperObjectFields = new ArrayList<>(valueParameters.size() + childParameters.size() + additionalChildParameters.size());
276273

277274
// build primitive values
278275
valueParameters.forEach(valueParameter -> {
@@ -282,7 +279,7 @@ protected OpenApiSimpleType buildWrapperObjectConstraintValue(@Nonnull Constrain
282279
!valueParameter.type().isArray(),
283280
valueTypeSupplier
284281
);
285-
wrapperObjectBuilder.property(p -> p
282+
wrapperObjectFields.add(newProperty()
286283
.name(valueParameter.name())
287284
.type(nestedPrimitiveConstraintValue));
288285
});
@@ -301,7 +298,7 @@ protected OpenApiSimpleType buildWrapperObjectConstraintValue(@Nonnull Constrain
301298
nestedChildConstraintValue = nonNull(nestedChildConstraintValue);
302299
}
303300

304-
wrapperObjectBuilder.property(newProperty()
301+
wrapperObjectFields.add(newProperty()
305302
.name(childParameter.name())
306303
.type(nestedChildConstraintValue));
307304
} else {
@@ -319,7 +316,7 @@ protected OpenApiSimpleType buildWrapperObjectConstraintValue(@Nonnull Constrain
319316
) {
320317
nestedChildConstraint.setValue(nonNull(nestedChildConstraint.getValue()));
321318
}
322-
wrapperObjectBuilder.property(newProperty()
319+
wrapperObjectFields.add(newProperty()
323320
.name(key)
324321
.type(nestedChildConstraint.getValue()));
325322
});
@@ -335,12 +332,24 @@ protected OpenApiSimpleType buildWrapperObjectConstraintValue(@Nonnull Constrain
335332
nestedAdditionalChildConstraintValue = nonNull(nestedAdditionalChildConstraintValue);
336333
}
337334

338-
wrapperObjectBuilder.property(newProperty()
335+
wrapperObjectFields.add(newProperty()
339336
.name(additionalChildParameter.name())
340337
.type(nestedAdditionalChildConstraintValue));
341338
});
342339
});
343340

341+
if (wrapperObjectFields.isEmpty()) {
342+
// no fields, parent constraint should be omitted
343+
return null;
344+
}
345+
346+
final String wrapperObjectName = constructWrapperObjectName(wrapperObjectKey);
347+
final OpenApiObject.Builder wrapperObjectBuilder = newObject().name(wrapperObjectName);
348+
wrapperObjectFields.forEach(wrapperObjectBuilder::property);
349+
final OpenApiTypeReference wrapperObjectPointer = typeRefTo(wrapperObjectName);
350+
// cache wrapper object for reuse
351+
sharedContext.cacheWrapperObject(wrapperObjectKey, wrapperObjectPointer);
352+
344353
sharedContext.addNewType(wrapperObjectBuilder.build());
345354
return wrapperObjectPointer;
346355
}

‎evita_functional_tests/src/test/java/io/evitadb/externalApi/graphql/api/catalog/dataApi/resolver/constraint/AbstractConstraintResolverTest.java

+6-3
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* | __/\ V /| | || (_| | |_| | |_) |
77
* \___| \_/ |_|\__\__,_|____/|____/
88
*
9-
* Copyright (c) 2023
9+
* Copyright (c) 2023-2025
1010
*
1111
* Licensed under the Business Source License, Version 1.1 (the "License");
1212
* you may not use this file except in compliance with the License.
@@ -96,8 +96,11 @@ public Optional<EntitySchemaContract> getEntitySchema(@Nonnull String entityType
9696
.withPrice()
9797
.withAttribute("CODE", String.class)
9898
.withAttribute("AGE", Integer.class, thatIs -> thatIs.filterable())
99-
.withReferenceToEntity(Entities.CATEGORY, Entities.CATEGORY, Cardinality.ONE_OR_MORE, thatIs -> thatIs.withAttribute("CODE", String.class))
100-
.withReferenceToEntity(Entities.BRAND, Entities.BRAND, Cardinality.EXACTLY_ONE)
99+
.withReferenceToEntity(Entities.CATEGORY, Entities.CATEGORY, Cardinality.ONE_OR_MORE, thatIs -> thatIs
100+
.withAttribute("CODE", String.class)
101+
.withGroupType("categoryGroup"))
102+
.withReferenceToEntity(Entities.BRAND, Entities.BRAND, Cardinality.EXACTLY_ONE, thatIs -> thatIs
103+
.withGroupType("brandGroup"))
101104
.toInstance();
102105

103106
entitySchemaIndex.put(Entities.PRODUCT, productSchema);

‎evita_functional_tests/src/test/java/io/evitadb/externalApi/graphql/api/catalog/dataApi/resolver/constraint/RequireConstraintResolverTest.java

+4-15
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* | __/\ V /| | || (_| | |_| | |_) |
77
* \___| \_/ |_|\__\__,_|____/|____/
88
*
9-
* Copyright (c) 2023-2024
9+
* Copyright (c) 2023-2025
1010
*
1111
* Licensed under the Business Source License, Version 1.1 (the "License");
1212
* you may not use this file except in compliance with the License.
@@ -152,10 +152,7 @@ void shouldResolveComplexRequireAndFilterOutUndefinedConstraints() {
152152
facetGroupsDisjunction(
153153
"CATEGORY",
154154
filterBy(
155-
or(
156-
attributeEquals("NAME", "apple"),
157-
attributeStartsWith("NAME", "sam")
158-
)
155+
entityPrimaryKeyInSet(123)
159156
)
160157
)
161158
),
@@ -168,16 +165,8 @@ void shouldResolveComplexRequireAndFilterOutUndefinedConstraints() {
168165
.e("priceType", null)
169166
.e("facetCategoryGroupsDisjunction", map()
170167
.e("filterBy", map()
171-
.e("or", list()
172-
.i(map()
173-
.e("attributeNameEquals", "apple")
174-
.e("and", null))
175-
.i(map()
176-
.e("or", list()
177-
.i(map()
178-
.e("attributeNameStartsWith", "sam"))
179-
.i(map()
180-
.e("attributeNameEndsWith", null)))))))
168+
.e("entityPrimaryKeyInSet", list()
169+
.i(123))))
181170
.build()
182171
)
183172
)

‎evita_functional_tests/src/test/java/io/evitadb/externalApi/rest/api/catalog/dataApi/resolver/constraint/AbstractConstraintResolverTest.java

+6-3
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* | __/\ V /| | || (_| | |_| | |_) |
77
* \___| \_/ |_|\__\__,_|____/|____/
88
*
9-
* Copyright (c) 2023-2024
9+
* Copyright (c) 2023-2025
1010
*
1111
* Licensed under the Business Source License, Version 1.1 (the "License");
1212
* you may not use this file except in compliance with the License.
@@ -98,8 +98,11 @@ public Optional<EntitySchemaContract> getEntitySchema(@Nonnull String entityType
9898
.withPrice()
9999
.withAttribute("CODE", String.class)
100100
.withAttribute("AGE", Integer.class, thatIs -> thatIs.filterable())
101-
.withReferenceToEntity(Entities.CATEGORY, Entities.CATEGORY, Cardinality.ONE_OR_MORE, thatIs -> thatIs.withAttribute("CODE", String.class))
102-
.withReferenceToEntity(Entities.BRAND, Entities.BRAND, Cardinality.EXACTLY_ONE)
101+
.withReferenceToEntity(Entities.CATEGORY, Entities.CATEGORY, Cardinality.ONE_OR_MORE, thatIs -> thatIs
102+
.withAttribute("CODE", String.class)
103+
.withGroupType("categoryGroup"))
104+
.withReferenceToEntity(Entities.BRAND, Entities.BRAND, Cardinality.EXACTLY_ONE, thatIs -> thatIs
105+
.withGroupType("brandGroup"))
103106
.toInstance();
104107

105108
entitySchemaIndex.put(Entities.PRODUCT, productSchema);

0 commit comments

Comments
 (0)