Skip to content

Commit 5afaece

Browse files
jasmith-hsjasmith-hs
authored
Merge pull request #1309 from HubSpot/2.8.x-integer-set-to-long-conversion
Auto-convert Integer to Long in set filters (2.8.x)
2 parents a1af87c + e520666 commit 5afaece

3 files changed

Lines changed: 127 additions & 7 deletions

File tree

‎src/main/java/com/hubspot/jinjava/features/BuiltInFeatures.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,4 +8,5 @@ public interface BuiltInFeatures {
88
String IGNORE_NESTED_INTERPRETATION_PARSE_ERRORS =
99
"IGNORE_NESTED_INTERPRETATION_PARSE_ERRORS";
1010
String OUTPUT_UNDEFINED_VARIABLES_ERROR = "OUTPUT_UNDEFINED_VARIABLES_ERROR";
11+
String INTEGER_SET_TO_LONG_CONVERSION = "INTEGER_SET_TO_LONG_CONVERSION";
1112
}

‎src/main/java/com/hubspot/jinjava/lib/filter/AbstractSetFilter.java‎

Lines changed: 61 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.hubspot.jinjava.lib.filter;
22

3+
import com.hubspot.jinjava.features.BuiltInFeatures;
34
import com.hubspot.jinjava.interpret.JinjavaInterpreter;
45
import com.hubspot.jinjava.interpret.TemplateError;
56
import com.hubspot.jinjava.interpret.TemplateSyntaxException;
@@ -43,7 +44,27 @@ public Object filter(
4344
Set<Object> varSet = objectToSet(var);
4445
Set<Object> argSet = objectToSet(parseArgs(interpreter, args));
4546

46-
attachMismatchedTypesWarning(interpreter, varSet, argSet);
47+
if (!varSet.isEmpty() && !argSet.isEmpty()) {
48+
Object oneVar = varSet.iterator().next();
49+
Object oneArg = argSet.iterator().next();
50+
51+
boolean featureActive = interpreter
52+
.getConfig()
53+
.getFeatures()
54+
.isActive(
55+
BuiltInFeatures.INTEGER_SET_TO_LONG_CONVERSION,
56+
interpreter.getContext()
57+
);
58+
if (featureActive) {
59+
if (oneVar instanceof Integer && oneArg instanceof Long) {
60+
varSet = convertIntegersToLongs(varSet);
61+
} else if (oneArg instanceof Integer && oneVar instanceof Long) {
62+
argSet = convertIntegersToLongs(argSet);
63+
}
64+
}
65+
66+
attachMismatchedTypesWarning(interpreter, varSet, argSet, oneVar, oneArg);
67+
}
4768

4869
return filter(varSet, argSet);
4970
}
@@ -55,17 +76,31 @@ protected void attachMismatchedTypesWarning(
5576
Set<Object> varSet,
5677
Set<Object> argSet
5778
) {
58-
boolean hasAtLeastOneSetEmpty = varSet.isEmpty() || argSet.isEmpty();
59-
if (hasAtLeastOneSetEmpty) {
79+
if (varSet.isEmpty() || argSet.isEmpty()) {
6080
return;
6181
}
82+
attachMismatchedTypesWarning(
83+
interpreter,
84+
varSet,
85+
argSet,
86+
varSet.iterator().next(),
87+
argSet.iterator().next()
88+
);
89+
}
6290

63-
boolean areMatchedElementTypes = getTypeOfSetElements(varSet)
64-
.equals(getTypeOfSetElements(argSet));
65-
if (areMatchedElementTypes) {
91+
private void attachMismatchedTypesWarning(
92+
JinjavaInterpreter interpreter,
93+
Set<Object> varSet,
94+
Set<Object> argSet,
95+
Object oneVarObj,
96+
Object oneArgObj
97+
) {
98+
if (getTypeOfSetElements(varSet).equals(getTypeOfSetElements(argSet))) {
99+
return;
100+
}
101+
if (potentiallyConvertibleNumbers(oneVarObj, oneArgObj)) {
66102
return;
67103
}
68-
69104
interpreter.addError(
70105
new TemplateError(
71106
TemplateError.ErrorType.WARNING,
@@ -84,6 +119,25 @@ protected void attachMismatchedTypesWarning(
84119
);
85120
}
86121

122+
private boolean potentiallyConvertibleNumbers(Object oneVarObj, Object oneArgObj) {
123+
return (
124+
(oneArgObj instanceof Integer && oneVarObj instanceof Long) ||
125+
(oneVarObj instanceof Integer && oneArgObj instanceof Long)
126+
);
127+
}
128+
129+
private Set<Object> convertIntegersToLongs(Set<Object> set) {
130+
Set<Object> result = new LinkedHashSet<>();
131+
for (Object element : set) {
132+
if (element instanceof Integer) {
133+
result.add(Long.valueOf(element.toString()));
134+
} else {
135+
result.add(element);
136+
}
137+
}
138+
return result;
139+
}
140+
87141
private String getTypeOfSetElements(Set<Object> set) {
88142
return TypeFunction.type(set.iterator().next());
89143
}

‎src/test/java/com/hubspot/jinjava/lib/filter/AbstractSetFilterTest.java‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,18 @@
33
import static org.assertj.core.api.Assertions.assertThat;
44

55
import com.hubspot.jinjava.BaseJinjavaTest;
6+
import com.hubspot.jinjava.Jinjava;
7+
import com.hubspot.jinjava.JinjavaConfig;
8+
import com.hubspot.jinjava.LegacyOverrides;
9+
import com.hubspot.jinjava.features.BuiltInFeatures;
10+
import com.hubspot.jinjava.features.FeatureConfig;
11+
import com.hubspot.jinjava.features.FeatureStrategies;
612
import com.hubspot.jinjava.interpret.JinjavaInterpreter;
13+
import com.hubspot.jinjava.interpret.RenderResult;
714
import com.hubspot.jinjava.interpret.TemplateError;
15+
import java.util.HashMap;
816
import java.util.List;
17+
import java.util.Map;
918
import java.util.Set;
1019
import org.junit.Before;
1120
import org.junit.Test;
@@ -80,4 +89,60 @@ public void itThrowsWarningOnMismatchTypes() {
8089
"Mismatched Types: input set has elements of type 'long' but arg set has elements of type 'str'. Use |map filter to convert sets to the same type for filter to work correctly."
8190
);
8291
}
92+
93+
@Test
94+
public void itDoesNotThrowWarningOnIntegerLongMismatch() {
95+
JinjavaInterpreter interpreter = jinjava.newInterpreter();
96+
97+
Set<Object> varSet = concreteSetFilter.objectToSet(new Long[] { 1L, 2L, 3L });
98+
Set<Object> argSet = concreteSetFilter.objectToSet(new Integer[] { 1, 2, 3 });
99+
concreteSetFilter.attachMismatchedTypesWarning(interpreter, varSet, argSet);
100+
101+
assertThat(interpreter.getErrors()).isEmpty();
102+
}
103+
104+
@Test
105+
public void itConvertsIntegerToLongWhenFeatureActive() {
106+
Jinjava jinjavaWithFeature = new Jinjava(
107+
JinjavaConfig
108+
.newBuilder()
109+
.withLegacyOverrides(
110+
LegacyOverrides
111+
.newBuilder()
112+
.withUsePyishObjectMapper(true)
113+
.withKeepNullableLoopValues(true)
114+
.build()
115+
)
116+
.withFeatureConfig(
117+
FeatureConfig
118+
.newBuilder()
119+
.add(BuiltInFeatures.INTEGER_SET_TO_LONG_CONVERSION, FeatureStrategies.ACTIVE)
120+
.build()
121+
)
122+
.build()
123+
);
124+
125+
Map<String, Object> vars = new HashMap<>();
126+
vars.put("longList", new Long[] { 1L, 2L, 3L });
127+
vars.put("intList", new Integer[] { 2, 3, 4 });
128+
129+
String result = jinjavaWithFeature.render("{{ longList|intersect(intList) }}", vars);
130+
131+
assertThat(result).isEqualTo("[2, 3]");
132+
}
133+
134+
@Test
135+
public void itDoesNotConvertWhenFeatureInactive() {
136+
Map<String, Object> vars = new HashMap<>();
137+
vars.put("longList", new Long[] { 1L, 2L, 3L });
138+
vars.put("intList", new Integer[] { 2, 3, 4 });
139+
140+
RenderResult renderResult = jinjava.renderForResult(
141+
"{{ longList|intersect(intList) }}",
142+
vars
143+
);
144+
145+
assertThat(renderResult.getOutput()).isEqualTo("[]");
146+
assertThat(renderResult.getErrors()).isEmpty();
147+
}
83148
}

0 commit comments

Comments
 (0)