Creatable / ScopeFactory Proposal

[Leland-Takamine]

This proposal supersedes #108 and addresses the API issues mentioned in the comments of that proposal.

This proposal introduces 2 new APIs:

• Creatable<D>: Replaces @Dependencies
• ScopeFactory: Alternative to new FooScopeImpl(dependencies)

Creatable<D>

The Creatable<D> API allows explicit declaration of dependencies expected from the parent Scope.

Existing Pattern

@Scope
interface FooScope {
    ...
    @motif.Dependencies
    interface Dependencies {
        String string();
    }
}

Limitations

• Can't define Dependencies interface outside of the Scope. This is problematic if you want to create a library that instantiates a Scope under the hood, but would like to hide that implementation detail from consumers.

Proposed API

@Scope
interface FooScope extends Creatable<FooDependencies> {
    ...
}

interface FooDependencies {
    String string();
}

Benefits

• Enables ScopeFactory API below by specifying Dependencies interface via generics.
• Allows Dependencies interface to be defined outside of the Scope class.

Trade-offs

• This is a breaking change since it will replace the @Dependencies API
• Slightly more verbose

ScopeFactory

The ScopeFactory API enables Scope instantiation without referencing generated code.

This does not affect the usage of child methods.

Existing Pattern

@Scope
interface FooScope {
    ...
}

FooScopeImpl.Dependencies dependencies = ...;
FooScope fooScope = new ScopeImpl(dependencies);

Limitations

The pattern above requires you to reference generated code (FooScopeImpl and FooScopeImpl.Dependencies). There are a few problems with this:

• IDE complains that these classes don't exist until you build the project
• Tight coupling between API and implementation
• Could cause slower annotation processing due to more rounds
• Susceptible to this bug: BasicAnnotationProcessor does not recursively validate superclasses / superinterfaces google/auto#660

New API

@Scope
interface FooScope extends Creatable<FooDependencies> {
    ...
}

FooDependencies dependencies = ...;
FooScope fooScope = ScopeFactory.create(FooScope.class, dependencies);

Benefits

You no longer need to reference generated code which unlocks the following benefits:

• IDE is able to index all user-written code before compiling
• API and implementation are decoupled
• Improved annotation processing speed in certain cases
• Avoids this bug: BasicAnnotationProcessor does not recursively validate superclasses / superinterfaces google/auto#660

Trade-offs

• Requires a small amount of reflection. However, we can ensure complete compile time safety with validation at annotation processing time.

Advantages over #108

This proposal couples a Scope to a single dependencies interface. This allows us to keep the exact same semantics as the current @Dependencies API which allows for an automatable migration path. The API proposed in #108 had slightly different semantics and would have been more difficult to migrate to.

Implementation

1. Validate that dependencies and Scope result in a valid graph.
2. Generate FooScopeFactoryHelper class which allows us to instantiate FooScope given FooDependencies.
3. The framework ScopeFactory.create method calls the generated FooScopeFactoryHelper.create method reflectively.

Generated Code

Generated constructor takes in user-defined FooDependencies interface.

public class FooScopeImpl {
  
  public FooScopeImpl(FooDependencies dependencies) { ... }

  // Inner FooScopeImpl.Dependencies class is not generated
}

New Framework Classes

// Simplified version of the actual implementation
public class ScopeFactory {

    private ScopeFactory() {}

    private static final Map<Class<?>, Constructor<?>> scopeClassToCreateMethod = new HashMap<>();

    public static <S extends Creatable<D>, D> S create(Class<S> scopeClass, D dependencies) {
        return (S) getConstructor(scopeClass).newInstance(dependencies);
    }

    private synchronized static Constructor<?> getConstructor(Class<?> scopeClass) {
        Constructor<?> constructor = scopeClassToCreateMethod.get(scopeClass);
        if (constructor == null) {
            Class<?> scopeImplClass = getScopeImplClass(scopeClass);
            constructor = scopeImplClass.getDeclaredConstructors()[0];
            scopeClassToCreateMethod.put(scopeClass, constructor);
        }
        return constructor;
    }

    private static Class<?> getScopeImplClass(Class<?> scopeClass) {
        String scopeImplClassName = getScopeImplClassName(scopeClass);
        return Class.forName(scopeImplClassName, true, scopeClass.getClassLoader());
    }
}

interface Creatable<D> {}

[kurtisnelson]

👍 from me.

[mcassiano]

Yay for not having to reference generated code! 👍

[andreasnomikos]

The only concern with this proposal is that it deals with only abstracting the input but not the output of the Scope

in theory we would like to implement the following libraries
/api
FooDependencies
FooApi

/impl
FooScope extends FooApi, Creatable<FooDependencies, FooApi>

/usage
ScopeFactory.create(FooApi.class, dependencies); -> instantiates a FooScope under the hood

