From 3d8a2e473c6bb0da2681b77cc0067671f1029390 Mon Sep 17 00:00:00 2001 From: Luke Sandberg Date: Thu, 19 Dec 2024 16:41:11 -0800 Subject: [PATCH] Allow server side implementations of SoyLogger to output `LoggingAttrs` to set attributes on velog roots This follows the js implementation and adds integration tests to cover all backends in addition to unit tests. PiperOrigin-RevId: 708080550 --- .../soy/data/LoggingFunctionInvocation.java | 2 +- .../soy/jbcsrc/api/OutputAppendable.java | 44 ++++- .../google/template/soy/logging/BUILD.bazel | 2 + .../template/soy/logging/SoyLogger.java | 181 +++++++++++++++++- .../template/soy/jbcsrc/VeLoggingTest.java | 54 +++++- .../soy/jbcsrc/api/OutputAppendableTest.java | 4 +- .../google/template/soy/logging/BUILD.bazel | 39 ++++ .../template/soy/logging/SoyLoggerTest.java | 64 +++++++ 8 files changed, 375 insertions(+), 15 deletions(-) create mode 100644 java/tests/com/google/template/soy/logging/BUILD.bazel create mode 100644 java/tests/com/google/template/soy/logging/SoyLoggerTest.java diff --git a/java/src/com/google/template/soy/data/LoggingFunctionInvocation.java b/java/src/com/google/template/soy/data/LoggingFunctionInvocation.java index 6fd77c0aa..8d604f488 100644 --- a/java/src/com/google/template/soy/data/LoggingFunctionInvocation.java +++ b/java/src/com/google/template/soy/data/LoggingFunctionInvocation.java @@ -30,7 +30,7 @@ public abstract class LoggingFunctionInvocation { new AutoValue_LoggingFunctionInvocation( "$$flushPendingAttributes", "", - ImmutableList.of(BooleanData.FALSE), + ImmutableList.of(BooleanData.TRUE), /* isFlushPendingAttributes= */ true, Optional.>empty()); diff --git a/java/src/com/google/template/soy/jbcsrc/api/OutputAppendable.java b/java/src/com/google/template/soy/jbcsrc/api/OutputAppendable.java index 23899dc98..6533d3359 100644 --- a/java/src/com/google/template/soy/jbcsrc/api/OutputAppendable.java +++ b/java/src/com/google/template/soy/jbcsrc/api/OutputAppendable.java @@ -19,6 +19,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.flogger.GoogleLogger; +import com.google.common.flogger.StackSize; import com.google.common.html.types.SafeHtml; import com.google.errorprone.annotations.CanIgnoreReturnValue; import com.google.template.soy.data.LogStatement; @@ -36,7 +37,6 @@ *

This object is for soy internal use only. Do not use. */ public final class OutputAppendable extends LoggingAdvisingAppendable { - public static OutputAppendable create(Appendable outputAppendable, @Nullable SoyLogger logger) { return new OutputAppendable(outputAppendable, logger); } @@ -54,6 +54,7 @@ public static OutputAppendable create(StringBuilder sb, @Nullable SoyLogger logg @Nullable private final SoyLogger logger; private final Appendable outputAppendable; private int logOnlyDepth; + @Nullable private SoyLogger.LoggingAttrs loggingAttrs; private OutputAppendable(Appendable outputAppendable, @Nullable SoyLogger logger) { this.outputAppendable = checkNotNull(outputAppendable); @@ -108,9 +109,8 @@ public LoggingAdvisingAppendable appendLoggingFunctionInvocation( value = funCall.placeholderValue(); } else { if (funCall.isFlushPendingAttributes()) { - // For now, just no-op these calls. - // TODO-b/383661457: implement this. - value = ""; + maybeFlushPendingAttributes(funCall); + return this; } else { value = logger.evalLoggingFunction(funCall); } @@ -133,6 +133,25 @@ public LoggingAdvisingAppendable appendLoggingFunctionInvocation( return this; } + private void maybeFlushPendingAttributes(LoggingFunctionInvocation funCall) throws IOException { + var loggingAttrs = this.loggingAttrs; + var consumer = funCall.resultConsumer(); + if (loggingAttrs != null) { + this.loggingAttrs = null; + boolean isAnchorTag = funCall.args().get(0).booleanValue(); + if (consumer.isPresent()) { + StringBuilder sb = new StringBuilder(); + loggingAttrs.writeTo(isAnchorTag, sb); + consumer.get().accept(sb.toString()); + } else { + loggingAttrs.writeTo(isAnchorTag, outputAppendable); + } + } else if (consumer.isPresent()) { + // We have to ensure the consumer is always filled. + consumer.get().accept(""); + } + } + @CanIgnoreReturnValue @Override public LoggingAdvisingAppendable enterLoggableElement(LogStatement statement) { @@ -151,7 +170,13 @@ public LoggingAdvisingAppendable enterLoggableElement(LogStatement statement) { } logOnlyDepth = depth; } - appendDebugOutput(logger.enter(statement)); + var enterData = logger.enter(statement); + appendDebugOutput(enterData.debugHtml()); + var loggingAttrs = enterData.loggingAttrs().orElse(null); + if (loggingAttrs != null && depth > 0) { + loggingAttrs = null; // we cannot render them when logonly is set, so drop them now. + } + setLoggingAttrs(loggingAttrs); return this; } @@ -166,11 +191,20 @@ public LoggingAdvisingAppendable exitLoggableElement() { depth--; logOnlyDepth = depth; } + setLoggingAttrs(null); // should debug output be guarded by logonly? appendDebugOutput(logger.exit()); return this; } + private void setLoggingAttrs(@Nullable SoyLogger.LoggingAttrs loggingAttrs) { + if (this.loggingAttrs != null) { + googleLogger.atWarning().withStackTrace(StackSize.SMALL).log( + "a logger configured logging attrs that were not rendered onto an element. Your {velog}" + + " command must not be wrapping an element, this is undefined behavior."); + } + this.loggingAttrs = loggingAttrs; + } private void appendDebugOutput(Optional veDebugOutput) { if (veDebugOutput.isPresent()) { diff --git a/java/src/com/google/template/soy/logging/BUILD.bazel b/java/src/com/google/template/soy/logging/BUILD.bazel index 01e55ff55..c9850d418 100644 --- a/java/src/com/google/template/soy/logging/BUILD.bazel +++ b/java/src/com/google/template/soy/logging/BUILD.bazel @@ -43,7 +43,9 @@ java_library( deps = [ "//java/src/com/google/template/soy/data", "//java/src/com/google/template/soy/plugin/restricted", + "@com_google_auto_value_auto_value", "@maven//:com_google_common_html_types_types", + "@maven//:com_google_guava_guava", ], ) diff --git a/java/src/com/google/template/soy/logging/SoyLogger.java b/java/src/com/google/template/soy/logging/SoyLogger.java index 119eefde6..fe36b4533 100644 --- a/java/src/com/google/template/soy/logging/SoyLogger.java +++ b/java/src/com/google/template/soy/logging/SoyLogger.java @@ -15,9 +15,18 @@ */ package com.google.template.soy.logging; +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkNotNull; + +import com.google.auto.value.AutoValue; +import com.google.common.collect.ImmutableMap; +import com.google.common.html.HtmlEscapers; import com.google.common.html.types.SafeHtml; +import com.google.common.html.types.SafeUrl; +import com.google.errorprone.annotations.CanIgnoreReturnValue; import com.google.template.soy.data.LogStatement; import com.google.template.soy.data.LoggingFunctionInvocation; +import java.io.IOException; import java.util.Optional; /** @@ -26,15 +35,177 @@ *

This implements a callback protocol with the {@code velog} syntax. */ public interface SoyLogger { + /** Logging attributes for {@code velog} root elements. */ + @AutoValue + public abstract class LoggingAttrs { + public static Builder builder() { + return new Builder(); + } + + LoggingAttrs() {} + + /** + * The attributes to be added to the element. + * + *

The attributes are added in the order they are added to the builder. The values are + * escaped and suitable for encoding in a double-quoted attribute value. + */ + abstract ImmutableMap attrs(); + + abstract boolean hasAnchorAttributes(); + + /** Writes the attributes to the output appendable. */ + public void writeTo(boolean isAnchorTag, Appendable outputAppendable) throws IOException { + if (hasAnchorAttributes() && !isAnchorTag) { + throw new IllegalStateException( + "logger attempted to add anchor attributes to a non-anchor element."); + } + for (var entry : attrs().entrySet()) { + var name = entry.getKey(); + outputAppendable + .append(' ') + .append(name) + .append("=\"") + .append(entry.getValue()) + .append('"'); + } + } + + @Override + public final String toString() { + StringBuilder sb = new StringBuilder(); + try { + writeTo(true, sb); + } catch (IOException e) { + throw new AssertionError(e); + } + return sb.substring(1); // skip the leading space + } + + /** Builder for {@link LoggingAttrs}. */ + public static final class Builder { + private boolean hasAnchorAttributes = false; + private final ImmutableMap.Builder attrsBuilder = ImmutableMap.builder(); + + private Builder() {} + + private void addAttribute(String key, String value) { + attrsBuilder.put(key, HtmlEscapers.htmlEscaper().escape(value)); + } + + /** + * Adds a data attribute to the logging attributes. + * + *

The key must start with "data-" + */ + @CanIgnoreReturnValue + public Builder addDataAttribute(String key, String value) { + checkArgument(key.startsWith("data-"), "data attribute key must start with 'data-'."); + checkNotNull(value); + addAttribute(key, value); + return this; + } + + /** + * Adds an anchor href attribute to the logging attributes. + * + *

If the element this is attached to is an anchor tag, this will be used as the href, if + * it isn't an anchor an error will be thrown. + */ + @CanIgnoreReturnValue + public Builder addAnchorHref(SafeUrl value) { + addAttribute("href", value.getSafeUrlString()); + this.hasAnchorAttributes = true; + return this; + } + + /** + * Adds an anchor ping attribute to the logging attributes. + * + *

If the element this is attached to is an anchor tag, this will be used as the ping, if + * it isn't an anchor an error will be thrown. + */ + @CanIgnoreReturnValue + public Builder addAnchorPing(SafeUrl value) { + addAttribute("ping", value.getSafeUrlString()); + this.hasAnchorAttributes = true; + return this; + } + + /** + * Adds an anchor ping attribute to the logging attributes. + * + *

If the element this is attached to is an anchor tag, this will be used as the ping, if + * it isn't an anchor an error will be thrown. + */ + @CanIgnoreReturnValue + public Builder addAnchorPing(Iterable value) { + StringBuilder pingBuilder = new StringBuilder(); + boolean first = true; + for (SafeUrl url : value) { + if (!first) { + pingBuilder.append(' '); + } + pingBuilder.append(url.getSafeUrlString()); + first = false; + } + addAttribute("ping", pingBuilder.toString()); + return this; + } + + /** + * Builds the {@link LoggingAttrs}. + * + * @throws IllegalStateException if no attributes were added. + */ + public LoggingAttrs build() { + var attrs = attrsBuilder.buildOrThrow(); + if (attrs.isEmpty()) { + throw new IllegalStateException("LoggingAttrs must have at least one attribute"); + } + return new AutoValue_SoyLogger_LoggingAttrs(attrs, hasAnchorAttributes); + } + } + } + + /** Data to be used to output VE logging info to be outputted to the DOM while in debug mode. */ + @AutoValue + public abstract static class EnterData { + public static final EnterData EMPTY = create(Optional.empty(), Optional.empty()); + + public static EnterData create(SafeHtml debugHtml) { + return create(Optional.of(debugHtml), Optional.empty()); + } + + public static EnterData create(LoggingAttrs loggingAttrs) { + return create(Optional.empty(), Optional.of(loggingAttrs)); + } + + public static EnterData create(SafeHtml debugHtml, LoggingAttrs loggingAttrs) { + return create(Optional.of(debugHtml), Optional.of(loggingAttrs)); + } + + private static EnterData create( + Optional debugHtml, Optional loggingAttrs) { + return new AutoValue_SoyLogger_EnterData(debugHtml, loggingAttrs); + } + + public abstract Optional debugHtml(); + + public abstract Optional loggingAttrs(); + + EnterData() {} + } + /** * Called when a {@code velog} statement is entered. * - * @return Optional VE logging info to be outputted to the DOM while in debug mode. Method - * implementation must check and only return VE logging info if in debug mode. Most - * implementations will likely return Optional.empty(). TODO(b/148167210): This is currently - * under implementation. + * @return Data to be used to output VE logging info to be outputted to the DOM while in debug + * mode. Method implementation must check and only return VE logging info if in debug mode. + * Most implementations will likely return Optional.empty(). TODO(b/148167210): This is + * currently under implementation. */ - Optional enter(LogStatement statement); + EnterData enter(LogStatement statement); /** * Called when a {@code velog} statement is exited. diff --git a/java/tests/com/google/template/soy/jbcsrc/VeLoggingTest.java b/java/tests/com/google/template/soy/jbcsrc/VeLoggingTest.java index 736f1f8ed..1a3715580 100644 --- a/java/tests/com/google/template/soy/jbcsrc/VeLoggingTest.java +++ b/java/tests/com/google/template/soy/jbcsrc/VeLoggingTest.java @@ -16,11 +16,13 @@ package com.google.template.soy.jbcsrc; import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; import static org.junit.Assert.fail; import com.google.common.base.Joiner; import com.google.common.collect.ImmutableMap; import com.google.common.html.types.SafeHtml; +import com.google.common.html.types.SafeUrls; import com.google.template.soy.SoyFileSetParser; import com.google.template.soy.SoyFileSetParser.ParseResult; import com.google.template.soy.data.LogStatement; @@ -32,11 +34,13 @@ import com.google.template.soy.jbcsrc.shared.StackFrame; import com.google.template.soy.logging.LoggingFunction; import com.google.template.soy.logging.SoyLogger; +import com.google.template.soy.logging.SoyLogger.LoggingAttrs; import com.google.template.soy.shared.restricted.Signature; import com.google.template.soy.shared.restricted.SoyFunctionSignature; import com.google.template.soy.testing.Foo; import com.google.template.soy.testing.SoyFileSetParserBuilder; import java.io.IOException; +import java.util.HashMap; import java.util.Map; import java.util.Optional; import org.junit.Test; @@ -49,16 +53,21 @@ public final class VeLoggingTest { private static class TestLogger implements SoyLogger { final StringBuilder builder = new StringBuilder(); + final HashMap attrsMap = new HashMap<>(); int depth; @Override - public Optional enter(LogStatement statement) { + public EnterData enter(LogStatement statement) { if (builder.length() > 0) { builder.append('\n'); } builder.append(" ".repeat(depth)).append(statement); depth++; - return Optional.empty(); + LoggingAttrs attrs = attrsMap.get(statement.id()); + if (attrs != null) { + return EnterData.create(attrs); + } + return EnterData.EMPTY; } @Override @@ -277,6 +286,46 @@ public void testLogging_elvis() throws Exception { assertThat(sb.toString()).isEqualTo("

hello
"); } + @Test + public void testLoggingAttributes_basic() throws Exception { + StringBuilder sb = new StringBuilder(); + TestLogger testLogger = new TestLogger(); + testLogger.attrsMap.put(1L, LoggingAttrs.builder().addDataAttribute("data-foo", "bar").build()); + renderTemplate( + OutputAppendable.create(sb, testLogger), "{velog FooVe}
hello
{/velog}"); + assertThat(testLogger.builder.toString()).isEqualTo("velog{id=1}"); + assertThat(sb.toString()).isEqualTo("
hello
"); + } + + @Test + public void testLoggingAttributes_anchor() throws Exception { + StringBuilder sb = new StringBuilder(); + TestLogger testLogger = new TestLogger(); + testLogger.attrsMap.put( + 1L, LoggingAttrs.builder().addAnchorHref(SafeUrls.fromConstant("./go")).build()); + renderTemplate(OutputAppendable.create(sb, testLogger), "{velog FooVe}hello{/velog}"); + assertThat(testLogger.builder.toString()).isEqualTo("velog{id=1}"); + assertThat(sb.toString()).isEqualTo("hello"); + } + + @Test + public void testLoggingAttributes_hrefToNonAnchor() throws Exception { + StringBuilder sb = new StringBuilder(); + TestLogger testLogger = new TestLogger(); + testLogger.attrsMap.put( + 1L, LoggingAttrs.builder().addAnchorHref(SafeUrls.fromConstant("./go")).build()); + var t = + assertThrows( + IllegalStateException.class, + () -> + renderTemplate( + OutputAppendable.create(sb, testLogger), + "{velog FooVe}
hello
{/velog}")); + assertThat(t) + .hasMessageThat() + .isEqualTo("logger attempted to add anchor attributes to a non-anchor element."); + } + private void renderTemplate(OutputAppendable output, String... templateBodyLines) throws IOException { renderTemplate(ImmutableMap.of(), output, templateBodyLines); @@ -296,6 +345,7 @@ private void renderTemplate( + "\n{/template}", Foo.getDescriptor()) .addSoySourceFunction(new DepthFunction()) + .addHtmlAttributesForLogging(true) .runAutoescaper(true); SoyFileSetParser parser = builder.build(); ParseResult parseResult = parser.parse(); diff --git a/java/tests/com/google/template/soy/jbcsrc/api/OutputAppendableTest.java b/java/tests/com/google/template/soy/jbcsrc/api/OutputAppendableTest.java index 68641fd0c..5cfc7e0b1 100644 --- a/java/tests/com/google/template/soy/jbcsrc/api/OutputAppendableTest.java +++ b/java/tests/com/google/template/soy/jbcsrc/api/OutputAppendableTest.java @@ -50,8 +50,8 @@ public String evalLoggingFunction(LoggingFunctionInvocation value) { } @Override - public Optional enter(LogStatement statement) { - return Optional.empty(); + public EnterData enter(LogStatement statement) { + return EnterData.EMPTY; } }; diff --git a/java/tests/com/google/template/soy/logging/BUILD.bazel b/java/tests/com/google/template/soy/logging/BUILD.bazel new file mode 100644 index 000000000..830e18e32 --- /dev/null +++ b/java/tests/com/google/template/soy/logging/BUILD.bazel @@ -0,0 +1,39 @@ +## +# Copyright 2024 Google Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +## + +load("@rules_java//java:defs.bzl", "java_library") +load("//builddefs:internal.bzl", "java_individual_tests") + +package( + default_testonly = 1, + default_visibility = ["//:soy_internal"], +) + +java_library( + name = "tests", + srcs = glob(["*.java"]), + deps = [ + "//java/src/com/google/template/soy/logging:public", + "@maven//:com_google_code_findbugs_jsr305", + "@maven//:com_google_common_html_types_types", + "@maven//:com_google_truth_truth", + "@maven//:junit_junit", + ], +) + +java_individual_tests( + deps = [":tests"], +) diff --git a/java/tests/com/google/template/soy/logging/SoyLoggerTest.java b/java/tests/com/google/template/soy/logging/SoyLoggerTest.java new file mode 100644 index 000000000..45eae1e94 --- /dev/null +++ b/java/tests/com/google/template/soy/logging/SoyLoggerTest.java @@ -0,0 +1,64 @@ +/* + * Copyright 2024 Google Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.template.soy.logging; + +import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.assertThrows; + +import com.google.common.html.types.SafeUrls; +import com.google.template.soy.logging.SoyLogger.LoggingAttrs; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +@RunWith(JUnit4.class) +public final class SoyLoggerTest { + + @Test + public void loggingAttr_basic() { + var loggingAttrs = + LoggingAttrs.builder() + .addDataAttribute("data-foo", "bar") + .addAnchorHref(SafeUrls.fromConstant("./go")) + .addAnchorPing(SafeUrls.fromConstant("./log")) + .build(); + assertThat(loggingAttrs.toString()).isEqualTo("data-foo=\"bar\" href=\"./go\" ping=\"./log\""); + } + + @Test + public void loggingAttr_testBadDataAttr() { + var e = + assertThrows( + IllegalArgumentException.class, + () -> LoggingAttrs.builder().addDataAttribute("foo", "bar")); + assertThat(e).hasMessageThat().isEqualTo("data attribute key must start with 'data-'."); + } + + @Test + public void loggingAttr_testBadAnchorHref() { + var e = + assertThrows( + IllegalStateException.class, + () -> + LoggingAttrs.builder() + .addAnchorHref(SafeUrls.fromConstant("./go")) + .build() + .writeTo(/* isAnchorTag= */ false, new StringBuilder())); + assertThat(e) + .hasMessageThat() + .isEqualTo("logger attempted to add anchor attributes to a non-anchor element."); + } +}