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.

Sign up for GitHub

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

Jump to bottom

Allow statistics service to be loaded from plugin directory #639

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Allow statistics service to be loaded from plugin directory #639

wants to merge 1 commit into from

Conversation

WimMaerevoet-Octopus
Copy link

@WimMaerevoet-Octopus WimMaerevoet-Octopus commented Nov 13, 2023 •
edited
Loading

Pull Request Description

Allow a clean separation between oxalis and statistics implementation using the already available plugin loading mechanism.

Type of Pull Request

  • New feature/Enhancement - non-breaking change which adds functionality
  • Bug fix
  • Breaking change (Require Major version change?)

Type of Change

  • OpenPeppol AS2/AS4 specification
  • OpenPeppol Spring/Fall release
  • Oxalis software change or enhancement
  • CEF change

Pull Request Checklist:

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas. But did not added unnecessary annotation/comment say @author name etc
  • I have checked my code for variable and method name and corrected grammar/spelling mistakes if any
  • I have made corresponding changes to the documentation where needed
  • My changes generate no new/additional warnings
  • My change is not breaking or creating conflict with associated dependencies
  • I have performed a self-review of my own code
  • I ran mvn clean install before commit and all tests run successfully
  • I conducted basic QA to assure all features are working fine
  • My pull request generate no conflicts with master branch
  • I requested code review from other team members

@dladlk
Copy link

dladlk commented Nov 13, 2023

Are you sure that you need this change in the core statistics of Oxalis? You could just create your own module class PluginStatisticsModule with this provider method and activate it via reference.conf like

oxalis.module.as4.pluginstatistics {
     class=XXXXX.MyPluginStatisticsModule 
}

pack it to jar and use in your Oxalis deployment classpath...

@dladlk
Copy link

dladlk commented Nov 13, 2023

I've made a bunch of such modules during testing, you can take a look at the setup:
OxalisCommunity/Oxalis-AS4#228

@WimMaerevoet-Octopus
Copy link
Author

Yes, I know that is possible. But you need to copy the jar into the same folder as the rest of the oxalis jars. For deployment with a war, this is not so simple.
I did this PR to have a nice separation between 'own plugin code' and oxalis code.

@dladlk
Copy link

dladlk commented Nov 13, 2023

Do you suggest that EACH Oxalis component has such "-plugin" provider? Searching for "-plugin" in the source code I found nothing.

Isn't it too specific for your use case? For me it looks strange to have 2 different implementations distinguished by the "source" of the module... This new definition will be specific only for your module, which knows about existence of "statistics-plugin" named bean...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aaron-kumar aaron-kumar Awaiting requested review from aaron-kumar

@dladlk dladlk Awaiting requested review from dladlk

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@WimMaerevoet-Octopus @dladlk