Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Output pyish versions of objects using legacy override flag #621

Merged
merged 4 commits into from
Mar 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 1 addition & 13 deletions src/main/java/com/hubspot/jinjava/JinjavaConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ public class JinjavaConfig {
private final Map<Context.Library, Set<String>> disabled;
private final boolean failOnUnknownTokens;
private final boolean nestedInterpretationEnabled;
private final boolean usePyishObjectMapper;
private final RandomNumberGeneratorStrategy randomNumberGenerator;
private final boolean validationMode;
private final long maxStringLength;
Expand Down Expand Up @@ -103,7 +102,6 @@ private JinjavaConfig(Builder builder) {
failOnUnknownTokens = builder.failOnUnknownTokens;
maxOutputSize = builder.maxOutputSize;
nestedInterpretationEnabled = builder.nestedInterpretationEnabled;
usePyishObjectMapper = builder.usePyishObjectMapper;
randomNumberGenerator = builder.randomNumberGeneratorStrategy;
validationMode = builder.validationMode;
maxStringLength = builder.maxStringLength;
Expand Down Expand Up @@ -176,10 +174,6 @@ public boolean isNestedInterpretationEnabled() {
return nestedInterpretationEnabled;
}

public boolean isUsePyishObjectMapper() {
return usePyishObjectMapper;
}

public boolean isValidationMode() {
return validationMode;
}
Expand Down Expand Up @@ -235,7 +229,6 @@ public static class Builder {
private int maxMacroRecursionDepth;
private boolean failOnUnknownTokens;
private boolean nestedInterpretationEnabled = true;
private boolean usePyishObjectMapper = false;
private RandomNumberGeneratorStrategy randomNumberGeneratorStrategy =
RandomNumberGeneratorStrategy.THREAD_LOCAL;
private boolean validationMode = false;
Expand Down Expand Up @@ -330,11 +323,6 @@ public Builder withNestedInterpretationEnabled(boolean nestedInterpretationEnabl
return this;
}

public Builder withUsePyishObjectMapper(boolean usePyishObjectMapper) {
this.usePyishObjectMapper = usePyishObjectMapper;
return this;
}

public Builder withValidationMode(boolean validationMode) {
this.validationMode = validationMode;
return this;
Expand Down Expand Up @@ -366,7 +354,7 @@ public Builder withTokenScannerSymbols(TokenScannerSymbols tokenScannerSymbols)
}

/**
* @deprecated Replaced by {@link LegacyOverrides.Builder#withIterateOverMapKeys(boolean)} ()}
* @deprecated Replaced by {@link LegacyOverrides.Builder#withIterateOverMapKeys(boolean)}}
*/
@Deprecated
public Builder withIterateOverMapKeys(boolean iterateOverMapKeys) {
Expand Down
15 changes: 14 additions & 1 deletion src/main/java/com/hubspot/jinjava/LegacyOverrides.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@ public class LegacyOverrides {
public static final LegacyOverrides NONE = new LegacyOverrides.Builder().build();
private final boolean evaluateMapKeys;
private final boolean iterateOverMapKeys;
private final boolean usePyishObjectMapper;

private LegacyOverrides(Builder builder) {
evaluateMapKeys = builder.evaluateMapKeys;
iterateOverMapKeys = builder.iterateOverMapKeys;
usePyishObjectMapper = builder.usePyishObjectMapper;
}

public static Builder newBuilder() {
Expand All @@ -26,9 +28,14 @@ public boolean isIterateOverMapKeys() {
return iterateOverMapKeys;
}

public boolean isUsePyishObjectMapper() {
return usePyishObjectMapper;
}

public static class Builder {
private boolean evaluateMapKeys = false;
private boolean iterateOverMapKeys = false;
private boolean usePyishObjectMapper = false;

private Builder() {}

Expand All @@ -39,7 +46,8 @@ public LegacyOverrides build() {
public static Builder from(LegacyOverrides legacyOverrides) {
return new Builder()
.withEvaluateMapKeys(legacyOverrides.evaluateMapKeys)
.withIterateOverMapKeys(legacyOverrides.iterateOverMapKeys);
.withIterateOverMapKeys(legacyOverrides.iterateOverMapKeys)
.withUsePyishObjectMapper(legacyOverrides.usePyishObjectMapper);
}

public Builder withEvaluateMapKeys(boolean evaluateMapKeys) {
Expand All @@ -51,5 +59,10 @@ public Builder withIterateOverMapKeys(boolean iterateOverMapKeys) {
this.iterateOverMapKeys = iterateOverMapKeys;
return this;
}

public Builder withUsePyishObjectMapper(boolean usePyishObjectMapper) {
this.usePyishObjectMapper = usePyishObjectMapper;
return this;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -479,7 +479,7 @@ public String resolveString(String variable, int lineNumber, int startPosition)
}

public String getAsString(Object object) {
if (config.isUsePyishObjectMapper()) {
if (config.getLegacyOverrides().isUsePyishObjectMapper()) {
return PyishObjectMapper.getAsUnquotedPyishString(object);
}
return Objects.toString(object, "");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import com.hubspot.jinjava.tree.parse.ExpressionToken;
import com.hubspot.jinjava.util.ChunkResolver;
import com.hubspot.jinjava.util.Logging;
import com.hubspot.jinjava.util.WhitespaceUtils;
import org.apache.commons.lang3.StringUtils;

public class EagerExpressionStrategy implements ExpressionStrategy {
Expand Down Expand Up @@ -48,11 +47,12 @@ private EagerStringResult eagerResolveExpression(
: ""
);
if (chunkResolver.getDeferredWords().isEmpty()) {
String fullyResolved = chunkResolver.resolveChunk(
resolvedExpression.getResult(),
""
String result = interpreter.getAsString(
interpreter.resolveELExpression(
resolvedExpression.getResult(),
interpreter.getLineNumber()
)
);
String result = WhitespaceUtils.unquoteAndUnescape(fullyResolved);
if (
!StringUtils.equals(result, master.getImage()) &&
(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import com.hubspot.jinjava.tree.parse.TagToken;
import com.hubspot.jinjava.util.ChunkResolver;
import com.hubspot.jinjava.util.LengthLimitingStringJoiner;
import com.hubspot.jinjava.util.WhitespaceUtils;
import org.apache.commons.lang3.StringUtils;

public class EagerPrintTag extends EagerStateChangingTag<PrintTag> {
Expand Down Expand Up @@ -78,7 +77,12 @@ public static String interpretExpression(
(
includeExpressionResult
? wrapInRawIfNeeded(
WhitespaceUtils.unquote(resolvedExpression.getResult()),
interpreter.getAsString(
interpreter.resolveELExpression(
resolvedExpression.getResult(),
interpreter.getLineNumber()
)
),
interpreter
)
: ""
Expand Down
20 changes: 8 additions & 12 deletions src/main/java/com/hubspot/jinjava/util/ChunkResolver.java
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ private List<String> getChunk(Character chunkLevelMarker) {
} else if (CHUNK_LEVEL_MARKER_MAP.containsKey(c)) {
setPrevChar(c);
tokenBuilder.append(c);
tokenBuilder.append(resolveChunk(String.join("", getChunk(c)), JINJAVA_NULL));
tokenBuilder.append(resolveChunk(String.join("", getChunk(c))));
tokenBuilder.append(prevChar);
continue;
} else if (isTokenSplitter(c)) {
Expand All @@ -201,7 +201,7 @@ private List<String> getChunk(Character chunkLevelMarker) {
}
tokenBuilder = new StringBuilder();
if (isMiniChunkSplitter(c)) {
chunks.add(resolveChunk(miniChunkBuilder.toString(), JINJAVA_NULL));
chunks.add(resolveChunk(miniChunkBuilder.toString()));
chunks.add(String.valueOf(c));
miniChunkBuilder = new StringBuilder();
} else {
Expand All @@ -221,7 +221,7 @@ private List<String> getChunk(Character chunkLevelMarker) {
tokenBuilder.append(c);
}
miniChunkBuilder.append(resolveToken(tokenBuilder.toString()));
chunks.add(resolveChunk(miniChunkBuilder.toString(), JINJAVA_NULL));
chunks.add(resolveChunk(miniChunkBuilder.toString()));
return chunks;
}

Expand Down Expand Up @@ -290,9 +290,7 @@ private String resolveToken(String token) {
// val is still null
}
}
if (val == null || !isResolvableObject(val)) {
resolvedToken = token;
} else {
if (val != null && isResolvableObject(val)) {
resolvedToken = PyishObjectMapper.getAsPyishString(val);
}
}
Expand All @@ -303,7 +301,7 @@ private String resolveToken(String token) {
}

// Try resolving the chunk/mini chunk as an ELExpression
public String resolveChunk(String chunk, String nullDefault) {
private String resolveChunk(String chunk) {
if (StringUtils.isBlank(chunk)) {
return chunk;
}
Expand All @@ -320,18 +318,16 @@ public String resolveChunk(String chunk, String nullDefault) {
);
if (val != null) {
// If this isn't the final call, don't prematurely resolve complex objects.
if (JINJAVA_NULL.equals(nullDefault) && !isResolvableObject(val)) {
if (!isResolvableObject(val)) {
return chunk;
}
}
} catch (TemplateSyntaxException ignored) {}

Object val = interpreter.resolveELExpression(chunk, token.getLineNumber());
if (val == null) {
resolvedChunk = nullDefault;
} else if (JINJAVA_NULL.equals(nullDefault) && !isResolvableObject(val)) {
resolvedChunk = chunk;
} else {
resolvedChunk = ChunkResolver.JINJAVA_NULL;
} else if (isResolvableObject(val)) {
resolvedChunk = PyishObjectMapper.getAsPyishString(val);
}
}
Expand Down
9 changes: 8 additions & 1 deletion src/test/java/com/hubspot/jinjava/BaseJinjavaTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,13 @@ public abstract class BaseJinjavaTest {
@Before
public void baseSetup() {
jinjava =
new Jinjava(JinjavaConfig.newBuilder().withUsePyishObjectMapper(true).build());
new Jinjava(
JinjavaConfig
.newBuilder()
.withLegacyOverrides(
LegacyOverrides.newBuilder().withUsePyishObjectMapper(true).build()
)
.build()
);
}
}
8 changes: 6 additions & 2 deletions src/test/java/com/hubspot/jinjava/EagerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,9 @@ public Optional<LocationResolver> getLocationResolver() {
.withRandomNumberGeneratorStrategy(RandomNumberGeneratorStrategy.DEFERRED)
.withExecutionMode(EagerExecutionMode.instance())
.withNestedInterpretationEnabled(true)
.withUsePyishObjectMapper(true)
.withLegacyOverrides(
LegacyOverrides.newBuilder().withUsePyishObjectMapper(true).build()
)
.withMaxMacroRecursionDepth(5)
.withEnableRecursiveMacroCalls(true)
.build();
Expand Down Expand Up @@ -712,7 +714,9 @@ public void itWrapsCertainOutputInRaw() {
.withRandomNumberGeneratorStrategy(RandomNumberGeneratorStrategy.DEFERRED)
.withExecutionMode(EagerExecutionMode.instance())
.withNestedInterpretationEnabled(false)
.withUsePyishObjectMapper(true)
.withLegacyOverrides(
LegacyOverrides.newBuilder().withUsePyishObjectMapper(true).build()
)
.build();
JinjavaInterpreter parentInterpreter = new JinjavaInterpreter(
jinjava,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import static org.assertj.core.api.Assertions.assertThat;

import com.hubspot.jinjava.JinjavaConfig;
import com.hubspot.jinjava.LegacyOverrides;
import com.hubspot.jinjava.interpret.DeferredValue;
import com.hubspot.jinjava.interpret.JinjavaInterpreter;
import com.hubspot.jinjava.mode.EagerExecutionMode;
Expand Down Expand Up @@ -44,7 +45,9 @@ public void itPreservesRawTags() {
JinjavaConfig
.newBuilder()
.withNestedInterpretationEnabled(false)
.withUsePyishObjectMapper(true)
.withLegacyOverrides(
LegacyOverrides.newBuilder().withUsePyishObjectMapper(true).build()
)
.withExecutionMode(EagerExecutionMode.instance())
.build()
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import com.hubspot.jinjava.ExpectedNodeInterpreter;
import com.hubspot.jinjava.JinjavaConfig;
import com.hubspot.jinjava.LegacyOverrides;
import com.hubspot.jinjava.interpret.DeferredValue;
import com.hubspot.jinjava.interpret.JinjavaInterpreter;
import com.hubspot.jinjava.interpret.TemplateError.ErrorReason;
Expand All @@ -29,6 +30,9 @@ public void eagerSetup() {
.newBuilder()
.withMaxOutputSize(MAX_OUTPUT_SIZE)
.withExecutionMode(EagerExecutionMode.instance())
.withLegacyOverrides(
LegacyOverrides.newBuilder().withUsePyishObjectMapper(true).build()
)
.build()
);

Expand Down Expand Up @@ -64,14 +68,4 @@ public void itLimitsLength() {
assertThat(interpreter.getErrors().get(0).getReason())
.isEqualTo(ErrorReason.OUTPUT_TOO_BIG);
}

/** This is broken in normal Jinjava as <code>hey</code> does not get output in quotes.
* It works in Eager Jinjava as <code>hey</code> is quoted properly.
*/
@Test
@Override
public void itResolvesExpressions() {
String template = "{% set output = [] %}{% do output.append('hey') %}{{ output }}";
assertThat(interpreter.render(template)).isEqualTo("['hey']");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import com.google.common.io.Resources;
import com.hubspot.jinjava.JinjavaConfig;
import com.hubspot.jinjava.LegacyOverrides;
import com.hubspot.jinjava.interpret.Context;
import com.hubspot.jinjava.interpret.DeferredValue;
import com.hubspot.jinjava.interpret.JinjavaInterpreter;
Expand Down Expand Up @@ -39,6 +40,9 @@ public void eagerSetup() {
JinjavaConfig
.newBuilder()
.withExecutionMode(EagerExecutionMode.instance())
.withLegacyOverrides(
LegacyOverrides.newBuilder().withUsePyishObjectMapper(true).build()
)
.build()
);
Tag tag = EagerTagFactory
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ public void itSerializesDateProperly() {
// don't prematurely resolve date because of datetime functions.
assertThat(WhitespaceUtils.unquoteAndUnescape(chunkResolver.resolveChunks()))
.isEqualTo("date");
assertThat(WhitespaceUtils.unquoteAndUnescape(chunkResolver.resolveChunk("date", "")))
assertThat(WhitespaceUtils.unquoteAndUnescape(interpreter.resolveString("date", -1)))
.isEqualTo(date.toString());
}

Expand Down