Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Backport some Log4j API 3.x features #2392

Merged
merged 12 commits into from
Mar 22, 2024
Merged

Backport some Log4j API 3.x features #2392

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
b2c0a85
Simplify `ServiceLoaderUtil`
ppkarwasz Mar 14, 2024
9b455b2
Adapt `ReusableMessage` to be used in 3.x
ppkarwasz Mar 14, 2024
ffe06dd
Remove deprecations from functional interfaces
ppkarwasz Mar 15, 2024
f3c0f6f
Add `Strings#trimToOptional`
ppkarwasz Mar 15, 2024
f7184e5
Make default message factories GraalVM friendly
ppkarwasz Mar 15, 2024
9157957
Port `Tags` from 3.x
ppkarwasz Mar 15, 2024
c2c3a01
Remove reflection from `log4j-to-jul` and `log4j-to-slf4j`
ppkarwasz Mar 19, 2024
a567c58
Add constants to mark global resource usage
ppkarwasz Mar 19, 2024
08a17c3
Apply suggestions of review #2392
ppkarwasz Mar 22, 2024
a1c2bd1
Add changelog for GraalVM-compatibility
ppkarwasz Mar 22, 2024
f0e82d1
Fix BND baseline error
ppkarwasz Mar 22, 2024
11209a3
Rollback breaking change in `ReusableMessage`
ppkarwasz Mar 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 29 additions & 4 deletions log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/Resources.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,36 @@

