diff --git a/pom.xml b/pom.xml index ee78e721a..57723ed5d 100644 --- a/pom.xml +++ b/pom.xml @@ -20,6 +20,7 @@ 0.8.3 3.0.1 1.9 + 1.5 @@ -101,6 +102,11 @@ immutables-exceptions ${dep.hubspot-immutables.version} + + com.hubspot + algebra + ${dep.algebra.version} + @@ -213,6 +219,10 @@ com.hubspot.immutables immutables-exceptions + + com.hubspot + algebra + diff --git a/src/main/java/com/hubspot/jinjava/Jinjava.java b/src/main/java/com/hubspot/jinjava/Jinjava.java index 21179b86d..5c06e3d7a 100644 --- a/src/main/java/com/hubspot/jinjava/Jinjava.java +++ b/src/main/java/com/hubspot/jinjava/Jinjava.java @@ -20,6 +20,7 @@ import com.hubspot.jinjava.el.ExtendedSyntaxBuilder; import com.hubspot.jinjava.el.TruthyTypeConverter; import com.hubspot.jinjava.el.ext.eager.EagerExtendedSyntaxBuilder; +import com.hubspot.jinjava.interpret.AutoCloseableSupplier.AutoCloseableImpl; import com.hubspot.jinjava.interpret.Context; import com.hubspot.jinjava.interpret.FatalTemplateErrorsException; import com.hubspot.jinjava.interpret.InterpretException; @@ -245,52 +246,55 @@ public RenderResult renderForResult( context = new Context(copyGlobalContext(), bindings, renderConfig.getDisabled()); } - JinjavaInterpreter interpreter = globalConfig - .getInterpreterFactory() - .newInstance(this, context, renderConfig); - JinjavaInterpreter.pushCurrent(interpreter); - - try { - String result = interpreter.render(template); - return new RenderResult( - result, - interpreter.getContext(), - interpreter.getErrorsCopy() - ); - } catch (InterpretException e) { - if (e instanceof TemplateSyntaxException) { + try ( + AutoCloseableImpl interpreterAutoCloseable = JinjavaInterpreter + .closeablePushCurrent( + globalConfig.getInterpreterFactory().newInstance(this, context, renderConfig) + ) + .get() + ) { + JinjavaInterpreter interpreter = interpreterAutoCloseable.value(); + try { + String result = interpreter.render(template); + return new RenderResult( + result, + interpreter.getContext(), + interpreter.getErrorsCopy() + ); + } catch (InterpretException e) { + if (e instanceof TemplateSyntaxException) { + return new RenderResult( + TemplateError.fromException((TemplateSyntaxException) e), + interpreter.getContext(), + interpreter.getErrorsCopy() + ); + } + return new RenderResult( + TemplateError.fromSyntaxError(e), + interpreter.getContext(), + interpreter.getErrorsCopy() + ); + } catch (InvalidArgumentException e) { + return new RenderResult( + TemplateError.fromInvalidArgumentException(e), + interpreter.getContext(), + interpreter.getErrorsCopy() + ); + } catch (InvalidInputException e) { return new RenderResult( - TemplateError.fromException((TemplateSyntaxException) e), + TemplateError.fromInvalidInputException(e), + interpreter.getContext(), + interpreter.getErrorsCopy() + ); + } catch (Exception e) { + return new RenderResult( + TemplateError.fromException(e), interpreter.getContext(), interpreter.getErrorsCopy() ); } - return new RenderResult( - TemplateError.fromSyntaxError(e), - interpreter.getContext(), - interpreter.getErrorsCopy() - ); - } catch (InvalidArgumentException e) { - return new RenderResult( - TemplateError.fromInvalidArgumentException(e), - interpreter.getContext(), - interpreter.getErrorsCopy() - ); - } catch (InvalidInputException e) { - return new RenderResult( - TemplateError.fromInvalidInputException(e), - interpreter.getContext(), - interpreter.getErrorsCopy() - ); - } catch (Exception e) { - return new RenderResult( - TemplateError.fromException(e), - interpreter.getContext(), - interpreter.getErrorsCopy() - ); } finally { globalContext.reset(); - JinjavaInterpreter.popCurrent(); } } diff --git a/src/main/java/com/hubspot/jinjava/el/ext/AstMacroFunction.java b/src/main/java/com/hubspot/jinjava/el/ext/AstMacroFunction.java index 185c60792..6975d5ce6 100644 --- a/src/main/java/com/hubspot/jinjava/el/ext/AstMacroFunction.java +++ b/src/main/java/com/hubspot/jinjava/el/ext/AstMacroFunction.java @@ -1,10 +1,13 @@ package com.hubspot.jinjava.el.ext; import com.google.common.collect.ImmutableMap; +import com.hubspot.algebra.Result; +import com.hubspot.jinjava.interpret.AutoCloseableSupplier; +import com.hubspot.jinjava.interpret.AutoCloseableSupplier.AutoCloseableImpl; import com.hubspot.jinjava.interpret.CallStack; import com.hubspot.jinjava.interpret.DeferredValueException; import com.hubspot.jinjava.interpret.JinjavaInterpreter; -import com.hubspot.jinjava.interpret.MacroTagCycleException; +import com.hubspot.jinjava.interpret.TagCycleException; import com.hubspot.jinjava.interpret.TemplateError; import com.hubspot.jinjava.interpret.errorcategory.BasicTemplateErrorCategory; import com.hubspot.jinjava.lib.fn.MacroFunction; @@ -18,6 +21,10 @@ public class AstMacroFunction extends AstFunction { + public enum MacroCallError { + CYCLE_DETECTED, + } + public AstMacroFunction(String name, int index, AstParameters params, boolean varargs) { super(name, index, params, varargs); } @@ -37,30 +44,16 @@ public Object eval(Bindings bindings, ELContext context) { interpreter.getPosition() ); } - if (!macroFunction.isCaller()) { - if (checkAndPushMacroStack(interpreter, getName())) { - return ""; - } + if (macroFunction.isCaller()) { + return wrapInvoke(bindings, context, macroFunction); } - - try { - return invoke( - bindings, - context, - macroFunction, - AbstractCallableMethod.EVAL_METHOD - ); - } catch (IllegalAccessException e) { - throw new ELException(LocalMessages.get("error.function.access", getName()), e); - } catch (InvocationTargetException e) { - throw new ELException( - LocalMessages.get("error.function.invocation", getName()), - e.getCause() - ); - } finally { - if (!macroFunction.isCaller()) { - interpreter.getContext().getMacroStack().pop(); - } + try ( + AutoCloseableImpl> macroStackPush = + checkAndPushMacroStackWithWrapper(interpreter, getName()).get() + ) { + return macroStackPush + .value() + .match(err -> "", path -> wrapInvoke(bindings, context, macroFunction)); } } @@ -69,62 +62,104 @@ public Object eval(Bindings bindings, ELContext context) { : super.eval(bindings, context); } - public static boolean checkAndPushMacroStack( + private Object wrapInvoke( + Bindings bindings, + ELContext context, + MacroFunction macroFunction + ) { + try { + return invoke(bindings, context, macroFunction, AbstractCallableMethod.EVAL_METHOD); + } catch (IllegalAccessException e) { + throw new ELException(LocalMessages.get("error.function.access", getName()), e); + } catch (InvocationTargetException e) { + throw new ELException( + LocalMessages.get("error.function.invocation", getName()), + e.getCause() + ); + } + } + + public static AutoCloseableSupplier> checkAndPushMacroStackWithWrapper( JinjavaInterpreter interpreter, String name ) { CallStack macroStack = interpreter.getContext().getMacroStack(); - try { - if (interpreter.getConfig().isEnableRecursiveMacroCalls()) { - if (interpreter.getConfig().getMaxMacroRecursionDepth() != 0) { - macroStack.pushWithMaxDepth( + if (interpreter.getConfig().isEnableRecursiveMacroCalls()) { + if (interpreter.getConfig().getMaxMacroRecursionDepth() != 0) { + return macroStack + .closeablePushWithMaxDepth( name, interpreter.getConfig().getMaxMacroRecursionDepth(), interpreter.getLineNumber(), interpreter.getPosition() + ) + .map(result -> + result.mapErr(err -> { + handleMacroCycleError(interpreter, name, err); + return MacroCallError.CYCLE_DETECTED; + }) ); - } else { - macroStack.pushWithoutCycleCheck( + } else { + return macroStack + .closeablePushWithoutCycleCheck( name, interpreter.getLineNumber(), interpreter.getPosition() - ); - } - } else { - macroStack.push(name, -1, -1); - } - } catch (MacroTagCycleException e) { - int maxDepth = interpreter.getConfig().getMaxMacroRecursionDepth(); - if (maxDepth != 0 && interpreter.getConfig().isValidationMode()) { - // validation mode is only concerned with syntax - return true; + ) + .map(Result::ok); } - - String message = maxDepth == 0 - ? String.format("Cycle detected for macro '%s'", name) - : String.format( - "Max recursion limit of %d reached for macro '%s'", - maxDepth, - name - ); - - interpreter.addError( - new TemplateError( - TemplateError.ErrorType.WARNING, - TemplateError.ErrorReason.EXCEPTION, - TemplateError.ErrorItem.TAG, - message, - null, - e.getLineNumber(), - e.getStartPosition(), - e, - BasicTemplateErrorCategory.CYCLE_DETECTED, - ImmutableMap.of("name", name) - ) + } + return macroStack + .closeablePush(name, -1, -1) + .map(result -> + result.mapErr(err -> { + handleMacroCycleError(interpreter, name, err); + return MacroCallError.CYCLE_DETECTED; + }) ); + } - return true; + private static void handleMacroCycleError( + JinjavaInterpreter interpreter, + String name, + TagCycleException e + ) { + int maxDepth = interpreter.getConfig().getMaxMacroRecursionDepth(); + if (maxDepth != 0 && interpreter.getConfig().isValidationMode()) { + // validation mode is only concerned with syntax + return; } - return false; + + String message = maxDepth == 0 + ? String.format("Cycle detected for macro '%s'", name) + : String.format("Max recursion limit of %d reached for macro '%s'", maxDepth, name); + + interpreter.addError( + new TemplateError( + TemplateError.ErrorType.WARNING, + TemplateError.ErrorReason.EXCEPTION, + TemplateError.ErrorItem.TAG, + message, + null, + e.getLineNumber(), + e.getStartPosition(), + e, + BasicTemplateErrorCategory.CYCLE_DETECTED, + ImmutableMap.of("name", name) + ) + ); + } + + @Deprecated + public static boolean checkAndPushMacroStack( + JinjavaInterpreter interpreter, + String name + ) { + return checkAndPushMacroStackWithWrapper(interpreter, name) + .dangerouslyGetWithoutClosing() + .match( + err -> true, // cycle detected + ok -> false // no cycle + ); } } diff --git a/src/main/java/com/hubspot/jinjava/interpret/AutoCloseableSupplier.java b/src/main/java/com/hubspot/jinjava/interpret/AutoCloseableSupplier.java new file mode 100644 index 000000000..12a7b8dfe --- /dev/null +++ b/src/main/java/com/hubspot/jinjava/interpret/AutoCloseableSupplier.java @@ -0,0 +1,68 @@ +package com.hubspot.jinjava.interpret; + +import com.google.common.base.Suppliers; +import com.hubspot.jinjava.interpret.AutoCloseableSupplier.AutoCloseableImpl; +import java.util.function.Consumer; +import java.util.function.Function; +import java.util.function.Supplier; + +public class AutoCloseableSupplier implements Supplier> { + + public static AutoCloseableSupplier of(T tSupplier) { + return of(() -> tSupplier, ignored -> {}); + } + + public static AutoCloseableSupplier of( + Supplier tSupplier, + Consumer closeConsumer + ) { + return new AutoCloseableSupplier<>( + Suppliers.memoize(() -> new AutoCloseableImpl<>(tSupplier.get(), closeConsumer)) + ); + } + + private final Supplier> autoCloseableImplWrapper; + + private AutoCloseableSupplier(Supplier> autoCloseableImplWrapper) { + this.autoCloseableImplWrapper = autoCloseableImplWrapper; + } + + @Override + public AutoCloseableImpl get() { + return autoCloseableImplWrapper.get(); + } + + public T dangerouslyGetWithoutClosing() { + return autoCloseableImplWrapper.get().value(); + } + + public AutoCloseableSupplier map(Function mapper) { + return new AutoCloseableSupplier<>(() -> { + T t = autoCloseableImplWrapper.get().value(); + return new AutoCloseableImpl<>( + mapper.apply(t), + r -> autoCloseableImplWrapper.get().closeConsumer.accept(t) + ); + }); + } + + public static class AutoCloseableImpl implements java.lang.AutoCloseable { + + private final T t; + private final Consumer closeConsumer; + + protected AutoCloseableImpl(T t, Consumer closeConsumer) { + this.t = t; + this.closeConsumer = closeConsumer; + } + + public T value() { + return t; + } + + @Override + public void close() { + closeConsumer.accept(t); + } + } +} diff --git a/src/main/java/com/hubspot/jinjava/interpret/CallStack.java b/src/main/java/com/hubspot/jinjava/interpret/CallStack.java index 83bff5d69..f67d96d6b 100644 --- a/src/main/java/com/hubspot/jinjava/interpret/CallStack.java +++ b/src/main/java/com/hubspot/jinjava/interpret/CallStack.java @@ -1,5 +1,6 @@ package com.hubspot.jinjava.interpret; +import com.hubspot.algebra.Result; import java.util.Optional; import java.util.Stack; @@ -111,6 +112,57 @@ public boolean isEmpty() { return stack.empty() && (parent == null || parent.isEmpty()); } + public AutoCloseableSupplier> closeablePush( + String path, + int lineNumber, + int startPosition + ) { + return AutoCloseableSupplier.of( + () -> { + try { + push(path, lineNumber, startPosition); + return Result.ok(path); + } catch (TagCycleException e) { + return Result.err(e); + } + }, + result -> result.ifOk(ok -> pop()) + ); + } + + public AutoCloseableSupplier closeablePushWithoutCycleCheck( + String path, + int lineNumber, + int startPosition + ) { + return AutoCloseableSupplier.of( + () -> { + pushWithoutCycleCheck(path, lineNumber, startPosition); + return path; + }, + ignored -> pop() + ); + } + + public AutoCloseableSupplier> closeablePushWithMaxDepth( + String path, + int maxDepth, + int lineNumber, + int startPosition + ) { + return AutoCloseableSupplier.of( + () -> { + try { + pushWithMaxDepth(path, maxDepth, lineNumber, startPosition); + return Result.ok(path); + } catch (TagCycleException e) { + return Result.err(e); + } + }, + result -> result.ifOk(ok -> pop()) + ); + } + private void pushToStack(String path, int lineNumber, int startPosition) { if (isEmpty()) { topLineNumber = lineNumber; diff --git a/src/main/java/com/hubspot/jinjava/interpret/Context.java b/src/main/java/com/hubspot/jinjava/interpret/Context.java index 4126371e6..a9892c060 100644 --- a/src/main/java/com/hubspot/jinjava/interpret/Context.java +++ b/src/main/java/com/hubspot/jinjava/interpret/Context.java @@ -21,6 +21,7 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.SetMultimap; import com.google.common.collect.Sets; +import com.hubspot.jinjava.interpret.AutoCloseableSupplier.AutoCloseableImpl; import com.hubspot.jinjava.interpret.ContextConfigurationIF.ErrorHandlingStrategyIF.TemplateErrorTypeHandlingStrategy; import com.hubspot.jinjava.lib.Importable; import com.hubspot.jinjava.lib.expression.ExpressionStrategy; @@ -677,6 +678,10 @@ public CallStack getImportPathStack() { return importPathStack; } + public CallStack getFromPathStack() { + return fromStack; + } + public CallStack getIncludePathStack() { return includePathStack; } @@ -693,10 +698,12 @@ public CallStack getCurrentPathStack() { return currentPathStack; } + @Deprecated public void pushFromStack(String path, int lineNumber, int startPosition) { fromStack.push(path, lineNumber, startPosition); } + @Deprecated public void popFromStack() { fromStack.pop(); } @@ -717,10 +724,17 @@ public void setRenderDepth(int renderDepth) { this.renderDepth = renderDepth; } + public AutoCloseableSupplier closeablePushRenderStack(String template) { + renderStack.push(template); + return AutoCloseableSupplier.of(() -> template, t -> renderStack.pop()); + } + + @Deprecated public void pushRenderStack(String template) { renderStack.push(template); } + @Deprecated public String popRenderStack() { return renderStack.pop(); } @@ -878,25 +892,16 @@ public TemporaryValueClosable withUnwrapRawOverride() { return temporaryValueClosable; } - public static class TemporaryValueClosable implements AutoCloseable { - - private final T previousValue; - private final Consumer resetValueConsumer; + public static class TemporaryValueClosable extends AutoCloseableImpl { private TemporaryValueClosable(T previousValue, Consumer resetValueConsumer) { - this.previousValue = previousValue; - this.resetValueConsumer = resetValueConsumer; + super(previousValue, resetValueConsumer); } public static TemporaryValueClosable noOp() { return new NoOpTemporaryValueClosable<>(); } - @Override - public void close() { - resetValueConsumer.accept(previousValue); - } - private static class NoOpTemporaryValueClosable extends TemporaryValueClosable { private NoOpTemporaryValueClosable() { diff --git a/src/main/java/com/hubspot/jinjava/interpret/JinjavaInterpreter.java b/src/main/java/com/hubspot/jinjava/interpret/JinjavaInterpreter.java index 843eb0c49..e60c70fc9 100644 --- a/src/main/java/com/hubspot/jinjava/interpret/JinjavaInterpreter.java +++ b/src/main/java/com/hubspot/jinjava/interpret/JinjavaInterpreter.java @@ -24,11 +24,13 @@ import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.common.collect.Multimap; +import com.hubspot.algebra.Result; import com.hubspot.jinjava.Jinjava; import com.hubspot.jinjava.JinjavaConfig; import com.hubspot.jinjava.el.ExpressionResolver; import com.hubspot.jinjava.el.ext.DeferredParsingException; import com.hubspot.jinjava.el.ext.ExtendedParser; +import com.hubspot.jinjava.interpret.AutoCloseableSupplier.AutoCloseableImpl; import com.hubspot.jinjava.interpret.Context.TemporaryValueClosable; import com.hubspot.jinjava.interpret.ContextConfigurationIF.ErrorHandlingStrategyIF; import com.hubspot.jinjava.interpret.ContextConfigurationIF.ErrorHandlingStrategyIF.TemplateErrorTypeHandlingStrategy; @@ -75,6 +77,7 @@ import java.util.Set; import java.util.Stack; import java.util.concurrent.ThreadLocalRandom; +import java.util.function.Function; import java.util.stream.Collectors; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.exception.ExceptionUtils; @@ -380,14 +383,18 @@ private String render(Node root, boolean processExtendRoots, long renderLimit) { output.addNode(new RenderedOutputNode(renderStr)); } else { OutputNode out; - context.pushRenderStack(renderStr); - try { - out = node.render(this); - } catch (DeferredValueException e) { - context.handleDeferredNode(node); - out = new RenderedOutputNode(node.getMaster().getImage()); + try ( + AutoCloseableImpl closeable = context + .closeablePushRenderStack(renderStr) + .get() + ) { + try { + out = node.render(this); + } catch (DeferredValueException e) { + context.handleDeferredNode(node); + out = new RenderedOutputNode(node.getMaster().getImage()); + } } - context.popRenderStack(); output.addNode(out); } } catch (OutputTooBigException e) { @@ -434,46 +441,54 @@ private String render(Node root, boolean processExtendRoots, long renderLimit) { break; } extendPaths.add(extendPath.orElse("")); - context - .getCurrentPathStack() - .push( - extendPath.orElse(""), - context.getExtendPathStack().getTopLineNumber(), - context.getExtendPathStack().getTopStartPosition() - ); - Node parentRoot = extendParentRoots.removeFirst(); - if (context.getDeferredTokens().size() > numDeferredTokensBefore) { - ignoredOutput.append( - output - .getNodes() - .stream() - .filter(node -> node instanceof RenderedOutputNode) - .map(OutputNode::getValue) - .collect(Collectors.joining()) - ); - } - numDeferredTokensBefore = context.getDeferredTokens().size(); - output = new OutputList(config.getMaxOutputSize()); - output.addNode(pathSetter); - boolean hasNestedExtends = false; - for (Node node : parentRoot.getChildren()) { - lineNumber = node.getLineNumber() - 1; // The line number is off by one when rendering the extend parent - position = node.getStartPosition(); - try { - OutputNode out = node.render(this); - output.addNode(out); - if (isExtendsTag(node)) { - hasNestedExtends = true; + try ( + AutoCloseableImpl> closeableCurrentPath = + context + .getCurrentPathStack() + .closeablePush( + extendPath.orElse(""), + context.getExtendPathStack().getTopLineNumber(), + context.getExtendPathStack().getTopStartPosition() + ) + .get() + ) { + String currentPath = closeableCurrentPath + .value() + .unwrapOrElseThrow(Function.identity()); + Node parentRoot = extendParentRoots.removeFirst(); + if (context.getDeferredTokens().size() > numDeferredTokensBefore) { + ignoredOutput.append( + output + .getNodes() + .stream() + .filter(node -> node instanceof RenderedOutputNode) + .map(OutputNode::getValue) + .collect(Collectors.joining()) + ); + } + numDeferredTokensBefore = context.getDeferredTokens().size(); + output = new OutputList(config.getMaxOutputSize()); + output.addNode(pathSetter); + boolean hasNestedExtends = false; + for (Node node : parentRoot.getChildren()) { + lineNumber = node.getLineNumber() - 1; // The line number is off by one when rendering the extend parent + position = node.getStartPosition(); + try { + OutputNode out = node.render(this); + output.addNode(out); + if (isExtendsTag(node)) { + hasNestedExtends = true; + } + } catch (OutputTooBigException e) { + addError(TemplateError.fromOutputTooBigException(e)); + return output.getValue(); } - } catch (OutputTooBigException e) { - addError(TemplateError.fromOutputTooBigException(e)); - return output.getValue(); } + Optional currentExtendPath = context.getExtendPathStack().pop(); + extendPath = + hasNestedExtends ? currentExtendPath : context.getExtendPathStack().peek(); + basePath = Optional.of(currentPath); } - Optional currentExtendPath = context.getExtendPathStack().pop(); - extendPath = - hasNestedExtends ? currentExtendPath : context.getExtendPathStack().peek(); - basePath = context.getCurrentPathStack().pop(); } } @@ -547,37 +562,29 @@ private void resolveBlockStubs(OutputList output, Stack blockNames) { OutputList blockValueBuilder = new OutputList(config.getMaxOutputSize()); DynamicRenderedOutputNode prefix = new DynamicRenderedOutputNode(); blockValueBuilder.addNode(prefix); - boolean pushedParentPathOntoStack = false; int numDeferredTokensBefore = context.getDeferredTokens().size(); - if ( - block.getParentPath().isPresent() && - !getContext().getCurrentPathStack().contains(block.getParentPath().get()) + + try ( + AutoCloseableImpl parentPathPush = conditionallyPushParentPath(block) + .get() ) { - getContext() - .getCurrentPathStack() - .push( - block.getParentPath().get(), - block.getParentLineNo(), - block.getParentPosition() - ); - pushedParentPathOntoStack = true; - lineNumber--; // The line number is off by one when rendering the block from the parent template - } - for (Node child : block.getNodes()) { - lineNumber = child.getLineNumber(); - position = child.getStartPosition(); + if (parentPathPush.value()) { + lineNumber--; // The line number is off by one when rendering the block from the parent template + } - blockValueBuilder.addNode(child.render(this)); - } - if (context.getDeferredTokens().size() > numDeferredTokensBefore) { - EagerReconstructionUtils.reconstructPathAroundBlock( - prefix, - blockValueBuilder, - this - ); - } - if (pushedParentPathOntoStack) { - getContext().getCurrentPathStack().pop(); + for (Node child : block.getNodes()) { + lineNumber = child.getLineNumber(); + position = child.getStartPosition(); + + blockValueBuilder.addNode(child.render(this)); + } + if (context.getDeferredTokens().size() > numDeferredTokensBefore) { + EagerReconstructionUtils.reconstructPathAroundBlock( + prefix, + blockValueBuilder, + this + ); + } } blockNames.push(blockPlaceholder.getBlockName()); resolveBlockStubs(blockValueBuilder, blockNames); @@ -596,6 +603,24 @@ private void resolveBlockStubs(OutputList output, Stack blockNames) { } } + private AutoCloseableSupplier conditionallyPushParentPath(BlockInfo block) { + if ( + block.getParentPath().isPresent() && + !getContext().getCurrentPathStack().contains(block.getParentPath().get()) + ) { + return getContext() + .getCurrentPathStack() + .closeablePush( + block.getParentPath().get(), + block.getParentLineNo(), + block.getParentPosition() + ) + .map(path -> true); + } else { + return AutoCloseableSupplier.of(false); + } + } + /** * Resolve a variable from the interpreter context, returning null if not found. This method updates the template error accumulators when a variable is not found. * @@ -957,10 +982,20 @@ public static Optional getCurrentMaybe() { return Optional.ofNullable(getCurrent()); } + public static AutoCloseableSupplier closeablePushCurrent( + JinjavaInterpreter interpreter + ) { + Stack stack = CURRENT_INTERPRETER.get(); + stack.push(interpreter); + return AutoCloseableSupplier.of(() -> interpreter, i -> stack.pop()); + } + + @Deprecated public static void pushCurrent(JinjavaInterpreter interpreter) { CURRENT_INTERPRETER.get().push(interpreter); } + @Deprecated public static void popCurrent() { if (!CURRENT_INTERPRETER.get().isEmpty()) { CURRENT_INTERPRETER.get().pop(); diff --git a/src/main/java/com/hubspot/jinjava/lib/fn/MacroFunction.java b/src/main/java/com/hubspot/jinjava/lib/fn/MacroFunction.java index 14cdaa47c..39f2cfd5c 100644 --- a/src/main/java/com/hubspot/jinjava/lib/fn/MacroFunction.java +++ b/src/main/java/com/hubspot/jinjava/lib/fn/MacroFunction.java @@ -1,6 +1,8 @@ package com.hubspot.jinjava.lib.fn; import com.hubspot.jinjava.el.ext.AbstractCallableMethod; +import com.hubspot.jinjava.interpret.AutoCloseableSupplier; +import com.hubspot.jinjava.interpret.AutoCloseableSupplier.AutoCloseableImpl; import com.hubspot.jinjava.interpret.Context; import com.hubspot.jinjava.interpret.Context.TemporaryValueClosable; import com.hubspot.jinjava.interpret.DeferredValue; @@ -72,31 +74,39 @@ public Object doEvaluate( List varArgs ) { JinjavaInterpreter interpreter = JinjavaInterpreter.getCurrent(); - Optional importFile = getImportFile(interpreter); - try (InterpreterScopeClosable c = interpreter.enterScope()) { + try ( + InterpreterScopeClosable c = interpreter.enterScope(); + AutoCloseableImpl> importFile = getImportFileWithWrapper( + interpreter + ) + .get() + ) { return getEvaluationResult(argMap, kwargMap, varArgs, interpreter); - } finally { - importFile.ifPresent(path -> interpreter.getContext().getCurrentPathStack().pop()); } } public Optional getImportFile(JinjavaInterpreter interpreter) { + return getImportFileWithWrapper(interpreter).dangerouslyGetWithoutClosing(); + } + + public AutoCloseableSupplier> getImportFileWithWrapper( + JinjavaInterpreter interpreter + ) { Optional importFile = Optional.ofNullable( (String) localContextScope.get(Context.IMPORT_RESOURCE_PATH_KEY) ); - - // pushWithoutCycleCheck() is used to here so that macros calling macros from the same file will not throw a TagCycleException - importFile.ifPresent(path -> - interpreter - .getContext() - .getCurrentPathStack() - .pushWithoutCycleCheck( - path, - interpreter.getLineNumber(), - interpreter.getPosition() - ) - ); - return importFile; + if (importFile.isEmpty()) { + return AutoCloseableSupplier.of(Optional.empty()); + } + return interpreter + .getContext() + .getCurrentPathStack() + .closeablePushWithoutCycleCheck( + importFile.get(), + interpreter.getLineNumber(), + interpreter.getPosition() + ) + .map(Optional::of); } public String getEvaluationResult( diff --git a/src/main/java/com/hubspot/jinjava/lib/fn/eager/EagerMacroFunction.java b/src/main/java/com/hubspot/jinjava/lib/fn/eager/EagerMacroFunction.java index af8d8fbd2..29a4fcf69 100644 --- a/src/main/java/com/hubspot/jinjava/lib/fn/eager/EagerMacroFunction.java +++ b/src/main/java/com/hubspot/jinjava/lib/fn/eager/EagerMacroFunction.java @@ -4,6 +4,8 @@ import com.hubspot.jinjava.el.ext.AstMacroFunction; import com.hubspot.jinjava.el.ext.DeferredInvocationResolutionException; import com.hubspot.jinjava.el.ext.eager.MacroFunctionTempVariable; +import com.hubspot.jinjava.interpret.AutoCloseableSupplier; +import com.hubspot.jinjava.interpret.AutoCloseableSupplier.AutoCloseableImpl; import com.hubspot.jinjava.interpret.Context; import com.hubspot.jinjava.interpret.DeferredMacroValueImpl; import com.hubspot.jinjava.interpret.DeferredValue; @@ -69,8 +71,13 @@ public Object doEvaluate( ) { JinjavaInterpreter interpreter = JinjavaInterpreter.getCurrent(); if (reconstructing) { - Optional importFile = getImportFile(interpreter); - try (InterpreterScopeClosable c = interpreter.enterScope()) { + try ( + InterpreterScopeClosable c = interpreter.enterScope(); + AutoCloseableImpl> importFile = getImportFileWithWrapper( + interpreter + ) + .get() + ) { EagerExecutionResult result = eagerEvaluateInDeferredExecutionMode( () -> getEvaluationResultDirectly(argMap, kwargMap, varArgs, interpreter), interpreter @@ -86,9 +93,6 @@ public Object doEvaluate( ); } return result.asTemplateString(); - } finally { - importFile.ifPresent(path -> interpreter.getContext().getCurrentPathStack().pop() - ); } } @@ -231,10 +235,12 @@ public String reconstructImage(String fullName) { String setTagForAliasedVariables = getSetTagForAliasedVariables(fullName); String suffix = ""; JinjavaInterpreter interpreter = JinjavaInterpreter.getCurrent(); - Optional importFile = getImportFile(interpreter); + + Optional importFile = Optional.ofNullable( + (String) localContextScope.get(Context.IMPORT_RESOURCE_PATH_KEY) + ); Object currentDeferredImportResource = null; if (importFile.isPresent()) { - interpreter.getContext().getCurrentPathStack().pop(); currentDeferredImportResource = interpreter.getContext().get(Context.DEFERRED_IMPORT_RESOURCE_PATH_KEY); if (currentDeferredImportResource instanceof DeferredValue) { @@ -259,12 +265,22 @@ public String reconstructImage(String fullName) { } if ( - (interpreter.getContext().getMacroStack().contains(getName()) && - !differentMacroWithSameNameExists(interpreter)) || - (!isCaller() && AstMacroFunction.checkAndPushMacroStack(interpreter, fullName)) + interpreter.getContext().getMacroStack().contains(getName()) && + !differentMacroWithSameNameExists(interpreter) ) { return ""; - } else { + } + + try ( + AutoCloseableImpl shouldReconstruct = shouldDoReconstruction( + fullName, + interpreter + ) + ) { + if (!shouldReconstruct.value()) { + return ""; + } + try (InterpreterScopeClosable c = interpreter.enterScope()) { reconstructing = true; String evaluation = (String) evaluate( @@ -288,14 +304,30 @@ public String reconstructImage(String fullName) { interpreter .getContext() .put(Context.DEFERRED_IMPORT_RESOURCE_PATH_KEY, currentDeferredImportResource); - if (!isCaller()) { - interpreter.getContext().getMacroStack().pop(); - } } } + return prefix + result + suffix; } + private AutoCloseableImpl shouldDoReconstruction( + String fullName, + JinjavaInterpreter interpreter + ) { + return ( + isCaller() + ? AutoCloseableSupplier.of(true) + : AstMacroFunction + .checkAndPushMacroStackWithWrapper(interpreter, fullName) + .map(result -> + result.match( + err -> false, // cycle detected, don't do reconstruction + ok -> true // no cycle, proceed with reconstruction + ) + ) + ).get(); + } + private String getSetTagForAliasedVariables(String fullName) { int lastDotIdx = fullName.lastIndexOf('.'); if (lastDotIdx > 0) { diff --git a/src/main/java/com/hubspot/jinjava/lib/tag/FromTag.java b/src/main/java/com/hubspot/jinjava/lib/tag/FromTag.java index 8bbb4df65..4e2013728 100644 --- a/src/main/java/com/hubspot/jinjava/lib/tag/FromTag.java +++ b/src/main/java/com/hubspot/jinjava/lib/tag/FromTag.java @@ -3,16 +3,19 @@ import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterators; import com.google.common.collect.PeekingIterator; +import com.hubspot.algebra.Result; import com.hubspot.jinjava.doc.annotations.JinjavaDoc; import com.hubspot.jinjava.doc.annotations.JinjavaParam; import com.hubspot.jinjava.doc.annotations.JinjavaSnippet; import com.hubspot.jinjava.doc.annotations.JinjavaTextMateSnippet; +import com.hubspot.jinjava.interpret.AutoCloseableSupplier; +import com.hubspot.jinjava.interpret.AutoCloseableSupplier.AutoCloseableImpl; import com.hubspot.jinjava.interpret.Context; import com.hubspot.jinjava.interpret.DeferredValue; import com.hubspot.jinjava.interpret.DeferredValueException; -import com.hubspot.jinjava.interpret.FromTagCycleException; import com.hubspot.jinjava.interpret.InterpretException; import com.hubspot.jinjava.interpret.JinjavaInterpreter; +import com.hubspot.jinjava.interpret.TagCycleException; import com.hubspot.jinjava.interpret.TemplateError; import com.hubspot.jinjava.interpret.TemplateError.ErrorItem; import com.hubspot.jinjava.interpret.TemplateError.ErrorReason; @@ -29,6 +32,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import org.apache.commons.lang3.StringUtils; @JinjavaDoc( value = "Alternative to the import tag that lets you import and use specific macros from one template to another", @@ -72,59 +76,77 @@ public String getName() { public String interpret(TagNode tagNode, JinjavaInterpreter interpreter) { List helper = getHelpers((TagToken) tagNode.getMaster()); - Optional maybeTemplateFile = getTemplateFile( - helper, - (TagToken) tagNode.getMaster(), - interpreter - ); - if (!maybeTemplateFile.isPresent()) { - return ""; - } - String templateFile = maybeTemplateFile.get(); - try { - Map imports = getImportMap(helper); + try ( + AutoCloseableImpl> templateFileResult = + getTemplateFileWithWrapper(helper, (TagToken) tagNode.getMaster(), interpreter) + .get() + ) { + return templateFileResult + .value() + .match( + err -> { + String path = StringUtils.trimToEmpty(helper.get(0)); + interpreter.addError( + new TemplateError( + ErrorType.WARNING, + ErrorReason.EXCEPTION, + ErrorItem.TAG, + "From cycle detected for path: '" + path + "'", + null, + ((TagToken) tagNode.getMaster()).getLineNumber(), + ((TagToken) tagNode.getMaster()).getStartPosition(), + err, + BasicTemplateErrorCategory.FROM_CYCLE_DETECTED, + ImmutableMap.of("path", path) + ) + ); + return ""; + }, + templateFile -> { + Map imports = getImportMap(helper); - try { - String template = interpreter.getResource(templateFile); - Node node = interpreter.parse(template); + try { + String template = interpreter.getResource(templateFile); + Node node = interpreter.parse(template); - JinjavaInterpreter child = interpreter - .getConfig() - .getInterpreterFactory() - .newInstance(interpreter); - child.getContext().put(Context.IMPORT_RESOURCE_PATH_KEY, templateFile); - JinjavaInterpreter.pushCurrent(child); - try { - child.render(node); - } finally { - JinjavaInterpreter.popCurrent(); - } + JinjavaInterpreter child = interpreter + .getConfig() + .getInterpreterFactory() + .newInstance(interpreter); + child.getContext().put(Context.IMPORT_RESOURCE_PATH_KEY, templateFile); + try ( + AutoCloseableImpl a = JinjavaInterpreter + .closeablePushCurrent(child) + .get() + ) { + child.render(node); + } - interpreter.addAllChildErrors(templateFile, child.getErrorsCopy()); + interpreter.addAllChildErrors(templateFile, child.getErrorsCopy()); - boolean importsDeferredValue = integrateChild(imports, child, interpreter); + boolean importsDeferredValue = integrateChild(imports, child, interpreter); - if (importsDeferredValue) { - handleDeferredNodesDuringImport( - (TagToken) tagNode.getMaster(), - templateFile, - imports, - child, - interpreter - ); - } + if (importsDeferredValue) { + handleDeferredNodesDuringImport( + (TagToken) tagNode.getMaster(), + templateFile, + imports, + child, + interpreter + ); + } - return ""; - } catch (IOException e) { - throw new InterpretException( - e.getMessage(), - e, - tagNode.getLineNumber(), - tagNode.getStartPosition() + return ""; + } catch (IOException e) { + throw new InterpretException( + e.getMessage(), + e, + tagNode.getLineNumber(), + tagNode.getStartPosition() + ); + } + } ); - } - } finally { - interpreter.getContext().popFromStack(); } } @@ -208,7 +230,7 @@ public static Map getImportMap(List helper) { return imports; } - public static Optional getTemplateFile( + public static AutoCloseableSupplier> getTemplateFileWithWrapper( List helper, TagToken tagToken, JinjavaInterpreter interpreter @@ -220,32 +242,40 @@ public static Optional getTemplateFile( ); templateFile = interpreter.resolveResourceLocation(templateFile); interpreter.getContext().addDependency("coded_files", templateFile); - try { - interpreter - .getContext() - .pushFromStack( - templateFile, - tagToken.getLineNumber(), - tagToken.getStartPosition() - ); - } catch (FromTagCycleException e) { - interpreter.addError( - new TemplateError( - ErrorType.WARNING, - ErrorReason.EXCEPTION, - ErrorItem.TAG, - "From cycle detected for path: '" + templateFile + "'", - null, - tagToken.getLineNumber(), - tagToken.getStartPosition(), - e, - BasicTemplateErrorCategory.FROM_CYCLE_DETECTED, - ImmutableMap.of("path", templateFile) - ) + return interpreter + .getContext() + .getFromPathStack() + .closeablePush(templateFile, tagToken.getLineNumber(), tagToken.getStartPosition()); + } + + @Deprecated + public static Optional getTemplateFile( + List helper, + TagToken tagToken, + JinjavaInterpreter interpreter + ) { + return getTemplateFileWithWrapper(helper, tagToken, interpreter) + .dangerouslyGetWithoutClosing() + .match( + err -> { + interpreter.addError( + new TemplateError( + ErrorType.WARNING, + ErrorReason.EXCEPTION, + ErrorItem.TAG, + "From cycle detected for path: '" + err.getPath() + "'", + null, + tagToken.getLineNumber(), + tagToken.getStartPosition(), + err, + BasicTemplateErrorCategory.FROM_CYCLE_DETECTED, + ImmutableMap.of("path", err.getPath()) + ) + ); + return Optional.empty(); + }, + Optional::of ); - return Optional.empty(); - } - return Optional.of(templateFile); } public static List getHelpers(TagToken tagToken) { diff --git a/src/main/java/com/hubspot/jinjava/lib/tag/ImportTag.java b/src/main/java/com/hubspot/jinjava/lib/tag/ImportTag.java index aa9ffbd30..8c73d4947 100644 --- a/src/main/java/com/hubspot/jinjava/lib/tag/ImportTag.java +++ b/src/main/java/com/hubspot/jinjava/lib/tag/ImportTag.java @@ -1,16 +1,19 @@ package com.hubspot.jinjava.lib.tag; import com.google.common.collect.ImmutableMap; +import com.hubspot.algebra.Result; import com.hubspot.jinjava.doc.annotations.JinjavaDoc; import com.hubspot.jinjava.doc.annotations.JinjavaParam; import com.hubspot.jinjava.doc.annotations.JinjavaSnippet; import com.hubspot.jinjava.doc.annotations.JinjavaTextMateSnippet; +import com.hubspot.jinjava.interpret.AutoCloseableSupplier; +import com.hubspot.jinjava.interpret.AutoCloseableSupplier.AutoCloseableImpl; import com.hubspot.jinjava.interpret.Context; import com.hubspot.jinjava.interpret.DeferredValue; import com.hubspot.jinjava.interpret.DeferredValueException; -import com.hubspot.jinjava.interpret.ImportTagCycleException; import com.hubspot.jinjava.interpret.InterpretException; import com.hubspot.jinjava.interpret.JinjavaInterpreter; +import com.hubspot.jinjava.interpret.TagCycleException; import com.hubspot.jinjava.interpret.TemplateError; import com.hubspot.jinjava.interpret.TemplateError.ErrorItem; import com.hubspot.jinjava.interpret.TemplateError.ErrorReason; @@ -81,64 +84,86 @@ public String interpret(TagNode tagNode, JinjavaInterpreter interpreter) { String contextVar = getContextVar(helper); - Optional maybeTemplateFile = getTemplateFile( - helper, - (TagToken) tagNode.getMaster(), - interpreter - ); - if (!maybeTemplateFile.isPresent()) { - return ""; - } - String templateFile = maybeTemplateFile.get(); - try { - Node node = parseTemplateAsNode(interpreter, templateFile); - - JinjavaInterpreter child = interpreter - .getConfig() - .getInterpreterFactory() - .newInstance(interpreter); - child.getContext().put(Context.IMPORT_RESOURCE_PATH_KEY, templateFile); + try ( + AutoCloseableImpl> templateFileResult = + getTemplateFileWithWrapper(helper, (TagToken) tagNode.getMaster(), interpreter) + .get() + ) { + return templateFileResult + .value() + .match( + err -> { + String path = StringUtils.trimToEmpty(helper.get(0)); + interpreter.addError( + new TemplateError( + ErrorType.WARNING, + ErrorReason.EXCEPTION, + ErrorItem.TAG, + "Import cycle detected for path: '" + path + "'", + null, + ((TagToken) tagNode.getMaster()).getLineNumber(), + ((TagToken) tagNode.getMaster()).getStartPosition(), + err, + BasicTemplateErrorCategory.IMPORT_CYCLE_DETECTED, + ImmutableMap.of("path", path) + ) + ); + return ""; + }, + templateFile -> { + try ( + AutoCloseableImpl node = parseTemplateAsNode( + interpreter, + templateFile + ) + .get() + ) { + JinjavaInterpreter child = interpreter + .getConfig() + .getInterpreterFactory() + .newInstance(interpreter); + child.getContext().put(Context.IMPORT_RESOURCE_PATH_KEY, templateFile); - JinjavaInterpreter.pushCurrent(child); + try ( + AutoCloseableImpl a = JinjavaInterpreter + .closeablePushCurrent(child) + .get() + ) { + child.render(node.value()); + } - try { - child.render(node); - } finally { - JinjavaInterpreter.popCurrent(); - } + interpreter.addAllChildErrors(templateFile, child.getErrorsCopy()); - interpreter.addAllChildErrors(templateFile, child.getErrorsCopy()); + Map childBindings = child.getContext().getSessionBindings(); - Map childBindings = child.getContext().getSessionBindings(); + // If the template depends on deferred values it should not be rendered and all defined variables and macros should be deferred too + if (!child.getContext().getDeferredNodes().isEmpty()) { + handleDeferredNodesDuringImport( + node.value(), + contextVar, + childBindings, + child, + interpreter + ); + throw new DeferredValueException( + templateFile, + tagNode.getLineNumber(), + tagNode.getStartPosition() + ); + } - // If the template depends on deferred values it should not be rendered and all defined variables and macros should be deferred too - if (!child.getContext().getDeferredNodes().isEmpty()) { - handleDeferredNodesDuringImport( - node, - contextVar, - childBindings, - child, - interpreter - ); - throw new DeferredValueException( - templateFile, - tagNode.getLineNumber(), - tagNode.getStartPosition() + integrateChild(contextVar, childBindings, child, interpreter); + return ""; + } catch (IOException e) { + throw new InterpretException( + e.getMessage(), + e, + tagNode.getLineNumber(), + tagNode.getStartPosition() + ); + } + } ); - } - - integrateChild(contextVar, childBindings, child, interpreter); - return ""; - } catch (IOException e) { - throw new InterpretException( - e.getMessage(), - e, - tagNode.getLineNumber(), - tagNode.getStartPosition() - ); - } finally { - interpreter.getContext().getCurrentPathStack().pop(); - interpreter.getContext().getImportPathStack().pop(); } } @@ -210,20 +235,19 @@ public static void handleDeferredNodesDuringImport( } } - public static Node parseTemplateAsNode( + public static AutoCloseableSupplier parseTemplateAsNode( JinjavaInterpreter interpreter, String templateFile ) throws IOException { - interpreter + String template = interpreter.getResource(templateFile); + return interpreter .getContext() .getCurrentPathStack() - .push(templateFile, interpreter.getLineNumber(), interpreter.getPosition()); - - String template = interpreter.getResource(templateFile); - return interpreter.parse(template); + .closeablePush(templateFile, interpreter.getLineNumber(), interpreter.getPosition()) + .map(currentPath -> interpreter.parse(template)); } - public static Optional getTemplateFile( + public static AutoCloseableSupplier> getTemplateFileWithWrapper( List helper, TagToken tagToken, JinjavaInterpreter interpreter @@ -236,29 +260,41 @@ public static Optional getTemplateFile( ); templateFile = interpreter.resolveResourceLocation(templateFile); interpreter.getContext().addDependency("coded_files", templateFile); - try { - interpreter - .getContext() - .getImportPathStack() - .push(path, tagToken.getLineNumber(), tagToken.getStartPosition()); - } catch (ImportTagCycleException e) { - interpreter.addError( - new TemplateError( - ErrorType.WARNING, - ErrorReason.EXCEPTION, - ErrorItem.TAG, - "Import cycle detected for path: '" + path + "'", - null, - tagToken.getLineNumber(), - tagToken.getStartPosition(), - e, - BasicTemplateErrorCategory.IMPORT_CYCLE_DETECTED, - ImmutableMap.of("path", path) - ) + return interpreter + .getContext() + .getImportPathStack() + .closeablePush(templateFile, tagToken.getLineNumber(), tagToken.getStartPosition()); + } + + @Deprecated + public static Optional getTemplateFile( + List helper, + TagToken tagToken, + JinjavaInterpreter interpreter + ) { + return getTemplateFileWithWrapper(helper, tagToken, interpreter) + .dangerouslyGetWithoutClosing() + .match( + err -> { + String path = StringUtils.trimToEmpty(helper.get(0)); + interpreter.addError( + new TemplateError( + ErrorType.WARNING, + ErrorReason.EXCEPTION, + ErrorItem.TAG, + "Import cycle detected for path: '" + path + "'", + null, + tagToken.getLineNumber(), + tagToken.getStartPosition(), + err, + BasicTemplateErrorCategory.IMPORT_CYCLE_DETECTED, + ImmutableMap.of("path", path) + ) + ); + return Optional.empty(); + }, + ok -> Optional.of(ok) ); - return Optional.empty(); - } - return Optional.of(templateFile); } public static String getContextVar(List helper) { diff --git a/src/main/java/com/hubspot/jinjava/lib/tag/IncludeTag.java b/src/main/java/com/hubspot/jinjava/lib/tag/IncludeTag.java index cd5ad675e..ff06a1e5f 100644 --- a/src/main/java/com/hubspot/jinjava/lib/tag/IncludeTag.java +++ b/src/main/java/com/hubspot/jinjava/lib/tag/IncludeTag.java @@ -16,13 +16,15 @@ package com.hubspot.jinjava.lib.tag; import com.google.common.collect.ImmutableMap; +import com.hubspot.algebra.Result; import com.hubspot.jinjava.doc.annotations.JinjavaDoc; import com.hubspot.jinjava.doc.annotations.JinjavaParam; import com.hubspot.jinjava.doc.annotations.JinjavaSnippet; import com.hubspot.jinjava.doc.annotations.JinjavaTextMateSnippet; -import com.hubspot.jinjava.interpret.IncludeTagCycleException; +import com.hubspot.jinjava.interpret.AutoCloseableSupplier.AutoCloseableImpl; import com.hubspot.jinjava.interpret.InterpretException; import com.hubspot.jinjava.interpret.JinjavaInterpreter; +import com.hubspot.jinjava.interpret.TagCycleException; import com.hubspot.jinjava.interpret.TemplateError; import com.hubspot.jinjava.interpret.TemplateError.ErrorItem; import com.hubspot.jinjava.interpret.TemplateError.ErrorReason; @@ -76,50 +78,90 @@ public String interpret(TagNode tagNode, JinjavaInterpreter interpreter) { ); templateFile = interpreter.resolveResourceLocation(templateFile); - try { - interpreter - .getContext() - .getIncludePathStack() - .push(templateFile, tagNode.getLineNumber(), tagNode.getStartPosition()); - } catch (IncludeTagCycleException e) { - interpreter.addError( - new TemplateError( - ErrorType.WARNING, - ErrorReason.EXCEPTION, - ErrorItem.TAG, - "Include cycle detected for path: '" + templateFile + "'", - null, - tagNode.getLineNumber(), - tagNode.getStartPosition(), - e, - BasicTemplateErrorCategory.INCLUDE_CYCLE_DETECTED, - ImmutableMap.of("path", templateFile) - ) - ); - return ""; - } - - try { - String template = interpreter.getResource(templateFile); - Node node = interpreter.parse(template); - - interpreter.getContext().addDependency("coded_files", templateFile); - interpreter - .getContext() - .getCurrentPathStack() - .push(templateFile, interpreter.getLineNumber(), interpreter.getPosition()); - - return interpreter.render(node, false); - } catch (IOException e) { - throw new InterpretException( - e.getMessage(), - e, - tagNode.getLineNumber(), - tagNode.getStartPosition() - ); - } finally { - interpreter.getContext().getIncludePathStack().pop(); - interpreter.getContext().getCurrentPathStack().pop(); + final String finalTemplateFile = templateFile; + final TagNode finalTagNode = tagNode; + try ( + AutoCloseableImpl> includeStackWrapper = + interpreter + .getContext() + .getIncludePathStack() + .closeablePush( + finalTemplateFile, + tagNode.getLineNumber(), + tagNode.getStartPosition() + ) + .get(); + AutoCloseableImpl> currentPathWrapper = + interpreter + .getContext() + .getCurrentPathStack() + .closeablePush( + finalTemplateFile, + interpreter.getLineNumber(), + interpreter.getPosition() + ) + .get() + ) { + return includeStackWrapper + .value() + .match( + err -> { + interpreter.addError( + new TemplateError( + ErrorType.WARNING, + ErrorReason.EXCEPTION, + ErrorItem.TAG, + "Include cycle detected for path: '" + finalTemplateFile + "'", + null, + finalTagNode.getLineNumber(), + finalTagNode.getStartPosition(), + err, + BasicTemplateErrorCategory.INCLUDE_CYCLE_DETECTED, + ImmutableMap.of("path", finalTemplateFile) + ) + ); + return ""; + }, + includeStackPath -> + currentPathWrapper + .value() + .match( + err -> { + interpreter.addError( + new TemplateError( + ErrorType.WARNING, + ErrorReason.EXCEPTION, + ErrorItem.TAG, + "Include cycle detected for path: '" + finalTemplateFile + "'", + null, + finalTagNode.getLineNumber(), + finalTagNode.getStartPosition(), + err, + BasicTemplateErrorCategory.INCLUDE_CYCLE_DETECTED, + ImmutableMap.of("path", finalTemplateFile) + ) + ); + return ""; + }, + currentPath -> { + try { + String template = interpreter.getResource(finalTemplateFile); + Node node = interpreter.parse(template); + interpreter + .getContext() + .addDependency("coded_files", finalTemplateFile); + return interpreter.render(node, false); + } catch (IOException e) { + throw new InterpretException( + e.getMessage(), + e, + finalTagNode.getLineNumber(), + finalTagNode.getStartPosition() + ); + } + } + ) + ); } } diff --git a/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerFromTag.java b/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerFromTag.java index 6514a82a0..2e2f4f0d5 100644 --- a/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerFromTag.java +++ b/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerFromTag.java @@ -1,10 +1,20 @@ package com.hubspot.jinjava.lib.tag.eager; import com.google.common.annotations.Beta; +import com.google.common.collect.ImmutableMap; +import com.hubspot.algebra.Result; +import com.hubspot.jinjava.interpret.AutoCloseableSupplier; +import com.hubspot.jinjava.interpret.AutoCloseableSupplier.AutoCloseableImpl; import com.hubspot.jinjava.interpret.Context; import com.hubspot.jinjava.interpret.DeferredValueException; import com.hubspot.jinjava.interpret.InterpretException; import com.hubspot.jinjava.interpret.JinjavaInterpreter; +import com.hubspot.jinjava.interpret.TagCycleException; +import com.hubspot.jinjava.interpret.TemplateError; +import com.hubspot.jinjava.interpret.TemplateError.ErrorItem; +import com.hubspot.jinjava.interpret.TemplateError.ErrorReason; +import com.hubspot.jinjava.interpret.TemplateError.ErrorType; +import com.hubspot.jinjava.interpret.errorcategory.BasicTemplateErrorCategory; import com.hubspot.jinjava.lib.fn.MacroFunction; import com.hubspot.jinjava.lib.fn.eager.EagerMacroFunction; import com.hubspot.jinjava.lib.tag.DoTag; @@ -18,7 +28,6 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; -import java.util.Optional; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -40,9 +49,10 @@ public String getEagerTagImage(TagToken tagToken, JinjavaInterpreter interpreter ); List helper = FromTag.getHelpers(tagToken); Map imports = FromTag.getImportMap(helper); - Optional maybeTemplateFile; + AutoCloseableSupplier> maybeTemplateFileSupplier; try { - maybeTemplateFile = FromTag.getTemplateFile(helper, tagToken, interpreter); + maybeTemplateFileSupplier = + FromTag.getTemplateFileWithWrapper(helper, tagToken, interpreter); } catch (DeferredValueException e) { imports .values() @@ -75,69 +85,94 @@ public String getEagerTagImage(TagToken tagToken, JinjavaInterpreter interpreter tagToken.getImage() ); } - if (!maybeTemplateFile.isPresent()) { - return ""; - } - String templateFile = maybeTemplateFile.get(); - try { - try { - String template = interpreter.getResource(templateFile); - Node node = interpreter.parse(template); + try ( + AutoCloseableImpl> maybeTemplateFile = + maybeTemplateFileSupplier.get() + ) { + return maybeTemplateFile + .value() + .match( + err -> { + interpreter.addError( + new TemplateError( + ErrorType.WARNING, + ErrorReason.EXCEPTION, + ErrorItem.TAG, + "From cycle detected for path: '" + err.getPath() + "'", + null, + tagToken.getLineNumber(), + tagToken.getStartPosition(), + err, + BasicTemplateErrorCategory.FROM_CYCLE_DETECTED, + ImmutableMap.of("path", err.getPath()) + ) + ); + return ""; + }, + templateFile -> { + try { + String template = interpreter.getResource(templateFile); + Node node = interpreter.parse(template); - JinjavaInterpreter child = interpreter - .getConfig() - .getInterpreterFactory() - .newInstance(interpreter); - child.getContext().put(Context.IMPORT_RESOURCE_PATH_KEY, templateFile); - JinjavaInterpreter.pushCurrent(child); - String output; - try { - output = child.render(node); - } finally { - JinjavaInterpreter.popCurrent(); - } + JinjavaInterpreter child = interpreter + .getConfig() + .getInterpreterFactory() + .newInstance(interpreter); + child.getContext().put(Context.IMPORT_RESOURCE_PATH_KEY, templateFile); + String output; + try ( + AutoCloseableImpl a = JinjavaInterpreter + .closeablePushCurrent(child) + .get() + ) { + output = child.render(node); + } - interpreter.addAllChildErrors(templateFile, child.getErrorsCopy()); + interpreter.addAllChildErrors(templateFile, child.getErrorsCopy()); - if (!child.getContext().getDeferredNodes().isEmpty()) { - FromTag.handleDeferredNodesDuringImport( - tagToken, - templateFile, - imports, - child, - interpreter - ); - } + if (!child.getContext().getDeferredNodes().isEmpty()) { + FromTag.handleDeferredNodesDuringImport( + tagToken, + templateFile, + imports, + child, + interpreter + ); + } - FromTag.integrateChild(imports, child, interpreter); - Map newToOldImportNames = getNewToOldWithoutMacros( - imports, - interpreter - ); - if (child.getContext().getDeferredTokens().isEmpty() || output == null) { - return ""; - } else if (newToOldImportNames.size() > 0) { - // Set after output - output = - output + - EagerReconstructionUtils.buildSetTag(newToOldImportNames, interpreter, true); - } - return EagerReconstructionUtils.wrapInTag( - output, - DoTag.TAG_NAME, - interpreter, - true - ); - } catch (IOException e) { - throw new InterpretException( - e.getMessage(), - e, - tagToken.getLineNumber(), - tagToken.getStartPosition() + FromTag.integrateChild(imports, child, interpreter); + Map newToOldImportNames = getNewToOldWithoutMacros( + imports, + interpreter + ); + if (child.getContext().getDeferredTokens().isEmpty() || output == null) { + return ""; + } else if (newToOldImportNames.size() > 0) { + // Set after output + output = + output + + EagerReconstructionUtils.buildSetTag( + newToOldImportNames, + interpreter, + true + ); + } + return EagerReconstructionUtils.wrapInTag( + output, + DoTag.TAG_NAME, + interpreter, + true + ); + } catch (IOException e) { + throw new InterpretException( + e.getMessage(), + e, + tagToken.getLineNumber(), + tagToken.getStartPosition() + ); + } + } ); - } - } finally { - interpreter.getContext().popFromStack(); } } diff --git a/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerImportTag.java b/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerImportTag.java index 6671aaee8..e6fc2f945 100644 --- a/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerImportTag.java +++ b/src/main/java/com/hubspot/jinjava/lib/tag/eager/EagerImportTag.java @@ -1,10 +1,19 @@ package com.hubspot.jinjava.lib.tag.eager; import com.google.common.annotations.Beta; +import com.google.common.collect.ImmutableMap; +import com.hubspot.algebra.Result; +import com.hubspot.jinjava.interpret.AutoCloseableSupplier.AutoCloseableImpl; import com.hubspot.jinjava.interpret.Context; import com.hubspot.jinjava.interpret.DeferredValueException; import com.hubspot.jinjava.interpret.InterpretException; import com.hubspot.jinjava.interpret.JinjavaInterpreter; +import com.hubspot.jinjava.interpret.TagCycleException; +import com.hubspot.jinjava.interpret.TemplateError; +import com.hubspot.jinjava.interpret.TemplateError.ErrorItem; +import com.hubspot.jinjava.interpret.TemplateError.ErrorReason; +import com.hubspot.jinjava.interpret.TemplateError.ErrorType; +import com.hubspot.jinjava.interpret.errorcategory.BasicTemplateErrorCategory; import com.hubspot.jinjava.lib.tag.DoTag; import com.hubspot.jinjava.lib.tag.ImportTag; import com.hubspot.jinjava.lib.tag.eager.importing.EagerImportingStrategy; @@ -15,7 +24,7 @@ import com.hubspot.jinjava.util.EagerReconstructionUtils; import java.io.IOException; import java.util.Map; -import java.util.Optional; +import org.apache.commons.lang3.StringUtils; @Beta public class EagerImportTag extends EagerStateChangingTag { @@ -38,80 +47,101 @@ public String getEagerTagImage(TagToken tagToken, JinjavaInterpreter interpreter importingData ); - Optional maybeTemplateFile; - try { - maybeTemplateFile = - ImportTag.getTemplateFile(importingData.getHelpers(), tagToken, interpreter); - } catch (DeferredValueException e) { - return eagerImportingStrategy.handleDeferredTemplateFile(e); - } - if (!maybeTemplateFile.isPresent()) { - return ""; - } - String templateFile = maybeTemplateFile.get(); - try { - Node node = ImportTag.parseTemplateAsNode(interpreter, templateFile); + try ( + AutoCloseableImpl> templateFileResult = ImportTag + .getTemplateFileWithWrapper(importingData.getHelpers(), tagToken, interpreter) + .get() + ) { + return templateFileResult + .value() + .match( + err -> { + String path = StringUtils.trimToEmpty(importingData.getHelpers().get(0)); + interpreter.addError( + new TemplateError( + ErrorType.WARNING, + ErrorReason.EXCEPTION, + ErrorItem.TAG, + "Import cycle detected for path: '" + path + "'", + null, + tagToken.getLineNumber(), + tagToken.getStartPosition(), + err, + BasicTemplateErrorCategory.IMPORT_CYCLE_DETECTED, + ImmutableMap.of("path", path) + ) + ); + return ""; + }, + templateFile -> { + try ( + AutoCloseableImpl node = ImportTag + .parseTemplateAsNode(interpreter, templateFile) + .get() + ) { + JinjavaInterpreter child = interpreter + .getConfig() + .getInterpreterFactory() + .newInstance(interpreter); + child.getContext().put(Context.IMPORT_RESOURCE_PATH_KEY, templateFile); + String output; + try ( + AutoCloseableImpl a = JinjavaInterpreter + .closeablePushCurrent(child) + .get() + ) { + eagerImportingStrategy.setup(child); + output = child.render(node.value()); + } + interpreter.addAllChildErrors(templateFile, child.getErrorsCopy()); + Map childBindings = child.getContext().getSessionBindings(); - JinjavaInterpreter child = interpreter - .getConfig() - .getInterpreterFactory() - .newInstance(interpreter); - child.getContext().put(Context.IMPORT_RESOURCE_PATH_KEY, templateFile); - JinjavaInterpreter.pushCurrent(child); - String output; - try { - eagerImportingStrategy.setup(child); - output = child.render(node); - } finally { - JinjavaInterpreter.popCurrent(); - } - interpreter.addAllChildErrors(templateFile, child.getErrorsCopy()); - Map childBindings = child.getContext().getSessionBindings(); - - // If the template depends on deferred values it should not be rendered, - // and all defined variables and macros should be deferred too. - if ( - !child.getContext().getDeferredNodes().isEmpty() || - (interpreter.getContext().isDeferredExecutionMode() && - !child.getContext().getGlobalMacros().isEmpty()) - ) { - ImportTag.handleDeferredNodesDuringImport( - node, - ImportTag.getContextVar(importingData.getHelpers()), - childBindings, - child, - interpreter - ); - throw new DeferredValueException( - templateFile, - tagToken.getLineNumber(), - tagToken.getStartPosition() + // If the template depends on deferred values it should not be rendered, + // and all defined variables and macros should be deferred too. + if ( + !child.getContext().getDeferredNodes().isEmpty() || + (interpreter.getContext().isDeferredExecutionMode() && + !child.getContext().getGlobalMacros().isEmpty()) + ) { + ImportTag.handleDeferredNodesDuringImport( + node.value(), + ImportTag.getContextVar(importingData.getHelpers()), + childBindings, + child, + interpreter + ); + throw new DeferredValueException( + templateFile, + tagToken.getLineNumber(), + tagToken.getStartPosition() + ); + } + eagerImportingStrategy.integrateChild(child); + if (child.getContext().getDeferredTokens().isEmpty() || output == null) { + return ""; + } + return EagerReconstructionUtils.wrapInTag( + EagerReconstructionUtils.wrapPathAroundText( + eagerImportingStrategy.getFinalOutput(output, child), + templateFile, + interpreter + ), + DoTag.TAG_NAME, + interpreter, + true + ); + } catch (IOException e) { + throw new InterpretException( + e.getMessage(), + e, + tagToken.getLineNumber(), + tagToken.getStartPosition() + ); + } + } ); - } - eagerImportingStrategy.integrateChild(child); - if (child.getContext().getDeferredTokens().isEmpty() || output == null) { - return ""; - } - return EagerReconstructionUtils.wrapInTag( - EagerReconstructionUtils.wrapPathAroundText( - eagerImportingStrategy.getFinalOutput(output, child), - templateFile, - interpreter - ), - DoTag.TAG_NAME, - interpreter, - true - ); - } catch (IOException e) { - throw new InterpretException( - e.getMessage(), - e, - tagToken.getLineNumber(), - tagToken.getStartPosition() - ); - } finally { - interpreter.getContext().getCurrentPathStack().pop(); - interpreter.getContext().getImportPathStack().pop(); + } catch (DeferredValueException e) { + return eagerImportingStrategy.handleDeferredTemplateFile(e); } } } diff --git a/src/test/java/com/hubspot/jinjava/lib/fn/TodayFunctionTest.java b/src/test/java/com/hubspot/jinjava/lib/fn/TodayFunctionTest.java index 659cca991..1179a3a34 100644 --- a/src/test/java/com/hubspot/jinjava/lib/fn/TodayFunctionTest.java +++ b/src/test/java/com/hubspot/jinjava/lib/fn/TodayFunctionTest.java @@ -5,6 +5,7 @@ import com.hubspot.jinjava.BaseInterpretingTest; import com.hubspot.jinjava.Jinjava; import com.hubspot.jinjava.JinjavaConfig; +import com.hubspot.jinjava.interpret.AutoCloseableSupplier.AutoCloseableImpl; import com.hubspot.jinjava.interpret.Context; import com.hubspot.jinjava.interpret.InvalidArgumentException; import com.hubspot.jinjava.interpret.JinjavaInterpreter; @@ -30,21 +31,22 @@ public void itDefaultsToUtcTimezone() { public void itUsesFixedDateTimeProvider() { long ts = 1233333414223L; - JinjavaInterpreter.pushCurrent( - new JinjavaInterpreter( - new Jinjava(), - new Context(), - JinjavaConfig - .newBuilder() - .withDateTimeProvider(new FixedDateTimeProvider(ts)) - .build() - ) - ); - try { + try ( + AutoCloseableImpl a = JinjavaInterpreter + .closeablePushCurrent( + new JinjavaInterpreter( + new Jinjava(), + new Context(), + JinjavaConfig + .newBuilder() + .withDateTimeProvider(new FixedDateTimeProvider(ts)) + .build() + ) + ) + .get() + ) { assertThat(Functions.today(ZONE_NAME)) .isEqualTo(ZonedDateTime.of(2009, 1, 30, 0, 0, 0, 0, ZONE_ID)); - } finally { - JinjavaInterpreter.popCurrent(); } } @@ -64,19 +66,24 @@ public void itIgnoresNullTimezone() { assertThat(Functions.today((String) null).getZone()).isEqualTo(ZoneOffset.UTC); } + @Test public void itDefersWhenExecutingEagerly() { - JinjavaInterpreter.pushCurrent( - new JinjavaInterpreter( - new Jinjava(), - new Context(), - JinjavaConfig - .newBuilder() - .withExecutionMode(EagerExecutionMode.instance()) - .build() - ) - ); - - ZonedDateTime today = Functions.today(ZONE_NAME); - assertThat(today.getYear()).isGreaterThan(2023); + try ( + AutoCloseableImpl a = JinjavaInterpreter + .closeablePushCurrent( + new JinjavaInterpreter( + new Jinjava(), + new Context(), + JinjavaConfig + .newBuilder() + .withExecutionMode(EagerExecutionMode.instance()) + .build() + ) + ) + .get() + ) { + ZonedDateTime today = Functions.today(ZONE_NAME); + assertThat(today.getYear()).isGreaterThan(2023); + } } }