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

Simplify core elements initialization with Spring #6882

Merged
merged 10 commits into from Mar 13, 2024

Conversation

jgiovaresco
Copy link
Contributor

@jgiovaresco jgiovaresco commented Mar 8, 2024

Description

The PR tries to simplify the spring configuration related to core elements.

As we don't want to reference Spring in the Core package, declaring beans was cumbersome because we had to add them in a Configuration file manually.

This PR suggests 2 remediations based on @ComponentScan annotation:

  • In this commit beans are discover using a regular expression. In is simple but not really readable and I'm not sure how it will behave if we create subpackages in domain_service...
  • The alternative is to rely on custom Annotation. (check relevant commit). I've introduced 2 annotations, @UseCase and @DomainService. Then, we can configure Component Scan to search for that annotation to discover beans.

I like the second approach for 2 reasons:

  • no regular expression: the component scan is easy to read
  • these annotations allow us to write inline documentation on the software architecture we'd like for APIM (a trick I read in Living Documentation book)

Additional context


📚  View the storybook of this branch here

@jgiovaresco jgiovaresco force-pushed the simplify-domain-service-registration-run-e2e branch 2 times, most recently from ccd0eaa to 2cd143b Compare March 11, 2024 13:31
Copy link
Member

@ytvnr ytvnr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The functional annotation @UseCase is definitely a better choice, but I would not have a separate @TransactionalUseCase

@jgiovaresco jgiovaresco force-pushed the simplify-domain-service-registration-run-e2e branch from 2cd143b to 61e0688 Compare March 11, 2024 14:25
Copy link

cypress bot commented Mar 11, 2024

Passing run #855 ↗︎

0 82 8 0 Flakiness 0

Details:

chore: remove empty lines
Project: APIM UI Tests Commit: 4a9bf5755e
Status: Passed Duration: 06:50 💡
Started: Mar 13, 2024 7:43 AM Ended: Mar 13, 2024 7:50 AM

Review all test suite changes for PR #6882 ↗︎

@jgiovaresco jgiovaresco marked this pull request as ready for review March 12, 2024 09:21
@jgiovaresco jgiovaresco requested a review from a team as a code owner March 12, 2024 09:21
@jgiovaresco jgiovaresco force-pushed the simplify-domain-service-registration-run-e2e branch from 61e0688 to 9ba8142 Compare March 12, 2024 09:30
Copy link
Member

@Okhelifi Okhelifi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done 👏🏼

Copy link
Member

@ytvnr ytvnr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do not forget to squash to avoid having the commit with the ComponentScan using pattern as it is not relevant anymore

@jgiovaresco jgiovaresco force-pushed the simplify-domain-service-registration-run-e2e branch from 9ba8142 to f2a35b8 Compare March 13, 2024 07:13
@jgiovaresco jgiovaresco force-pushed the simplify-domain-service-registration-run-e2e branch from f2a35b8 to 56dd256 Compare March 13, 2024 07:23
@jgiovaresco jgiovaresco force-pushed the simplify-domain-service-registration-run-e2e branch from 56dd256 to 4a9bf57 Compare March 13, 2024 07:29
@jgiovaresco jgiovaresco merged commit 266350d into master Mar 13, 2024
50 checks passed
@jgiovaresco jgiovaresco deleted the simplify-domain-service-registration-run-e2e branch March 13, 2024 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants