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.

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

Feature Request: MicroProfile-config integration: Allow injecting annotations with jakarta.inject.Qualifier #1679

Closed
scr-oath opened this issue Apr 15, 2023 · 13 comments

Comments

@scr-oath
Copy link

I would like to integrate with MicroProfile Config. I got pretty far along, but… in order to allow Guice to allow @ConfigProperty, I had to do some very nasty reflection to muck with the innards of the strategyFor so that ensureIsBindingAnnotation(annotationType); would pass and allow me to Key.get(someType, someConfigPropertyAnnotation).

Ultimately, I would like to bind the @ConfigProperty and, of course, if this snippet can be made to work, any solution is awesome:

package org.example;

import com.google.common.reflect.ClassPath;
import com.google.inject.AbstractModule;
import com.google.inject.ConfigurationException;
import com.google.inject.Key;
import com.google.inject.Provider;
import com.google.inject.name.Names;
import com.google.inject.spi.Message;
import io.smallrye.config.SmallRyeConfigBuilder;
import org.eclipse.microprofile.config.Config;
import org.eclipse.microprofile.config.inject.ConfigProperty;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.io.IOException;
import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.lang.reflect.Parameter;
import java.util.Collections;
import java.util.HashSet;
import java.util.Set;

public class MPConfigModule extends AbstractModule {

    private static final Logger LOGGER = LoggerFactory.getLogger(MPConfigModule.class);

    @Override
    protected void configure() {

        SmallRyeConfigBuilder configBuilder = new SmallRyeConfigBuilder()
            .addDefaultSources()
            .addDefaultInterceptors()
            .addDiscoveredSources()
            .addDiscoveredConverters()
            .addDiscoveredInterceptors();

        Provider<Config> configProvider = getProvider(Config.class);

        try {
            for (ClassPath.ClassInfo clsInfo : ClassPath.from(ClassLoader.getSystemClassLoader()).getAllClasses()) {
                try {
                    if (clsInfo.getSimpleName().equals("module-info")) {
                        continue;
                    }
                    Class<?> cls;
                    cls = clsInfo.load();
                    Set<Key<?>> seen = new HashSet<>();

                    for (Constructor<?> constructor : cls.getDeclaredConstructors()) {
                        if (null != constructor.getDeclaredAnnotation(com.google.inject.Inject.class)
                            || null != constructor.getDeclaredAnnotation(javax.inject.Inject.class)) {
                            LOGGER.debug("constructor {}", constructor);
                        }
                        for (Parameter parameter : constructor.getParameters()) {
                            ConfigProperty configProperty = parameter.getDeclaredAnnotation(ConfigProperty.class);
                            if (null == configProperty) {
                                continue;
                            }
                            Class<?> tpe = parameter.getType();
                            LOGGER.debug("method {} {} ", tpe, configProperty);
                            Key<?> key = Key.get(tpe, configProperty);
                            if (seen.add(key)) {
                                String name = configProperty.name();
                                String defaultValue = configProperty.defaultValue();
                                if (defaultValue != null) {
                                    configBuilder.withDefaultValue(name, defaultValue);
                                }
                                //noinspection unchecked
                                bind((Key<Object>) key).toProvider(() -> configProvider.get().getValue(name, tpe));
                            }
                        }
                    }
                    for (Field field : cls.getDeclaredFields()) {
                        ConfigProperty configProperty = field.getDeclaredAnnotation(ConfigProperty.class);
                        if (null == configProperty) {
                            continue;
                        }
                        if (null != field.getDeclaredAnnotation(com.google.inject.Inject.class)
                            || null != field.getDeclaredAnnotation(javax.inject.Inject.class)) {
                            Class<?> tpe = field.getType();
                            LOGGER.debug("field {} {} ", tpe, configProperty);
                            Key<?> key = Key.get(tpe, configProperty);
                            if (seen.add(key)) {
                                String name = configProperty.name();
                                String defaultValue = configProperty.defaultValue();
                                if (defaultValue != null) {
                                    configBuilder.withDefaultValue(name, defaultValue);
                                }
                                //noinspection unchecked
                                bind((Key<Object>) key).toProvider(() -> configProvider.get().getValue(name, tpe));
                            }
                        }
                    }
                    for (Method method : cls.getDeclaredMethods()) {
                        if (null != method.getDeclaredAnnotation(com.google.inject.Inject.class)
                            || null != method.getDeclaredAnnotation(javax.inject.Inject.class)) {
                            for (Parameter parameter : method.getParameters()) {
                                ConfigProperty configProperty = parameter.getDeclaredAnnotation(ConfigProperty.class);
                                if (null == configProperty) {
                                    continue;
                                }
                                Class<?> tpe = parameter.getType();
                                LOGGER.debug("method {} {} ", tpe, configProperty);
                                Key<?> key = Key.get(tpe, configProperty);
                                if (seen.add(key)) {
                                    String name = configProperty.name();
                                    String defaultValue = configProperty.defaultValue();
                                    if (defaultValue != null) {
                                        configBuilder.withDefaultValue(name, defaultValue);
                                    }
                                    //noinspection unchecked
                                    bind((Key<Object>) key).toProvider(() -> configProvider.get().getValue(name, tpe));
                                }
                            }
                        }
                    }
                } catch (NoClassDefFoundError e) {
                    LOGGER.debug("Error loading class {}", clsInfo, e);
                }
            }
            bind(Config.class).toInstance(configBuilder.build());
        } catch (IOException e) {
            throw new ConfigurationException(Collections.singleton(new Message(e.getLocalizedMessage())));
        }
    }
}
@scr-oath
Copy link
Author

