Skip to content

IGNITE-17915 Support extensible injection mechanism handling.#10337

Open
splatch wants to merge 8 commits intoapache:masterfrom
splatch:IGNITE-17915
Open

IGNITE-17915 Support extensible injection mechanism handling.#10337
splatch wants to merge 8 commits intoapache:masterfrom
splatch:IGNITE-17915

Conversation

@splatch
Copy link
Copy Markdown

@splatch splatch commented Oct 19, 2022

First part of changes which opens some of ignite internal classes for re-use within IoC scenarios.

This is proposed way which utilize most of existing ioc code provided for Spring framework.
Some of internal classes had to be marked as public, so they might work outside of own package.

The Contribution Checklist

  • There is a single JIRA ticket related to the pull request.
  • The web-link to the pull request is attached to the JIRA ticket.
  • The JIRA ticket has the Patch Available state.
  • The pull request body describes changes that have been made.
    The description explains WHAT and WHY was made instead of HOW.
  • The pull request title is treated as the final commit message.
    The following pattern must be used: IGNITE-XXXX Change summary where XXXX - number of JIRA issue.
  • A reviewer has been mentioned through the JIRA comments
    (see the Maintainers list)
  • The pull request has been checked by the Teamcity Bot and
    the green visa attached to the JIRA ticket (see TC.Bot: Check PR)

splatch and others added 5 commits October 21, 2022 12:25
First part of changes which opens some of ignite internal classes for
re-use within IoC scenarios.

Signed-off-by: Łukasz Dywicki <luke@code-house.org>
@SammyVimes
Copy link
Copy Markdown
Contributor

Hi, @splatch! I took the liberty to update your PR (just so it could pass our TeamCity checks). Everything else looks good to me. Here's one suggestion though: how about we wrap GridSpringResourceContext (in GridStartContext) into the GridInjectResourceContext so we would be able to remove GridSpringResourceContext springCtx field and will be one step closer to removing it altogether?

@splatch
Copy link
Copy Markdown
Author

splatch commented Oct 21, 2022

Hello @SammyVimes, thank you for picking it up and following it. I was afraid of submitting this PR as I was not able to run all the tests, hence I was not able to confirm it prior doing official attempt.
I thought of bridging GridSpringResourceContext to updated API and I think its entirely doable with adapter I attempted to draw in ignite-spring module. We just need to map @SpringResource and @SpringApplicationContext annotations to the GridInjectResourceContext. One note - I am not certain about use @SpringApplicationContext and if we will be able to cover its all use cases, as it was carried a bit differently than regular beans. From compilation point of view it should work, because most of ioc stuff was private to ignite-core so far.

If you can hold for few days I will use this time to make a prototype of injection registry working under osgi to confirm whole concept works beyond Spring. I will be able to contribute other thing to ignite-osgi-ext in later step.

@SammyVimes
Copy link
Copy Markdown
Contributor

Hi @splatch! I added a spring bridge to the core module, so that we could remove GridSpringResourceContext from IgniteKernal entirely and in the same time maintain backward compatibility for spring users (they won't have to move to ioc + spring modules right away). I tried running GridSpringResourceInjectionSelfTest and other spring resource injection tests, looks like it works fine. WDYT?

@SammyVimes
Copy link
Copy Markdown
Contributor

As for next steps: I agree, we should implement spring and osgi injectors (in their respective modules)

@splatch
Copy link
Copy Markdown
Author

splatch commented Oct 27, 2022

@SammyVimes I managed to get registries for other components, however I could not test ignite-ext at all due to OSGi metadata troubles both for core and ext modules themselves. I made an attempt to adjust feature file and ignite-osgi-ext tests, but I failed to make any progress. It probably requires partial, if not complete, rewrite due to outdated dependencies and changes in tooling.

I will test changes within opennms build where we do "wrapping" of ignite-core making it work under Apache Karaf. Will let you know about results and try to book more time for ignite osgi adjustments later this year.

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.

2 participants