vy commented on code in PR #2691: URL: https://github.com/apache/logging-log4j2/pull/2691#discussion_r1657060422
########## log4j-core-test/src/test/java/org/apache/logging/log4j/core/util/StringBuildersTest.java: ########## @@ -0,0 +1,47 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you 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 org.apache.logging.log4j.core.util; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.util.stream.Stream; +import org.apache.logging.log4j.core.util.internal.StringBuilders; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +public class StringBuildersTest { + + static Stream<Arguments> testTruncateLines_dataSource() { Review Comment: No, those were the `buffer` content to test. ########## log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/RootThrowablePatternConverter.java: ########## @@ -68,27 +67,17 @@ public void format(final LogEvent event, final StringBuilder toAppendTo) { super.format(event, toAppendTo); return; } - final String trace = proxy.getCauseStackTraceAsString( - options.getIgnorePackages(), options.getTextRenderer(), getSuffix(event), options.getSeparator()); final int len = toAppendTo.length(); if (len > 0 && !Character.isWhitespace(toAppendTo.charAt(len - 1))) { toAppendTo.append(' '); } - if (!options.allLines() || !Strings.LINE_SEPARATOR.equals(options.getSeparator())) { - final StringBuilder sb = new StringBuilder(); - final String[] array = trace.split(Strings.LINE_SEPARATOR); - final int limit = options.minLines(array.length) - 1; - for (int i = 0; i <= limit; ++i) { - sb.append(array[i]); - if (i < limit) { - sb.append(options.getSeparator()); - } - } - toAppendTo.append(sb.toString()); - - } else { - toAppendTo.append(trace); - } + proxy.formatCauseStackTraceTo( + toAppendTo, + options.getIgnorePackages(), + options.getTextRenderer(), + getSuffix(event), + options.getSeparator(), + options.allLines() ? null : options.getLines()); Review Comment: @alan0428a, sounds good. ########## log4j-core/src/main/java/org/apache/logging/log4j/core/util/internal/StringBuilders.java: ########## @@ -0,0 +1,57 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you 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 org.apache.logging.log4j.core.util.internal; + +/** + * StringBuilder helpers + */ +public class StringBuilders { + + /** + * Truncates the content of the given {@code StringBuilder} to the specified maximum number of lines. + * + * <p>If {@code maxLineCount} is {@code null}, {@link Integer#MAX_VALUE}, or if {@code lineSeparator} is empty, + * the method returns without making any changes to the {@code StringBuilder}. + * + * @param buffer the {@code StringBuilder} whose content is to be truncated + * @param lineSeparator the line separator used to determine the end of a line + * @param maxLineCount the maximum number of lines to retain in the {@code StringBuilder}; + * if this value is {@code null} or {@link Integer#MAX_VALUE}, no truncation will occur + */ + public static void truncateLines( Review Comment: You're right massage the `if` condition as follows: ``` buffer.length() < delimiter.length() || delimiter.isEmpty() || maxOccurrenceCount == Integer.MAX_VALUE ``` ########## log4j-core-test/src/test/java/org/apache/logging/log4j/core/pattern/ThrowableFilterTest.java: ########## @@ -0,0 +1,56 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you 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 org.apache.logging.log4j.core.pattern; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.util.List; +import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.core.LoggerContext; +import org.apache.logging.log4j.core.test.appender.ListAppender; +import org.apache.logging.log4j.core.test.junit.LoggerContextSource; +import org.apache.logging.log4j.core.test.junit.Named; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Unit tests for {@code throwable} pattern. + */ +@LoggerContextSource("log4j-throwable-filter.xml") +public class ThrowableFilterTest { Review Comment: @alan0428a, please proceed with the `ConfigurationBuilder` approach. I will internally talk with @ppkarwasz and if changes are needed, we will carry them out ourselves later. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: notifications-unsubscr...@logging.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