FYI - nasty reflection was:

package com.google.inject.internal;

import com.google.inject.internal.Annotations.AnnotationChecker;
import org.eclipse.microprofile.config.inject.ConfigProperty;

import java.lang.annotation.Annotation;
import java.util.Collections;

public class ConfigPropertyOrAnnotationChecker extends AnnotationChecker {

    private final AnnotationChecker delegate;

    /**
     * Constructs a new checker that looks for annotations of the given types.
     *
     * @param delegate delegate checker
     */
    public ConfigPropertyOrAnnotationChecker(Object delegate) {
        super(Collections.singleton(ConfigProperty.class));
        this.delegate = (AnnotationChecker) delegate;
    }

    @Override
    boolean hasAnnotations(Class<? extends Annotation> annotated) {
        return annotated == ConfigProperty.class || delegate.hasAnnotations(annotated);
    }
}

and

        try {
            Field bindingAnnotationCheckerField = Annotations.class.getDeclaredField("bindingAnnotationChecker");
            bindingAnnotationCheckerField.setAccessible(true);
            Field modifiersField = Field.class.getDeclaredField("modifiers");
            modifiersField.setAccessible(true);
            modifiersField.setInt(
                bindingAnnotationCheckerField, bindingAnnotationCheckerField.getModifiers() & ~Modifier.FINAL);
            Object bindingAnnotationChecker = bindingAnnotationCheckerField.get(Annotations.class);
            bindingAnnotationCheckerField.set(
                Annotations.class, new ConfigPropertyOrAnnotationChecker(bindingAnnotationChecker));
            bindingAnnotationCheckerField.setAccessible(false);
        } catch (NoSuchFieldException | IllegalAccessException e) {
            throw new ConfigurationException(Collections.singleton(new Message(e.getLocalizedMessage())));
        }

@GedMarc
Copy link

GedMarc commented Apr 15, 2023

Field + Construct interceptors will be a much better option here, Qualifier should already be there - Strictly speaking this should actually be more than do-able

I'll join you on this one

@GedMarc
Copy link

GedMarc commented Apr 15, 2023

TypeListeners seem to be the best route

@scr-oath
Copy link
Author

scr-oath commented Apr 17, 2023

TypeListeners seem to be the best route

Sorry not really sure how they help... the point is to make things annotated with ConfigProperty injectable - do listeners allow not having a binding but influencing how to inject the value?

