Skip to content

Commit

Permalink
fix resolution of MoneyRange, move it into FilterValue (#3600)
Browse files Browse the repository at this point in the history
fix resolution of MoneyRange, move it into FilterValue

---------

Co-authored-by: MT <12283268+thoniTUB@users.noreply.github.com>
  • Loading branch information
awildturtok and thoniTUB authored Oct 15, 2024
1 parent 0b025f4 commit 985bf94
Show file tree
Hide file tree
Showing 7 changed files with 91 additions and 59 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
import com.bakdata.conquery.io.cps.CPSType;
import com.bakdata.conquery.models.common.Range;
import com.bakdata.conquery.models.common.Range.LongRange;
import com.bakdata.conquery.models.common.Range.MoneyRange;
import com.bakdata.conquery.models.config.ConqueryConfig;
import com.bakdata.conquery.models.datasets.concepts.filters.Filter;
import com.bakdata.conquery.models.identifiable.ids.specific.FilterId;
import com.bakdata.conquery.models.query.QueryResolveContext;
Expand All @@ -19,44 +21,51 @@
import com.bakdata.conquery.sql.conversion.cqelement.concept.FilterContext;
import com.bakdata.conquery.sql.conversion.model.SqlIdColumns;
import com.bakdata.conquery.sql.conversion.model.filter.SqlFilters;
import com.fasterxml.jackson.annotation.JacksonInject;
import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonIgnore;
import com.fasterxml.jackson.annotation.JsonTypeInfo;
import com.fasterxml.jackson.annotation.OptBoolean;
import lombok.AllArgsConstructor;
import lombok.Data;
import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.NoArgsConstructor;
import lombok.RequiredArgsConstructor;
import lombok.Setter;
import lombok.ToString;
import org.jetbrains.annotations.TestOnly;
import org.jooq.Condition;

@Getter
@Setter
@RequiredArgsConstructor
@NoArgsConstructor
/**
* @implNote The {@link JsonCreator} annos are necessary. Otherwise, Jackson will deserilaize all values as generic objects.
*/
@JsonTypeInfo(use = JsonTypeInfo.Id.CUSTOM, property = "type")
@CPSBase
@EqualsAndHashCode
@ToString(of = "value")
@Data
@AllArgsConstructor
public abstract class FilterValue<VALUE> {

@NotNull
@Nonnull
@ToString.Exclude
private FilterId filter;

@NotNull
@Nonnull
private VALUE value;

private Object value;

public void resolve(QueryResolveContext context) {
}

public FilterNode<?> createNode() {
final Filter<VALUE> resolve = (Filter<VALUE>) getFilter().resolve();
return resolve.createFilterNode(getValue());
return resolve.createFilterNode(readValue());
}

public VALUE readValue() {
return ((VALUE) value);
}

public SqlFilters convertToSqlFilter(SqlIdColumns ids, ConversionContext context, ConnectorSqlTables tables) {
FilterContext<VALUE> filterContext = FilterContext.forConceptConversion(ids, value, context, tables);
FilterContext<VALUE> filterContext = FilterContext.forConceptConversion(ids, readValue(), context, tables);
final Filter<VALUE> resolve = (Filter<VALUE>) filter.resolve();
SqlFilters sqlFilters = resolve.createConverter().convertToSqlFilter(resolve, filterContext);
if (context.isNegation()) {
Expand All @@ -66,85 +75,90 @@ public SqlFilters convertToSqlFilter(SqlIdColumns ids, ConversionContext context
}

public Condition convertForTableExport(SqlIdColumns ids, ConversionContext context) {
FilterContext<VALUE> filterContext = FilterContext.forTableExport(ids, value, context);
FilterContext<VALUE> filterContext = FilterContext.forTableExport(ids, readValue(), context);
final Filter<VALUE> resolve = (Filter<VALUE>) filter.resolve();
return resolve.createConverter().convertForTableExport(resolve, filterContext);
}

@NoArgsConstructor
@CPSType(id = FrontendFilterType.Fields.MULTI_SELECT, base = FilterValue.class)
public static class CQMultiSelectFilter extends FilterValue<Set<String>> {
@JsonCreator
public CQMultiSelectFilter(FilterId filter, Set<String> value) {
super(filter, value);
}

@Override
public String toString() {
final String valueString;
if (getValue().size() > 20) {
valueString = getValue().size() + " values";
final int size = readValue().size();

if (size > 20) {
valueString = size + " values";
}
else {
valueString = getValue().toString();
valueString = readValue().toString();
}

return "%s(value=%s)".formatted(FrontendFilterType.Fields.BIG_MULTI_SELECT, valueString);
return "%s(value=%s)".formatted(FrontendFilterType.Fields.MULTI_SELECT, valueString);
}

}

@NoArgsConstructor
@CPSType(id = FrontendFilterType.Fields.BIG_MULTI_SELECT, base = FilterValue.class)
public static class CQBigMultiSelectFilter extends FilterValue<Set<String>> {

@JsonCreator
public CQBigMultiSelectFilter(FilterId filter, Set<String> value) {
super(filter, value);
}

@Override
public String toString() {
final String valueString;
if (getValue().size() > 20) {
valueString = getValue().size() + " values";
final int size = readValue().size();

if (size > 20) {
valueString = size + " values";
}
else {
valueString = getValue().toString();
valueString = readValue().toString();
}

return "%s(value=%s)".formatted(FrontendFilterType.Fields.BIG_MULTI_SELECT, valueString);
}
}

@NoArgsConstructor
@CPSType(id = FrontendFilterType.Fields.SELECT, base = FilterValue.class)
@ToString(callSuper = true)
public static class CQSelectFilter extends FilterValue<String> {
@JsonCreator
public CQSelectFilter(FilterId filter, String value) {
super(filter, value);
}
}

@NoArgsConstructor
@CPSType(id = FrontendFilterType.Fields.STRING, base = FilterValue.class)
@ToString(callSuper = true)
public static class CQStringFilter extends FilterValue<String> {
@JsonCreator
public CQStringFilter(FilterId filter, String value) {
super(filter, value);
}
}

@NoArgsConstructor
@CPSType(id = FrontendFilterType.Fields.INTEGER, base = FilterValue.class)
@ToString(callSuper = true)
public static class CQIntegerFilter extends FilterValue<Long> {
@JsonCreator
public CQIntegerFilter(FilterId filter, Long value) {
super(filter, value);
}
}

@NoArgsConstructor
@CPSType(id = FrontendFilterType.Fields.INTEGER_RANGE, base = FilterValue.class)
@ToString(callSuper = true)
public static class CQIntegerRangeFilter extends FilterValue<LongRange> {
@JsonCreator
public CQIntegerRangeFilter(FilterId filter, LongRange value) {
super(filter, value);
}
Expand All @@ -154,31 +168,45 @@ public CQIntegerRangeFilter(FilterId filter, LongRange value) {
* @implNote Is basically the same as INTEGER_RANGE, but when a deserialized MONEY_RANGE was serialized again
* it became an INTEGER_RANGE, which is handled differently by the frontend.
*/
@NoArgsConstructor
@CPSType(id = FrontendFilterType.Fields.MONEY_RANGE, base = FilterValue.class)
@ToString(callSuper = true)
public static class CQMoneyRangeFilter extends FilterValue<LongRange> {
public static class CQMoneyRangeFilter extends FilterValue<MoneyRange> {

@JsonIgnore
@JacksonInject(useInput = OptBoolean.FALSE)
@NotNull
@EqualsAndHashCode.Exclude
@Setter(onMethod_ = {@TestOnly})
private ConqueryConfig config;

@JsonCreator
public CQMoneyRangeFilter(FilterId filter, LongRange value) {
super(filter, value);
}

@Override
public MoneyRange readValue() {
return MoneyRange.fromNumberRange((LongRange) getValue(), config.getFrontend().getCurrency());
}
}


@NoArgsConstructor
@CPSType(id = FrontendFilterType.Fields.REAL, base = FilterValue.class)
@ToString(callSuper = true)
public static class CQRealFilter extends FilterValue<BigDecimal> {
public static class CQRealFilter extends FilterValue<Double> {
@JsonCreator
public CQRealFilter(FilterId filter, BigDecimal value) {
super(filter, value);
}
}

@NoArgsConstructor
@CPSType(id = FrontendFilterType.Fields.REAL_RANGE, base = FilterValue.class)
@ToString(callSuper = true)
public static class CQRealRangeFilter extends FilterValue<Range<BigDecimal>> {
@JsonCreator
public CQRealRangeFilter(FilterId filter, Range<BigDecimal> value) {
super(filter, value);
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -61,31 +61,16 @@ public void configureFrontend(FrontendFilterConfiguration.Top f, ConqueryConfig
public FilterNode<?> createFilterNode(RANGE value) {
final Column column = getColumn().resolve();
final MajorTypeId typeId = column.getType();
final IRange<? extends Number, ?> range = readFilterValue(value, typeId, config);

return switch (typeId) {
case MONEY -> new MoneyFilterNode(column, (Range.MoneyRange) range);
case INTEGER -> new IntegerFilterNode(column, (Range.LongRange) range);
case DECIMAL -> new DecimalFilterNode(column, (Range<BigDecimal>) range);
case REAL -> new RealFilterNode(column, (Range.DoubleRange) range);

case MONEY -> new MoneyFilterNode(column, (Range.MoneyRange) value);
case INTEGER -> new IntegerFilterNode(column, (Range.LongRange) value);
case DECIMAL -> new DecimalFilterNode(column, (Range<BigDecimal>) value);
case REAL -> new RealFilterNode(column, Range.DoubleRange.fromNumberRange(value));
default -> throw new IllegalStateException(String.format("Column type %s may not be used (Assignment should not have been possible)", column));
};
}

/**
* This method only exists because we messed up and never implemented a DECIMAL_RANGE, otherwise it could be embedded in the FilterValues themselves.
*/
public static IRange<? extends Number, ?> readFilterValue(IRange<? extends Number, ?> value, @NotNull MajorTypeId type, @NotNull ConqueryConfig config) {
return switch (type) {
case MONEY -> Range.MoneyRange.fromNumberRange(value, config.getFrontend().getCurrency());
case INTEGER -> (Range.LongRange) value;
case DECIMAL -> ((Range<BigDecimal>) value);
case REAL -> Range.DoubleRange.fromNumberRange(value);
default -> throw new IllegalStateException(String.format("Column type %s may not be used (Assignment should not have been possible)", type));
};
}

@Override
public FilterConverter<? extends NumberFilter<RANGE>, RANGE> createConverter() {
return new NumberFilterConverter<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ public List<ColumnId> getRequiredColumns() {
public FilterNode createFilterNode(RANGE value) {
IRange<? extends Number, ?> range = value;

// Double values are parsed as BigDecimal, we convert to double if necessary
// Real and Decimal share FilterValue
if (getColumn().resolve().getType() == MajorTypeId.REAL) {
range = Range.DoubleRange.fromNumberRange(value);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public SqlFilters convertToSqlFilter(NumberFilter<RANGE> filter, FilterContext<R
ExtractingSqlSelect<? extends Number> rootSelect = new ExtractingSqlSelect<>(tables.getRootTable(), column.getName(), numberClass);

Field<? extends Number> eventFilterCtePredecessor = rootSelect.qualify(tables.getPredecessor(ConceptCteStep.EVENT_FILTER)).select();
IRange<? extends Number, ?> filterValue = NumberFilter.readFilterValue(filterContext.getValue(), column.getType(), filter.getConfig());
IRange<? extends Number, ?> filterValue = filterContext.getValue();
NumberCondition condition = new NumberCondition(eventFilterCtePredecessor, filterValue);

ConnectorSqlSelects selects = ConnectorSqlSelects.builder().preprocessingSelects(List.of(rootSelect)).build();
Expand All @@ -43,7 +43,7 @@ public Condition convertForTableExport(NumberFilter<RANGE> filter, FilterContext
String tableName = column.getTable().getName();
String columnName = column.getName();
Field<Number> field = DSL.field(DSL.name(tableName, columnName), Number.class);
IRange<? extends Number, ?> range = NumberFilter.readFilterValue(filterContext.getValue(), column.getType(), filter.getConfig());
IRange<? extends Number, ?> range = filterContext.getValue();

return new NumberCondition(field, range).condition();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.bakdata.conquery.apiv1.query.ConceptQuery;
import com.bakdata.conquery.apiv1.query.QueryDescription;
import com.bakdata.conquery.apiv1.query.concept.filter.CQTable;
import com.bakdata.conquery.apiv1.query.concept.filter.FilterValue;
import com.bakdata.conquery.apiv1.query.concept.specific.CQConcept;
import com.bakdata.conquery.apiv1.query.concept.specific.CQOr;
import com.bakdata.conquery.io.AbstractSerializationTest;
Expand Down Expand Up @@ -76,6 +77,7 @@
import com.bakdata.conquery.models.i18n.I18n;
import com.bakdata.conquery.models.identifiable.IdMapSerialisationTest;
import com.bakdata.conquery.models.identifiable.ids.specific.DatasetId;
import com.bakdata.conquery.models.identifiable.ids.specific.FilterId;
import com.bakdata.conquery.models.identifiable.ids.specific.GroupId;
import com.bakdata.conquery.models.identifiable.ids.specific.ManagedExecutionId;
import com.bakdata.conquery.models.identifiable.mapping.EntityIdMap;
Expand Down Expand Up @@ -249,7 +251,8 @@ public void bucketCompoundDateRange() throws JSONException, IOException {
ColumnStore startStore = new IntegerDateStore(new ShortArrayStore(new short[]{1, 2, 3, 4}, Short.MIN_VALUE));
ColumnStore endStore = new IntegerDateStore(new ShortArrayStore(new short[]{5, 6, 7, 8}, Short.MIN_VALUE));

Bucket bucket = new Bucket(0, Object2IntMaps.singleton("0", 0), Object2IntMaps.singleton("0", 4), 4,imp.getId(), new ColumnStore[]{startStore, endStore, compoundStore});
Bucket bucket =
new Bucket(0, Object2IntMaps.singleton("0", 0), Object2IntMaps.singleton("0", 4), 4, imp.getId(), new ColumnStore[]{startStore, endStore, compoundStore});

compoundStore.setParent(bucket);

Expand Down Expand Up @@ -328,6 +331,20 @@ public void table() throws JSONException, IOException {
return table;
}

@Test
public void filterValueMoneyRange() throws JSONException, IOException {
FilterValue.CQMoneyRangeFilter filterValue =
new FilterValue.CQMoneyRangeFilter(FilterId.Parser.INSTANCE.parse("dataset.concept.connector.filter"), new Range.LongRange(2000L, 30000L));

filterValue.setConfig(getConfig());


SerializationTestUtil
.forType(FilterValue.class)
.objectMappers(getNamespaceInternalMapper(), getApiMapper())
.test(filterValue);
}

@Test
public void treeConcept() throws IOException, JSONException {
{
Expand Down Expand Up @@ -572,7 +589,9 @@ public void meInformation() throws IOException, JSONException {
.userName(user.getLabel())
.hideLogoutButton(false)
.groups(List.of(new IdLabel<>(new GroupId("test_group"), "test_group_label")))
.datasetAbilities(Map.of(new DatasetId("testdataset"), new MeProcessor.FrontendDatasetAbility(true, true, true)))
.datasetAbilities(Map.of(new DatasetId("testdataset"),
new MeProcessor.FrontendDatasetAbility(true, true, true)
))
.build();

SerializationTestUtil
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
}
},
"filterValue": {
"type": "INTEGER_RANGE",
"type": "MONEY_RANGE",
"value": {
"min": 10000,
"max": 20000
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"label": "Single Number-Real-Range Filter Query with INTEGER RANGE on column with MONEY type",
"label": "Single Number-Real-Range Filter Query with MONEY RANGE on column with MONEY type",
"type": "QUERY_TEST",
"sqlSpec": {
"isEnabled": true
Expand All @@ -22,7 +22,7 @@
"filters": [
{
"filter": "number.number_connector.money_number_filter",
"type": "INTEGER_RANGE",
"type": "MONEY_RANGE",
"value": {
"min": 10000,
"max": 20000
Expand Down

0 comments on commit 985bf94

Please sign in to comment.