You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@logging.apache.org by GitBox <gi...@apache.org> on 2022/02/08 03:54:24 UTC
[GitHub] [logging-log4j2] jvz opened a new pull request #744: Add inject API similar to Feather/Guice
jvz opened a new pull request #744:
URL: https://github.com/apache/logging-log4j2/pull/744
This replaces the previous attempt at unifying dependency injection and
configuration from a CDI-like API to an API more like javax.inject used
by libraries like Feather and Guice.
https://cwiki.apache.org/confluence/display/LOGGING/Dependency+Injection+and+Configuration
### Draft Status
There is still some work to be done in swapping out the old plugin system in `AbstractConfiguration` along with some updates to `LoggerContext` and potentially `Log4jContextFactory` to have `Injector` instances. This will result in removal of at least 14 more classes in log4j-plugins in `bind` and `inject` along with everything in `org.apache.logging.log4j.core.config.plugins.visitors`, `PluginConfigurationInjector`, and at least 100+ lines in `AbstractConfiguration`. Further code simplifications follow from there. I'm opening this draft PR before everything is ready for merging, so here's the remaining tasks:
- [ ] Hooked in to `AbstractConfiguration`
- [ ] Removal of old classes
- [ ] API docs
- [ ] Developer docs
- [ ] Port over debug logging from previous classes (injection strategies)
- [ ] Default `Module` with low priority to bind default implementations of customizable classes
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] jvz commented on pull request #744: [LOG4J2-2803] Create standardized dependency injection API
Posted by GitBox <gi...@apache.org>.
jvz commented on pull request #744:
URL: https://github.com/apache/logging-log4j2/pull/744#issuecomment-1061403755
I'd like to merge this sometime within the next couple weeks unless there are some issues raised.
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] jvz commented on a change in pull request #744: Add inject API similar to Feather/Guice
Posted by GitBox <gi...@apache.org>.
jvz commented on a change in pull request #744:
URL: https://github.com/apache/logging-log4j2/pull/744#discussion_r801247382
##########
File path: log4j-core/src/main/java/org/apache/logging/log4j/core/config/plugins/convert/CoreTypeConverters.java
##########
@@ -73,17 +73,6 @@ public BigInteger convert(final String s) {
}
}
- /**
- * Converts a {@link String} into a {@link Boolean}.
- */
- @Plugin(name = "Boolean", category = TypeConverters.CATEGORY)
- public static class BooleanConverter implements TypeConverter<Boolean> {
Review comment:
These are all moved into lambdas in `TypeConverterRegistry` for primitive types.
##########
File path: log4j-core/src/main/java/org/apache/logging/log4j/core/util/Builder.java
##########
@@ -32,7 +34,7 @@
* @param <T> This builder creates instances of this class.
* @deprecated Present only for compatibility with Log4j 2 2.x plugins. Use Builder from log4j-plugins.
*/
-public interface Builder<T> {
+public interface Builder<T> extends Supplier<T> {
Review comment:
Now both `Builder` interfaces work through a common super interface. No more wrappers!
##########
File path: log4j-plugins/src/main/java/org/apache/logging/log4j/plugins/bind/ConfigurationBinder.java
##########
@@ -21,6 +21,7 @@
* Strategy to bind and validate an {@linkplain org.apache.logging.log4j.plugins.inject.ConfigurationInjector injected
* configuration value} to a {@linkplain org.apache.logging.log4j.plugins.PluginFactory plugin factory}.
*/
+@Deprecated
Review comment:
Class will be deleted.
##########
File path: log4j-plugins/src/main/java/org/apache/logging/log4j/plugins/inject/PluginAttributeParser.java
##########
@@ -0,0 +1,79 @@
+/*
+ * 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.plugins.inject;
+
+import org.apache.logging.log4j.plugins.Inject;
+import org.apache.logging.log4j.plugins.Node;
+import org.apache.logging.log4j.plugins.PluginAttribute;
+import org.apache.logging.log4j.plugins.convert.TypeConverters;
+import org.apache.logging.log4j.plugins.name.AnnotatedElementAliasesProvider;
+import org.apache.logging.log4j.plugins.name.AnnotatedElementNameProvider;
+
+import java.lang.reflect.AnnotatedElement;
+import java.lang.reflect.Parameter;
+import java.lang.reflect.Type;
+import java.util.Collection;
+import java.util.Map;
+import java.util.function.Function;
+
+public class PluginAttributeParser implements NodeParser<PluginAttribute> {
+ private static final Map<Type, Function<PluginAttribute, ?>> DEFAULT_VALUE_EXTRACTORS = Map.ofEntries(
+ Map.entry(int.class, PluginAttribute::defaultInt),
+ Map.entry(Integer.class, PluginAttribute::defaultInt),
+ Map.entry(long.class, PluginAttribute::defaultLong),
+ Map.entry(Long.class, PluginAttribute::defaultLong),
+ Map.entry(boolean.class, PluginAttribute::defaultBoolean),
+ Map.entry(Boolean.class, PluginAttribute::defaultBoolean),
+ Map.entry(float.class, PluginAttribute::defaultFloat),
+ Map.entry(Float.class, PluginAttribute::defaultFloat),
+ Map.entry(double.class, PluginAttribute::defaultDouble),
+ Map.entry(Double.class, PluginAttribute::defaultDouble),
+ Map.entry(byte.class, PluginAttribute::defaultByte),
+ Map.entry(Byte.class, PluginAttribute::defaultByte),
+ Map.entry(char.class, PluginAttribute::defaultChar),
+ Map.entry(Character.class, PluginAttribute::defaultChar),
+ Map.entry(short.class, PluginAttribute::defaultShort),
+ Map.entry(Short.class, PluginAttribute::defaultShort),
+ Map.entry(Class.class, PluginAttribute::defaultClass)
+ );
+
+ private final Function<String, String> stringSubstitutionStrategy;
+
+ @Inject
Review comment:
Note how the parsing strategies for nodes can themselves be dependency-injected. In this case, we're using a `StrSubstitutor` function.
##########
File path: log4j-plugins/src/main/java/org/apache/logging/log4j/plugins/inject/ConfigurationInjector.java
##########
@@ -35,6 +35,7 @@
* @param <Ann> plugin annotation this injector uses
* @param <Cfg> configuration class
*/
+@Deprecated
Review comment:
To be deleted.
##########
File path: log4j-plugins/src/main/java/org/apache/logging/log4j/plugins/processor/PluginEntry.java
##########
@@ -17,14 +17,10 @@
package org.apache.logging.log4j.plugins.processor;
-import java.io.Serializable;
-
/**
* Memento object for storing a plugin entry to a cache file.
*/
-public class PluginEntry implements Serializable {
Review comment:
https://issues.apache.org/jira/browse/LOG4J2-3228 for the full story
##########
File path: log4j-plugins/src/main/java/org/apache/logging/log4j/plugins/inject/InjectorStrategy.java
##########
@@ -27,7 +27,7 @@
@Documented
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.ANNOTATION_TYPE)
-// TODO: annotation processor to validate type matches (help avoid runtime errors)
+@Deprecated
Review comment:
To be deleted.
##########
File path: log4j-plugin-processor/src/main/java9/module-info.java
##########
@@ -23,6 +23,5 @@
requires transitive org.osgi.framework;
provides javax.annotation.processing.Processor with
- org.apache.logging.log4j.plugin.processor.PluginProcessor,
- org.apache.logging.log4j.plugin.processor.BeanProcessor;
Review comment:
No longer necessary to do additional annotation processing as plugin annotations are all that matters for pre-indexing.
##########
File path: log4j-plugins-test/src/main/java/org/apache/logging/log4j/plugins/test/validation/ValidatingPluginWithTypedBuilder.java
##########
@@ -39,12 +39,6 @@ public String getName() {
return name;
}
- @PluginFactory
Review comment:
Otherwise ignored factory method, false advertising.
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] jvz commented on a change in pull request #744: [LOG4J2-2803] Create standardized dependency injection API
Posted by GitBox <gi...@apache.org>.
jvz commented on a change in pull request #744:
URL: https://github.com/apache/logging-log4j2/pull/744#discussion_r820151542
##########
File path: log4j-plugins/src/main/java/org/apache/logging/log4j/plugins/di/ReflectiveCallerContext.java
##########
@@ -0,0 +1,102 @@
+/*
+ * 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.plugins.di;
+
+import java.lang.reflect.AccessibleObject;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Field;
+import java.lang.reflect.InaccessibleObjectException;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+
+/**
+ * Represents the calling context {@link Injector} uses for reflection operations. This allows for customizing the caller class
+ * for invocations of {@link AccessibleObject#setAccessible(boolean)}.
+ */
+@FunctionalInterface
+public interface ReflectiveCallerContext {
Review comment:
- [x] Consider using `MethodHandles.Lookup` instead
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] jvz merged pull request #744: [LOG4J2-2803] Create standardized dependency injection API
Posted by GitBox <gi...@apache.org>.
jvz merged pull request #744:
URL: https://github.com/apache/logging-log4j2/pull/744
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] gubasza removed a comment on pull request #744: [LOG4J2-2803] Create standardized dependency injection API
Posted by GitBox <gi...@apache.org>.
gubasza removed a comment on pull request #744:
URL: https://github.com/apache/logging-log4j2/pull/744#issuecomment-1032933311
>
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] gubasza commented on pull request #744: [LOG4J2-2803] Create standardized dependency injection API
Posted by GitBox <gi...@apache.org>.
gubasza commented on pull request #744:
URL: https://github.com/apache/logging-log4j2/pull/744#issuecomment-1032933311
>
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] jvz commented on a change in pull request #744: [LOG4J2-2803] Create standardized dependency injection API
Posted by GitBox <gi...@apache.org>.
jvz commented on a change in pull request #744:
URL: https://github.com/apache/logging-log4j2/pull/744#discussion_r820151542
##########
File path: log4j-plugins/src/main/java/org/apache/logging/log4j/plugins/di/ReflectiveCallerContext.java
##########
@@ -0,0 +1,102 @@
+/*
+ * 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.plugins.di;
+
+import java.lang.reflect.AccessibleObject;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Field;
+import java.lang.reflect.InaccessibleObjectException;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+
+/**
+ * Represents the calling context {@link Injector} uses for reflection operations. This allows for customizing the caller class
+ * for invocations of {@link AccessibleObject#setAccessible(boolean)}.
+ */
+@FunctionalInterface
+public interface ReflectiveCallerContext {
Review comment:
- [ ] Consider using `MethodHandles.Lookup` instead
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [logging-log4j2] jvz commented on pull request #744: [LOG4J2-2803] Create standardized dependency injection API
Posted by GitBox <gi...@apache.org>.
jvz commented on pull request #744:
URL: https://github.com/apache/logging-log4j2/pull/744#issuecomment-1059662150
@rgoers you can have a look now
--
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-unsubscribe@logging.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org