From 20779d655007e4dc2610389e2fbaf13c8f32d63c Mon Sep 17 00:00:00 2001 From: Phil Clay Date: Sat, 4 Feb 2023 15:00:00 +0100 Subject: [PATCH] Fix not-JSON generators non-JSON generators were broken in 7.0 when a call to generator.setRootValueSeparator was introduced. The non-JSON generators throw UnsupportedOperationException when setRootValueSeparator is invoked. Handle the UnsupportedOperationException properly. Also moved disabling the FLUSH_PASSED_TO_STREAM generator feature to prior to decoration, because the YAML generation needs this feature enabled to work properly. Introduced basic tests for non-JSON generators to catch the above problems in the future. Fixes #919 --- .../AbstractCompositeJsonFormatter.java | 72 +++++++++++-------- .../yaml/YamlJsonFactoryDecorator.java | 7 ++ .../cbor/CborJsonFactoryDecoratorTest.java | 63 ++++++++++++++++ .../smile/SmileJsonFactoryDecoratorTest.java | 63 ++++++++++++++++ .../yaml/YamlJsonFactoryDecoratorTest.java | 65 +++++++++++++++++ 5 files changed, 241 insertions(+), 29 deletions(-) create mode 100644 src/test/java/net/logstash/logback/decorate/cbor/CborJsonFactoryDecoratorTest.java create mode 100644 src/test/java/net/logstash/logback/decorate/smile/SmileJsonFactoryDecoratorTest.java create mode 100644 src/test/java/net/logstash/logback/decorate/yaml/YamlJsonFactoryDecoratorTest.java diff --git a/src/main/java/net/logstash/logback/composite/AbstractCompositeJsonFormatter.java b/src/main/java/net/logstash/logback/composite/AbstractCompositeJsonFormatter.java index ea1357e2..f75142ef 100644 --- a/src/main/java/net/logstash/logback/composite/AbstractCompositeJsonFormatter.java +++ b/src/main/java/net/logstash/logback/composite/AbstractCompositeJsonFormatter.java @@ -94,8 +94,8 @@ public abstract class AbstractCompositeJsonFormatter threadLocalJsonFormatter; - - + + public AbstractCompositeJsonFormatter(ContextAware declaredOrigin) { super(declaredOrigin); } @@ -254,7 +254,25 @@ private JsonFactory createJsonFactory() { } private JsonFactory decorateFactory(JsonFactory factory) { - return this.jsonFactoryDecorator.decorate(factory) + JsonFactory factoryToDecorate = factory + /* + * When generators are flushed, don't flush the underlying outputStream. + * + * This allows some streaming optimizations when using an encoder. + * + * The encoder generally determines when the stream should be flushed + * by an 'immediateFlush' property. + * + * The 'immediateFlush' property of the encoder can be set to false + * when the appender performs the flushes at appropriate times + * (such as the end of a batch in the AbstractLogstashTcpSocketAppender). + * + * Set this prior to decorating, because some generators require + * FLUSH_PASSED_TO_STREAM to work properly (e.g. YAML) + */ + .disable(JsonGenerator.Feature.FLUSH_PASSED_TO_STREAM); + + return this.jsonFactoryDecorator.decorate(factoryToDecorate) /* * Jackson buffer recycling works by maintaining a pool of buffers per thread. This * feature works best when one JsonGenerator is created per thread, typically in J2EE @@ -284,32 +302,28 @@ private JsonGenerator createGenerator(OutputStream outputStream) throws IOExcept } private JsonGenerator decorateGenerator(JsonGenerator generator) { - return new SimpleObjectJsonGeneratorDelegate(jsonGeneratorDecorator.decorate(generator) - /* - * When generators are flushed, don't flush the underlying outputStream. - * - * This allows some streaming optimizations when using an encoder. - * - * The encoder generally determines when the stream should be flushed - * by an 'immediateFlush' property. - * - * The 'immediateFlush' property of the encoder can be set to false - * when the appender performs the flushes at appropriate times - * (such as the end of a batch in the AbstractLogstashTcpSocketAppender). - */ - .disable(JsonGenerator.Feature.FLUSH_PASSED_TO_STREAM) - - /* - * Don't let the json generator close the underlying outputStream and let the - * encoder managed it. - */ - .disable(JsonGenerator.Feature.AUTO_CLOSE_TARGET) - - /* - * JsonGenerator are reused to serialize multiple log events. - * Change the default root value separator to an empty string instead of a single space. - */ - .setRootValueSeparator(new SerializedString(CoreConstants.EMPTY_STRING))); + JsonGenerator decorated = jsonGeneratorDecorator.decorate(generator) + /* + * Don't let the json generator close the underlying outputStream and let the + * encoder managed it. + */ + .disable(JsonGenerator.Feature.AUTO_CLOSE_TARGET); + + try { + decorated = decorated + /* + * JsonGenerator are reused to serialize multiple log events. + * Change the default root value separator to an empty string instead of a single space. + */ + .setRootValueSeparator(new SerializedString(CoreConstants.EMPTY_STRING)); + } catch (UnsupportedOperationException e) { + /* + * Ignore. + * Some generators do not support setting the rootValueSeparator. + */ + } + + return new SimpleObjectJsonGeneratorDelegate(decorated); } public JsonFactory getJsonFactory() { diff --git a/src/main/java/net/logstash/logback/decorate/yaml/YamlJsonFactoryDecorator.java b/src/main/java/net/logstash/logback/decorate/yaml/YamlJsonFactoryDecorator.java index cf014ad6..50220ee5 100644 --- a/src/main/java/net/logstash/logback/decorate/yaml/YamlJsonFactoryDecorator.java +++ b/src/main/java/net/logstash/logback/decorate/yaml/YamlJsonFactoryDecorator.java @@ -18,6 +18,7 @@ import net.logstash.logback.decorate.JsonFactoryDecorator; import com.fasterxml.jackson.core.JsonFactory; +import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.dataformat.yaml.YAMLFactory; import com.fasterxml.jackson.dataformat.yaml.YAMLGenerator; @@ -35,6 +36,12 @@ public JsonFactory decorate(JsonFactory factory) { YAMLFactory yamlFactory = new YAMLFactory(); ObjectMapper mapper = new ObjectMapper(yamlFactory); yamlFactory.setCodec(mapper); + /* + * YAMLGenerator needs to pass the flush to the stream. + * It doesn't maintain an internal buffer like the other generators. + * To see this, look at the .flush() implementations of each of the generator classes. + */ + yamlFactory.enable(JsonGenerator.Feature.FLUSH_PASSED_TO_STREAM); return yamlFactory; } } diff --git a/src/test/java/net/logstash/logback/decorate/cbor/CborJsonFactoryDecoratorTest.java b/src/test/java/net/logstash/logback/decorate/cbor/CborJsonFactoryDecoratorTest.java new file mode 100644 index 00000000..d1f90b9d --- /dev/null +++ b/src/test/java/net/logstash/logback/decorate/cbor/CborJsonFactoryDecoratorTest.java @@ -0,0 +1,63 @@ +/* + * Copyright 2013-2022 the original author or authors. + * + * 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 net.logstash.logback.decorate.cbor; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.when; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; + +import net.logstash.logback.composite.loggingevent.LoggingEventCompositeJsonFormatter; +import net.logstash.logback.composite.loggingevent.LoggingEventJsonProviders; +import net.logstash.logback.composite.loggingevent.MessageJsonProvider; + +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.spi.ContextAware; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +class CborJsonFactoryDecoratorTest { + + @Mock + private ContextAware contextAware; + + @Mock + private ILoggingEvent event; + + @Test + void test() throws IOException { + CborJsonFactoryDecorator decorator = new CborJsonFactoryDecorator(); + + LoggingEventJsonProviders providers = new LoggingEventJsonProviders(); + providers.addMessage(new MessageJsonProvider()); + + LoggingEventCompositeJsonFormatter formatter = new LoggingEventCompositeJsonFormatter(contextAware); + formatter.setProviders(providers); + formatter.setJsonFactoryDecorator(decorator); + formatter.start(); + + when(event.getFormattedMessage()).thenReturn("a message"); + + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + formatter.writeEvent(event, baos); + + assertThat(baos.toByteArray()).asBase64Encoded().isEqualTo("v2dtZXNzYWdlaWEgbWVzc2FnZf8="); + } +} diff --git a/src/test/java/net/logstash/logback/decorate/smile/SmileJsonFactoryDecoratorTest.java b/src/test/java/net/logstash/logback/decorate/smile/SmileJsonFactoryDecoratorTest.java new file mode 100644 index 00000000..8858330c --- /dev/null +++ b/src/test/java/net/logstash/logback/decorate/smile/SmileJsonFactoryDecoratorTest.java @@ -0,0 +1,63 @@ +/* + * Copyright 2013-2022 the original author or authors. + * + * 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 net.logstash.logback.decorate.smile; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.when; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; + +import net.logstash.logback.composite.loggingevent.LoggingEventCompositeJsonFormatter; +import net.logstash.logback.composite.loggingevent.LoggingEventJsonProviders; +import net.logstash.logback.composite.loggingevent.MessageJsonProvider; + +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.spi.ContextAware; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +class SmileJsonFactoryDecoratorTest { + + @Mock + private ContextAware contextAware; + + @Mock + private ILoggingEvent event; + + @Test + void test() throws IOException { + SmileJsonFactoryDecorator decorator = new SmileJsonFactoryDecorator(); + + LoggingEventJsonProviders providers = new LoggingEventJsonProviders(); + providers.addMessage(new MessageJsonProvider()); + + LoggingEventCompositeJsonFormatter formatter = new LoggingEventCompositeJsonFormatter(contextAware); + formatter.setProviders(providers); + formatter.setJsonFactoryDecorator(decorator); + formatter.start(); + + when(event.getFormattedMessage()).thenReturn("a message"); + + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + formatter.writeEvent(event, baos); + + assertThat(baos.toByteArray()).asBase64Encoded().isEqualTo("OikKAfqGbWVzc2FnZUhhIG1lc3NhZ2X7"); + } +} diff --git a/src/test/java/net/logstash/logback/decorate/yaml/YamlJsonFactoryDecoratorTest.java b/src/test/java/net/logstash/logback/decorate/yaml/YamlJsonFactoryDecoratorTest.java new file mode 100644 index 00000000..26a49c32 --- /dev/null +++ b/src/test/java/net/logstash/logback/decorate/yaml/YamlJsonFactoryDecoratorTest.java @@ -0,0 +1,65 @@ +/* + * Copyright 2013-2022 the original author or authors. + * + * 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 net.logstash.logback.decorate.yaml; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.when; + +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.nio.charset.StandardCharsets; + +import net.logstash.logback.composite.loggingevent.LoggingEventCompositeJsonFormatter; +import net.logstash.logback.composite.loggingevent.LoggingEventJsonProviders; +import net.logstash.logback.composite.loggingevent.MessageJsonProvider; + +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.spi.ContextAware; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.Mock; +import org.mockito.junit.jupiter.MockitoExtension; + +@ExtendWith(MockitoExtension.class) +class YamlJsonFactoryDecoratorTest { + + @Mock + private ContextAware contextAware; + + @Mock + private ILoggingEvent event; + + @Test + void test() throws IOException { + YamlJsonFactoryDecorator decorator = new YamlJsonFactoryDecorator(); + + LoggingEventJsonProviders providers = new LoggingEventJsonProviders(); + providers.addMessage(new MessageJsonProvider()); + + LoggingEventCompositeJsonFormatter formatter = new LoggingEventCompositeJsonFormatter(contextAware); + formatter.setProviders(providers); + formatter.setJsonFactoryDecorator(decorator); + formatter.start(); + + when(event.getFormattedMessage()).thenReturn("a message"); + + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + formatter.writeEvent(event, baos); + + assertThat(new String(baos.toByteArray(), StandardCharsets.UTF_8)).isEqualTo("---\nmessage: \"a message\""); + } + +}