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

QuotedIDFactory refactoring introducing type annotation and lookup by type #644

Merged
merged 3 commits into from Jun 5, 2023

Conversation

fracorco
Copy link
Contributor

This PR introduces a mechanism to link a QuotedIDFactory to a factory type string via QuotedIDFactory.@IDFactoryType annotations, and to lookup a QuotedIDFactory instance via a QuotedIDFactory.Supplier interface obtainable via injection.

QuotedIDFactory.Supplier injection leverages the a refactoring of OntopAbstractModule that now allows the binding (overridable in settings file) of an implementation class to an arbitrary Guice <class, qualifier> key (instead of an abstract class key only), where the qualifier denotes here the factory type and allows having multiple coexisting QuotedIDFactory bindings for different type qualifiers. Specifically, QuotedIDFactory implementation classes are bound in OntopSQLCoreModule while an implementation of QuotedIDFactory.Supplier delegating to Guice Injector is bound in OntopModelModule.

Note that the extension of OntopAbstractModule is general and can be used for similar use cases. Two unit tests are provided in OntopAbstractModuleTest and QuotedIDFactoryInjectionTest, respectively for the handling of generic <class, qualifier> keys and its use for QuotedIDFactory.Supplier.

A QuotedIDFactory.Supplier is now injected into JsonMetadata when deserializing from JSON (via Jackson injection from JsonSerializedMetadataProvider). This allows obtaining a QuotedIDFactory instance for the factory type occurring in the JSON by ultimately delegating to Guice (where knowledge about object instantiation resides), instead of hard-coding a <type, implementation class> BiMap and directly using Class.newInstance().

Other changes in the PR are related to minor cleanup of other involved files (e.g., simplified binding expression in OntopReformulationPostModule, nullability handling in QuotedIDImpl) and in the use of a @NonNullByDefault annotation (taken from io.github.solf:nullanno just not to define it in Ontop code) to simplify and improve consistency - and thus improve IDE nullability analysis - of null-related annotations: when used on a class/package (as in classes modified in the PR), everything has to be non-null except where otherwise specified with @Nullable.

…ia delegation to injection mechanism, used in JsonMetadata deserialization
kontchakov and others added 2 commits May 20, 2023 16:33
These tests never worked and are currently disabled on version5. This `diasabled` tag must have been lost somehow.
@DRovara
Copy link
Collaborator

DRovara commented Jun 5, 2023

The test cases that are failing for athena never actually worked, as they are currently not supported. On version5, they are disabled.
Added the disabled tag to them here as well.

@bcogrel bcogrel merged commit 232f45b into version5 Jun 5, 2023
16 checks passed
@bcogrel bcogrel deleted the feature/quoted-id-factory-refactoring branch June 5, 2023 12:48
@bcogrel
Copy link
Member

bcogrel commented Jun 5, 2023

Thanks @fracorco and all for this nice feature!

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

Successfully merging this pull request may close these issues.

None yet

4 participants