this can handle the ServiceLocator pattern for motif scopes in a nice concise api
if we extend the capabilities of the annotation processor to understand the FooApi -> FooScopeImpl bindings and generate the correct bridge to instantiate the appropriate instance.

However this can also be achieved with another plugin system on top of motif that just operates as a ServiceLocator Pattern but it still needs to understand how to pass the dependencies to the motif constructor however since we are already creating a reflection api to do that seems a prime opportunity to consider merging this into the main framework.

[Leland-Takamine]

The only concern with this proposal is that it deals with only abstracting the input but not the output of the Scope

in theory we would like to implement the following libraries
/api
FooDependencies
FooApi

/impl
FooScope extends FooApi, Creatable<FooDependencies, FooApi>

/usage
ScopeFactory.create(FooApi.class, dependencies); -> instantiates a FooScope under the hood

this can handle the ServiceLocator pattern for motif scopes in a nice concise api
if we extend the capabilities of the annotation processor to understand the FooApi -> FooScopeImpl bindings and generate the correct bridge to instantiate the appropriate instance.

However this can also be achieved with another plugin system on top of motif that just operates as a ServiceLocator Pattern but it still needs to understand how to pass the dependencies to the motif constructor however since we are already creating a reflection api to do that seems a prime opportunity to consider merging this into the main framework.

There's a lot to consider when thinking about surfacing an API to instantiate an implementation of an interface without explicit knowledge of the implementation. As you mentioned, what you're describing is something analogous to a Java ServiceLoader which I'm hesitant to bake into the base framework. It seems like this is something that can be addressed as a follow up.

[andreasnomikos]

Understood the only limiting/conflicting factor I see with this proposal is that reusing the Creatable concept to specify that api might make sense however there is nothing precluding the usage of a different api to specify that.

e.g. the following does make sense too

@Service(FooApi.class)
interface FooScope extends Creatable<FooDependencies>, FooApi  

[andreasnomikos]

On another note since we are using reflection already we can avoid the creation of the helper class in favor of directly using the primary constructor in the FooScopeImpl
in this case it might make sense to completely avoid the creation of the FooScopeImpl.Dependencies
and convert the generated code to directly access dependencies through FooDependencies

[TonyTangAndroid]

Excited. I have a code lab to reproduce google/auto#660, let me know if you want to validate this case.

[Leland-Takamine]

Understood the only limiting/conflicting factor I see with this proposal is that reusing the Creatable concept to specify that api might make sense however there is nothing precluding the usage of a different api to specify that.

e.g. the following does make sense too

@Service(FooApi.class)
interface FooScope extends Creatable<FooDependencies>, FooApi  

The service locator functionality is something that would be an opt-in, which means that the addition of that API should be a non-breaking change (separate API).

it might make sense to completely avoid the creation of the FooScopeImpl.Dependencies
and convert the generated code to directly access dependencies through FooDependencies

I've thought about this as well. The benefit is that for those "root" scopes that use the Creatable API, we'd be saving some method count. The downside is that we have slightly separate implementations depending on whether you use the Creatable API or not. I think having completely uniform ScopeImpl implementations across the board is valuable for things like static analysis, reflection-based features, and maintainability of the Motif codebase. Since there will only be a small percentage of scopes using this API, I don't think it's worth sacrificing uniformity for the improvements to method count.

[Leland-Takamine]

Spoke offline with @andreasnomikos and we agreed on moving forward with the current approach since we believe there are non-breaking paths forward to addressing his concerns/use-cases.

[Leland-Takamine]

Understood the only limiting/conflicting factor I see with this proposal is that reusing the Creatable concept to specify that api might make sense however there is nothing precluding the usage of a different api to specify that.
e.g. the following does make sense too

@Service(FooApi.class)
interface FooScope extends Creatable<FooDependencies>, FooApi  

The service locator functionality is something that would be an opt-in, which means that the addition of that API should be a non-breaking change (separate API).

it might make sense to completely avoid the creation of the FooScopeImpl.Dependencies
and convert the generated code to directly access dependencies through FooDependencies

I've thought about this as well. The benefit is that for those "root" scopes that use the Creatable API, we'd be saving some method count. The downside is that we have slightly separate implementations depending on whether you use the Creatable API or not. I think having completely uniform ScopeImpl implementations across the board is valuable for things like static analysis, reflection-based features, and maintainability of the Motif codebase. Since there will only be a small percentage of scopes using this API, I don't think it's worth sacrificing uniformity for the improvements to method count.

An update on this - The final implementation is in fact going with @andreasnomikos's suggestion: The scope implementation does not generate the inner Dependencies class if the Scope extends Creatable. The generated constructor takes in the user-defined dependencies class that is the type argument to the Creatable generic superclass.