Or were you suggesting writing an extension that listened to type and then added a binding - the challenge there is that you can't create a key with any annotation, which doesn't contain javax.inject.Qualifier or com.google.inject.BindingAnnotation, which ConfigProperty does not

@GedMarc
Copy link

GedMarc commented Apr 17, 2023

Yep - I got pretty well through it but very busy time work wise for me right now -

Intercepting on the injection with a TypeListener and pushing the value into the configuration seems to be bringing some success for me, support for 3rd party annotations generally goes this route to cater for these scenarios,
Whats also pretty nice is the config library can also be bound as singletons or eager singletons as well which I'm playing with

Reflection probably not going to get away from, in one way or the other, but the binding mechanism :) I need it for something i'm working on during the 9-5 as well, but have a lot in front before I get there :)

I'm doing an approach very similar to the jsr250 implementation

https://github.com/mathieucarbou/guice/blob/master/extensions/injection/src/main/java/com/mycila/guice/ext/injection/MemberInjectorTypeListener.java

bindListener(Matchers.any(), new TypeListener() {
            @Override
            public <I> void hear(TypeLiteral<I> type, TypeEncounter<I> encounter) {
               
            }
        });

@scr-oath
Copy link
Author

scr-oath commented Apr 22, 2023

Your work is amazing and inspiring and makes me wonder - "how on earth did you figure out how to do that?" The documentation doesn't really go into much detail about extending or plugging in, and from what experimentation I've tried - I can't see how only having the TypeListener is able to do the injection and communicate to Guice internals that it's already taken care of... if I try with a simple listener and add a memberInjector and just set the value, Guice still complains. Is there some magic communication that happens when you ask the "encounter" for the Injector and then ask it for a provider? I see the key is rewritten to look for Named but how does Guice know that you've done that for it?

Ultimately, I would like to not have to prebind anything - and this mechanism looks really great if I can just get the Config object and get the value (which is easy) then inject it, it would be quite nice to use - but squelching Guice's lookup failures is the part I can't make work.

@scr-oath
Copy link
Author

Ohhhhhh… This doesn't work if you have @Inject AND @Resource (or some other non-injectible thing) - for some reason, I thought that you had figured out the magic to let listeners adjust the bindings, but, in fact, your clients would have to NOT declare @Inject - I'll have to think about what that means - it doesn't seem quite right… in CDI world, @ConfigProperty needs @Inject so it seems like you'd want that regardless of whether SE with Guice, or EE with CDI/Weld.

@GedMarc
Copy link

GedMarc commented Apr 23, 2023

OK that is strange I've already done this on a few of my Guiced-EE implementations and that wasn't the case - I will check it and get back to you, definitely supposed to override any @Inject settings hmm - To make sure though, this best works with PrivateModules with expose clauses - sorry about the delay the holidays in April were a few too many put quite a bit of the side-work on hold getting back into work xD

I'm going a bit overboard on the impl though, doing a main MP implementation, not just for config, let's see if I can keep them loosely coupled xD

@scr-oath
Copy link
Author

scr-oath commented Apr 24, 2023

@GedMarc - Here's a draft PR to show you what I haven't been able to make work - but in the context of your system.

mathieucarbou/guice#20

And reference showing that the desired behavior for ConfigProfile should include @Inject

https://javadoc.io/static/org.eclipse.microprofile.config/microprofile-config-api/3.0.3/org/eclipse/microprofile/config/inject/ConfigProperty.html

 @Inject
 @ConfigProperty(name = "my.long.property")
 private Long injectedLongValue;

@sameb
Copy link
Member

sameb commented Apr 25, 2023

Duplicate of #1383

