-
Notifications
You must be signed in to change notification settings - Fork 7
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
Issue #2274 - Rationalisation of IoC #2277
Open
homedirectory
wants to merge
77
commits into
Issue-#2258
Choose a base branch
from
Issue-#2274
base: Issue-#2258
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… use the injector instead
Instead of doing heavy lifting in the constructor, move all the work into Providers.
…d of constructor injection Now subclasses don't have to declare the mandatory constructor with the IFilter parameter.
…han the implementation
* PersistentEntitySaver depends directly on EntityFetcher, instead of QueryExecutionContext. * Session is passed to all methods as an argument, since it's variable and can't be injected. Alternatively, assisted inject can be used to combine both (injected dependencies and Session in a single constructor).
The injected String was simply an empty string. Instead of coming up with a Named annotation for it and binding it in a module, we can just use IUserProvider.
This avoids redundant allocation of EntityContainerEnhancer that was necessary to type-check (in enhancePropertyWithLinkToParent and enhanceProperty)
…traction Dragging a map of default Hibernate types and an injector has been cumbersome and only contributed to complexity. The next step is to rid Guice modules of the respective constructor parameters and provide an alternative way of overriding mappings (probably through Modules.override)
…r instead of constructor injection" This reverts commit c3ec1ee. A separate issue should be captured for this change due to its mass breaking effect on TG applications (affects all Dao classes).
…s constructors This is no longer necessary. The new approach is to bind HibernateTypeMappings.
…ule constructors This parameter is subsumed by IApplicationDomainProvider parameter.
Now we no longer need to instantiate mixins by hand and use Suppliers to provide dependencies.
…titySaver DeleteOperations are rarely used, if used at all, thus it makes sense to delay its instantiation. Furthermore, it can be reused afterwards, hence Lazy. Similarly for PersistentEntitySaver, although it is used more often.
…ructors IFilter has a default implementation. Applications can provide their own through a custom module that binds IFilter.
This covers the whole range of named annotations usages with Guice. For example, it can be bound in a module using the `Provides` annotation.
…d ISecurityTokenProvider Instead, specify a default implementation via the `ImplementedBy` annotation and let application modules override this binding.
…d ISerialisationClassProvider Provide a default implementation in the form of DefaultSerialisationClassProvider
It is also duplicated in all TG applications, from which it will be removed as well.
Reduces mutability, facilitates informal reasoning. With this change some tests in platform-pojo-bl started breaking due to unbound IAuthorisationModel. Previously no such error had occured because AuthorisationInterceptor was being injected with Injector but hadn't been used at all in those tests, effectively hiding the error. This indicates that this interceptor is bound too early, and should be relocated from EntityModule. A subsequent commit should address this.
Previously it was bound in EntityModule, which hadn't been the best choice, as there are many platform-level test with their own modules extend EntityModule but don't make any use of authorisation, with the exception of one -- AuthorisationProcessTest, which has been explicitly provided with the relevant binding in this commit. The new AuthorisationModule is now installed by BasicWebServerModule to ensure that applications have authorisation enabled, just as before.
1. `EntityFactory.setInjector` hadn't been used in the way its javadoc stated. The Injector was being injected only once, by Guice. Therefore, it can be made final. 2. Since EntityFactory had been bound with `asInstance`, let's annotate it with Singleton. 3. Remove obsolete javadoc
This complements the previous commit by replacing the fields declared by EntityModuleWithPropertyFactory by a more Guice-idiomatic approach: no state in modules.
…ction Less clutter for subclasses
This has been the case before, but a prior refactoring blunder changed this.
… for universal constants and dates Both IDates and IUniversalConstants have default implementations. Usages of the removed parameters were using those defaults.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolve #2274
Summary
specify them if they need customisation, which is described in the documentation of
HibernateTypeMappings
.CommonEntityDao
:CommonEntityDao(IFilter)
has been deprecated. It is no longer necessary forsubclasses to call the
super
constructor.dates
has been removed. Subclasses that dependend on it should requestIDates
to be injected explicitly.
CompanionModule
. Thelist of entities is taken from
IAplicationDomainProvider
.IDomainDrivenTestCaseConfiguration
,getInstance()
should be used instead.DefaultFilter
renamed toNoDataFilter
. Applications that define aNoDataFilter
of their ownare advised to use the platform level one.