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

Jollyday Pojos using only regular plain old java objects for configuration #451

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

gandulf
Copy link
Contributor

@gandulf gandulf commented Feb 2, 2024

Hi,
I plan to use jollydays in a quarkus graalvm microservice projects where i try to keep additional dependencies especially regarding jackson/jaxb etc, to a minimum.

Therefore I made a jollyday implementation that only uses plain old java objects for configuration.
I know that this means that a change/reconfiguration of the holiday data is no longer possible but in the past years I've been using it I never needed that flexibility since the holiday definitions are rather global and consistent.

Disclaimer:
At the moment the POJOs are generated based on the xml files inside a pseudo test class "JavaGeneratorTest" that takes the JacksonConfiguration Classes and creates the POJOs SourceCode using ugly String Writer logic.
Not my proudest part of java code, but it works is only needed in case the configuration changes and needs to be recreated.

@derTobsch
Copy link
Contributor

Hey @gandulf,
thanks for your PR and the idea to reduce the dependencies and write the holidays into POJOs. I had not the time in the last week to think deeper about this.

Some thoughts are:

  • We need one source of truth for the holidays
  • It should create the POJOs on compile time
  • We remove the possibility, for this module, to override the xml files with custom holidays e.g.
  • We need to unify the tests (We also need to do that with jackson and jaxb)
  • ...

@gako
Copy link

gako commented Mar 8, 2024

Hey,
also quite busy so sorry for the delay :-)

  • The single source of truth should be the xml files, I agreee. That's why I'm generating the POJOs from them, they are also easier to read/edit/understand.
  • Also true, I haven't looked into the pregeneration of source files, I know its possible since MapStruct e.g does it, not sure how complicated and what limitations there are.
  • Editing of the holidays via xml is no longer possible that's one of the drawbacks of this solution, to mitigate this a bit the POJOs could provide functionality to at least modifiy/add configurations in Java.
  • Ahh testing, the most important and most overlooked issue, yes would be nice to unify them.

I'll look into the code generation stuff as soon as there is some spare time...

@gandulf
Copy link
Contributor Author

gandulf commented Mar 13, 2024

I replaced the manual code generation with a proper maven plugin generator that creates the necessary holiday config files during/before compilation automatically and puts them into /target/generated_sources that way they no longer need to be versioned by git and are always uptodate with the xml files.

I also added a test case that adds an additional holiday to the existing ones in the PojoConfigurationService to prove that it is still possible to add,replace holiday configuration if necessary. It's not as convenient as with the xml files but still possible.

@derTobsch derTobsch force-pushed the jollyday-pojo branch 4 times, most recently from 926aecb to 92d094e Compare April 18, 2024 15:19
@derTobsch
Copy link
Contributor

I wanted to take a deeper look.

  • I rebased your branch against the actual main
  • Deleted the "deploy to local repo" commits. I think you can just use the install phase.
  • squashed some commits with the same message

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.

None yet

3 participants