@sameb sameb marked this as a duplicate of #1383 Apr 25, 2023
@sameb sameb closed this as not planned Won't fix, can't repro, duplicate, stale Apr 25, 2023
copybara-service bot pushed a commit that referenced this issue Apr 26, 2023
…rt for `toProvider(...)` to instances of `jakarta.inject.Provider` or classes/keys/typeLiterals of type `jakarta.inject.Provider`. See #1383 (comment) for details of the plans. The Guice 6.0 release will support `javax.inject` _and_ `jakarta.inject` (excluding `toProvider()` support for `jakarta.inject`), and the Guice 7.0+ release will remove the `javax.inject` references.

Further background on these issues:
 * #1630
 * #1679
 * #1490
 * #1383

PiperOrigin-RevId: 526763665
copybara-service bot pushed a commit that referenced this issue Apr 26, 2023
This does not add support for `toProvider(...)` to instances of `jakarta.inject.Provider` or classes/keys/typeLiterals of type `jakarta.inject.Provider`. See #1383 (comment) for details of the plans. The Guice 6.0 release will support `javax.inject` _and_ `jakarta.inject` (excluding `toProvider()` support for `jakarta.inject`), and the Guice 7.0+ release will remove the `javax.inject` references.

Further background on these issues:
 * #1630
 * #1679
 * #1490
 * #1383

PiperOrigin-RevId: 526763665
copybara-service bot pushed a commit that referenced this issue Apr 26, 2023
This does not add support for `toProvider(...)` to instances of `jakarta.inject.Provider` or classes/keys/typeLiterals of type `jakarta.inject.Provider`. See #1383 (comment) for details of the plans. The Guice 6.0 release will support `javax.inject` _and_ `jakarta.inject` (excluding `toProvider()` support for `jakarta.inject`), and the Guice 7.0+ release will remove the `javax.inject` references.

Further background on these issues:
 * #1630
 * #1679
 * #1490
 * #1383

PiperOrigin-RevId: 526763665
copybara-service bot pushed a commit that referenced this issue Apr 26, 2023
This does not add support for `toProvider(...)` to instances of `jakarta.inject.Provider` or classes/keys/typeLiterals of type `jakarta.inject.Provider`. See #1383 (comment) for details of the plans. The Guice 6.0 release will support `javax.inject` _and_ `jakarta.inject` (excluding `toProvider()` support for `jakarta.inject`), and the Guice 7.0+ release will remove the `javax.inject` references.

Further background on these issues:
 * #1630
 * #1679
 * #1490
 * #1383

PiperOrigin-RevId: 526763665
copybara-service bot pushed a commit that referenced this issue Apr 26, 2023
This does not add support for `toProvider(...)` to instances of `jakarta.inject.Provider` or classes/keys/typeLiterals of type `jakarta.inject.Provider`. See #1383 (comment) for details of the plans. The Guice 6.0 release will support `javax.inject` _and_ `jakarta.inject` (excluding `toProvider()` support for `jakarta.inject`), and the Guice 7.0+ release will remove the `javax.inject` references.

Further background on these issues:
 * #1630
 * #1679
 * #1490
 * #1383

PiperOrigin-RevId: 526763665
copybara-service bot pushed a commit that referenced this issue Apr 26, 2023
This does not add support for `toProvider(...)` to instances of `jakarta.inject.Provider` or classes/keys/typeLiterals of type `jakarta.inject.Provider`. See #1383 (comment) for details of the plans. The Guice 6.0 release will support `javax.inject` _and_ `jakarta.inject` (excluding `toProvider()` support for `jakarta.inject`), and the Guice 7.0+ release will remove the `javax.inject` references.

Further background on these issues:
 * #1630
 * #1679
 * #1490
 * #1383

PiperOrigin-RevId: 527349023
@GedMarc
Copy link

GedMarc commented Apr 27, 2024

@GedMarc
Copy link

GedMarc commented Apr 27, 2024

@scr-oath Not sure if you are still interested, I needed to switch to Vert.x as a much higher priority (y)

@lobaorn
Copy link

lobaorn commented Jun 1, 2024

Hey @scr-oath @GedMarc , letting you know since you may be targeting Guice 7.0.0, about the new Micronaut Guice project, that depending on your usecase could be a useful replacement: micronaut-projects/micronaut-guice#9

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants