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

feat: implement dependency injection #87

Merged
merged 102 commits into from
Apr 3, 2021
Merged

feat: implement dependency injection #87

merged 102 commits into from
Apr 3, 2021

Conversation

pollend
Copy link
Member

@pollend pollend commented Feb 27, 2021

This is partially inspired by some of the patterns adopted with how the current Context Scheme works in Terasology with some inspiration from Dagger, Micronaut and guice.

The BeanContext is created with a Service Registry that defines all the dependencies linked to a given context. Resources request from the context will attempt to resolve dependencies from the current context and then tries to look at a parent context if the dependency is not resolved there.

BeanEnvironment is an internal cache that is used to track loaded in dependencies that are defined from the annotationProcessor. this is loaded in through a META-INF/services config that is generated by classes marked with @Inject/@Introspected/@Singleton/@Transient. the class definitions are fully defined so there is a minimum amount of scanning over the application classpath. This dramatically improves application boot times.

@Singleton/@Transient are not drive by anything without defining a scanner that can be used to search for dependencies that match a given annotation or pattern. A sample is defined in org.terasology.gestalt.di.scanners but more standard ones can be added for convince.

Dependencies are associated with 3 different scopes

  • Singleton - lives as long as the context is still active
  • Transient - new instance for every request or injection
  • Scoped - new instance for every layer

Usage

Class DepA {
   @Inject
   protected DepB;
}

Class DepB {
}


static void main() {
   ServiceRegistry registry = new ServiceRegistry();
   registry.with(DepA.class).lifetime(Lifetime.Singleton);
   registry.with(DepB.class).lifetime(Lifetime.Singleton);

  BeanContext cntx = new DefaultBeanContext(registry);
  DepA  a = cntx.get(DepA.class);
}

features:

  • keep a stack chain of dependencies and print out a tree with how a dependency failed to resolve
  • catch cyclic dependencies and throw an exception where depA --> DepB --> DepA
  • support Lazy type dependencies that can be resolved at a later point.
  • support Optional type dependencies
  • for service registry we should have suppliers with multiple inputs that can be resolved.
  • support Provider type dependencies that can be resolved at request.
  • replaces Reflections with own ClassIndexProcessor and ResourcesProcessor which works with gradle and idea in same way.
  • integrated gestalt-module you can use DI inside modules.

pollend and others added 30 commits September 5, 2020 23:51
Basic and naive BeanDefinitionGeneration.
fix NPE when proccessing annotations and make BeanDefinition `public` and `final`
Fix some Override error
add exceptions.
Create its works test
Fix parent bean resolving
Fix default target for inject is null. (after `ServiceRegistry#with` only)
Write tests: childBeanContext and BeanDependecies
@pollend pollend requested a review from keturn March 21, 2021 01:54
@pollend
Copy link
Member Author

pollend commented Mar 30, 2021

I update master with develop and tagged it v7.0.7 so I think we should be good on that end just need to get approval for this PR @DarkWeird. anything we need to finalize since I think this is in a decent state to develop a v8.0.0 of this library.

Copy link
Contributor

@BenjaminAmos BenjaminAmos left a comment

Choose a reason for hiding this comment

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

Some minor comments here but the code works.

@@ -41,24 +31,15 @@ android {
sourceCompatibility JavaVersion.VERSION_1_8
targetCompatibility JavaVersion.VERSION_1_8
}
packagingOptions {
merge 'META-INF/annotations/*'
Copy link
Contributor

Choose a reason for hiding this comment

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

Just leaving a note here to explain that this merges all the META-INF/annotations files from the jars that are dependencies of gestalt-android-testbed into each other. Android APK files aggregate all dependencies and the main program into a single archive. This might need to be done with META-INF/subtypes in the future but hasn't been needed so-far.


repositories {
maven {
url = 'https://heisluft.tk/maven/'
Copy link
Contributor

Choose a reason for hiding this comment

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

heisluft.tk isn't the URL for the JSemver dependency anymore. The domain doesn't resolve anymore and was moved to heisluft.de. It was changed elsewhere in bf25da8.

Suggested change
url = 'https://heisluft.tk/maven/'
url = 'https://heisluft.de/maven/'

Copy link
Member Author

@pollend pollend Mar 31, 2021

Choose a reason for hiding this comment

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

I talked with @Cervator and I think we are planning on managing it in the Terasology maven.

gestalt-inject-java/build.gradle Outdated Show resolved Hide resolved
@@ -0,0 +1,8 @@
package org.terasology.context;
Copy link
Contributor

Choose a reason for hiding this comment

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

I added this in but looking at it again, maybe it should be in a different package?

List<Module> orderedModules = getModulesOrderedByDependencies();
Predicate<Class<?>> classpathModuleClassesPredicate = orderedModules.stream().map(Module::getClassPredicate).reduce(x -> false, Predicate::or);
for (final Module module : orderedModules) {
if (!module.getClasspaths().isEmpty() && !hasClassContent(module)) {
// TODO return non-code module handling.
Copy link
Contributor

Choose a reason for hiding this comment

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

Destination Sol only has one code module that I know of (warp). All the rest are asset-only modules. Would they not work with the current code?

private static final String METAINF = "META-INF";
// HACK: Android always claims that the URL of the "META-INF" directory or any of its subdirectories doesn't exist.
// It will return the URL of any files within the "META-INF" directory and its subdirectories though.
private static final String METAINF_TEST_FILE = METAINF + "/gestalt-indexes-present";
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm referencing #94 here to explain why the folder checks are done in such an unusual way. I added a work-around especially for android, essentially. The tests still passed though, so I assume this didn't break anything.

Copy link
Member Author

Choose a reason for hiding this comment

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

umm, that would be a question for @DarkWeird.

annotationTypeWriter.finish();
subtypesTypeWriter.finish();
FileObject file = filer.createResource(StandardLocation.CLASS_OUTPUT, "", "META-INF/gestalt-indexes-present");
Writer writer = file.openWriter();
Copy link
Contributor

Choose a reason for hiding this comment

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

I added the gestalt-indexes-present file for android, as it doesn't seem to support querying META-INF directories in an APK. This could possibly be moved to its own TypeWriter class.

JavaCompiler compiler = ToolProvider.getSystemJavaCompiler();
DiagnosticCollector<JavaFileObject> diagnosticCollector = new DiagnosticCollector<>();
final StandardJavaFileManager standardFileManager = compiler.getStandardFileManager(diagnosticCollector, Locale.getDefault(), UTF_8);
return null;
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it the intent of this method to always return null? Is it unfinished?

@pollend pollend merged commit 0832d97 into develop Apr 3, 2021
@pollend pollend deleted the feature/DI branch April 3, 2021 19:33
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

Successfully merging this pull request may close these issues.

6 participants