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

Upgrade spring boot to 3.x latest #149

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

c5ms
Copy link

@c5ms c5ms commented Nov 21, 2024

Why

Due to #141 , It's necessary to update the project with the latest spring boot version.
The DDD simple should have the modern technical stack to show how ddd gets implemented.
So, the project can be welcome to new committer with more ideas and best practices.

What

  • Upgrade the spring boot to 3.0
  • add spring doc with latest version for generating the open-api doc and embedded swagger-UI
  • add micrometer support for web request and database accessing

@c5ms c5ms changed the title Dev/springboot3 Upgrade spring boot to 3.0 Nov 21, 2024
@c5ms c5ms changed the title Upgrade spring boot to 3.0 Upgrade spring boot to 3.x latest Nov 21, 2024
@c5ms c5ms mentioned this pull request Nov 21, 2024
pom.xml Show resolved Hide resolved
@@ -4,7 +4,7 @@
* An entity, as explained in the DDD book.
*
*/
public interface Entity<T> {
public interface DomainEntity<T> {
Copy link
Contributor

@orende orende Nov 22, 2024

Choose a reason for hiding this comment

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

Why was this class renamed? Was it to avoid a namespace collision with JPA?

Copy link
Author

Choose a reason for hiding this comment

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

yes, IMHO, this class should not use a single word.

pom.xml Show resolved Hide resolved
pom.xml Show resolved Hide resolved
@@ -39,7 +40,8 @@
@RequestMapping("/admin")
public final class CargoAdminController {

private final BookingServiceFacade bookingServiceFacade;
@Autowired
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this dependency be injected into the constructor instead?

Copy link
Author

Choose a reason for hiding this comment

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

I think it's hard and dangerous to do it since the injection currently has some circular.
It's the next step I'm planning.

Copy link
Contributor

@orende orende Nov 22, 2024

Choose a reason for hiding this comment

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

Okay, got it. In that case, could you add a comment to this field explaining that it cannot be moved into the constructor params due to the circular dependency problem? Otherwise, my knee-jerk reaction seeing that autowired field would be to inline it 😃

Copy link
Author

Choose a reason for hiding this comment

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

I got the reason by new investigation and make it inline with constructor

Copy link
Author

Choose a reason for hiding this comment

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

May I know why some beans are defined in the context file (like DDDSampleApplicationContext) but some are defined by the annotation?

I think the domain layer should not have the runtime knowledge like CDI, but the infrastructure and interfaces may can do it.

I have some idea for optimizing the bean definitions to:

  1. no circular reference
  2. no duplicate bean definition

But I'm wondering about how to define the beans with the original principle.

pom.xml Show resolved Hide resolved
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