/**
* Constants to use the {@link ResourceLock} annotation.
*
*/
public class Resources {
public final class Resources {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This class is used in @ResourceLock JUnit 5 annotations.

Thanks to the DI, most tests in Log4j Core 3.x will not require to access service classes through static methods (yes, even for ThreadContext), but we want to mark those that need to be rewritten.


/**
* Marks tests that require access to {@link org.apache.logging.log4j.LogManager} methods or change its
* underlying {@link org.apache.logging.log4j.spi.LoggerContextFactory} implementation.
*/
public static final String LOG_MANAGER = "log4j.LogManager";

/**
* Marks tests that require access to {@link org.apache.logging.log4j.ThreadContext} methods or change its
* underlying {@link org.apache.logging.log4j.spi.ThreadContextMap} implementation.
*/
public static final String THREAD_CONTEXT = "log4j.ThreadContext";

/**
* Marks tests that require access to {@link org.apache.logging.log4j.MarkerManager} methods.
*/
public static final String MARKER_MANAGER = "log4j.MarkerManager";

/**
* Marks tests that requires access to {@link org.apache.logging.log4j.Level} static methods to create new levels.
*/
public static final String LEVEL = "log4j.Level";

public static final String THREAD_CONTEXT = "log4j2.ThreadContext";
/**
* Marks tests that require access to {@link org.apache.logging.log4j.status.StatusLogger} static methods or
* change its underlying implementation.
*/
public static final String STATUS_LOGGER = "log4j.StatusLogger";

public static final String MARKER_MANAGER = "log4j2.MarkerManager";
private Resources() {}
}
2 changes: 1 addition & 1 deletion log4j-api-test/src/main/java/org/apache/logging/log4j/test/junit/package-info.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
* limitations under the license.
*/
@Export
@Version("2.23.1")
@Version("2.24.0")
package org.apache.logging.log4j.test.junit;

import org.osgi.annotation.bundle.Export;
Expand Down
30 changes: 9 additions & 21 deletions ...t/src/test/java/org/apache/logging/log4j/internal/map/UnmodifiableArrayBackedMapTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,15 @@
import static org.junit.jupiter.api.Assertions.fail;

import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.Map;
import java.util.Set;
import org.apache.logging.log4j.util.ReadOnlyStringMap;
import org.apache.logging.log4j.util.TriConsumer;
import org.junit.jupiter.api.Test;

Expand Down Expand Up @@ -238,32 +240,18 @@ public void testEqualsWhenOneValueDiffers() {

@Test
public void testForEachBiConsumer_JavaUtil() {
UnmodifiableArrayBackedMap map = UnmodifiableArrayBackedMap.EMPTY_MAP.copyAndPutAll(getTestParameters());
Set<String> keys = new HashSet<>();
java.util.function.BiConsumer<String, String> java_util_action =
new java.util.function.BiConsumer<String, String>() {
@Override
public void accept(String key, String value) {
keys.add(key);
}
};
map.forEach(java_util_action);
final Map<String, String> map = UnmodifiableArrayBackedMap.EMPTY_MAP.copyAndPutAll(getTestParameters());
final Collection<String> keys = new HashSet<>();
map.forEach((key, value) -> keys.add(key));
assertEquals(map.keySet(), keys);
}

@Test
public void testForEachBiConsumer_Log4jUtil() {
UnmodifiableArrayBackedMap map = UnmodifiableArrayBackedMap.EMPTY_MAP.copyAndPutAll(getTestParameters());
Set<String> keys = new HashSet<>();
org.apache.logging.log4j.util.BiConsumer<String, String> log4j_util_action =
new org.apache.logging.log4j.util.BiConsumer<String, String>() {
@Override
public void accept(String key, String value) {
keys.add(key);
}
};
map.forEach(log4j_util_action);
assertEquals(map.keySet(), keys);
final ReadOnlyStringMap map = UnmodifiableArrayBackedMap.EMPTY_MAP.copyAndPutAll(getTestParameters());
final Collection<String> keys = new HashSet<>();
map.forEach((key, value) -> keys.add(key));
assertEquals(map.toMap().keySet(), keys);
}

@Test
Expand Down
25 changes: 21 additions & 4 deletions log4j-api-test/src/test/java/org/apache/logging/log4j/util/ServiceLoaderUtilTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,15 @@
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;

import java.lang.invoke.MethodHandles;
import java.util.ArrayList;
import java.util.List;
import java.util.ServiceConfigurationError;
import java.util.ServiceLoader;
import java.util.stream.Collectors;
import org.apache.logging.log4j.Level;
import org.apache.logging.log4j.Logger;
import org.apache.logging.log4j.status.StatusData;
import org.apache.logging.log4j.status.StatusLogger;
import org.apache.logging.log4j.test.BetterService;
import org.apache.logging.log4j.test.ListStatusListener;
import org.apache.logging.log4j.test.Service;
Expand All @@ -35,14 +37,26 @@

public class ServiceLoaderUtilTest {

private static final Logger LOGGER = StatusLogger.getLogger();

@Test
public void testServiceResolution() {
final List<Object> services = new ArrayList<>();
assertDoesNotThrow(() -> ServiceLoaderUtil.loadServices(BetterService.class, MethodHandles.lookup(), false)
ServiceLoaderUtil.safeStream(
BetterService.class,
ServiceLoader.load(BetterService.class, getClass().getClassLoader()),
LOGGER);
assertDoesNotThrow(() -> ServiceLoaderUtil.safeStream(
BetterService.class,
ServiceLoader.load(BetterService.class, getClass().getClassLoader()),
LOGGER)
.forEach(services::add));
assertThat(services).hasSize(1);
services.clear();
assertDoesNotThrow(() -> ServiceLoaderUtil.loadServices(PropertySource.class, MethodHandles.lookup(), false)
assertDoesNotThrow(() -> ServiceLoaderUtil.safeStream(
PropertySource.class,
ServiceLoader.load(PropertySource.class, getClass().getClassLoader()),
LOGGER)
.forEach(services::add));
assertThat(services).hasSize(3);
}
Expand All @@ -51,7 +65,10 @@ public void testServiceResolution() {
@UsingStatusListener
public void testBrokenServiceFile(final ListStatusListener listener) {
final List<Service> services = new ArrayList<>();
assertDoesNotThrow(() -> ServiceLoaderUtil.loadServices(Service.class, MethodHandles.lookup(), false)
assertDoesNotThrow(() -> ServiceLoaderUtil.safeStream(
Service.class,
ServiceLoader.load(Service.class, getClass().getClassLoader()),
LOGGER)
.forEach(services::add));
assertEquals(2, services.size());
// A warning for each broken service
Expand Down
4 changes: 4 additions & 0 deletions log4j-api/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@
<bnd-multi-release>true</bnd-multi-release>
<!-- Differs from OSGi bundle name -->
<bnd-module-name>org.apache.logging.log4j</bnd-module-name>
<bnd-extra-package-options>
<!-- Not exported by most OSGi system bundles, hence we use the system classloader to load `sun.reflect.Reflection` -->
!sun.reflect
</bnd-extra-package-options>
Comment on lines +39 to +42
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Has this problem ever been reported? I feel like we are solving a problem of your C64 that you tinker at nights in your attic.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A sun.reflect;resolution:=optional declaration has been in log4j-api since I joined the team.

We must have lost it, when we switched to BND, because it couldn't detect that we are using sun.reflect.Reflect. However after I replaced LoaderUtil.loadClass with Class.forName the entry reappeared and broke our OSGi tests.

<bnd-extra-module-options>
<!-- Used in StringBuilders through reflection -->
java.sql;static=true,
Expand Down
Loading
Loading