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

Tsystems 211 upgrade spring boot #707

Merged
merged 57 commits into from
Jan 29, 2025
Merged

Conversation

tkuzynow
Copy link
Contributor

@tkuzynow tkuzynow commented Jan 8, 2025

Fixes #

Proposed Changes

@vi-sudo
Copy link

vi-sudo commented Jan 8, 2025

Copy link
Member

@patric-dosch-vi patric-dosch-vi left a comment

Choose a reason for hiding this comment

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

hey @tkuzynow, added some comments, and for now I did not had a look at the tests, is that ok?

<phase>process-sources</phase>
<configuration>
<target>
<delete file="${project.build.directory}/generated-sources/de/caritas/cob/userservice/api/adapters/web/dto/GroupSessionConsultantDTO.java"/>
Copy link
Member

Choose a reason for hiding this comment

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

oh for what is this needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I encountered some issues with file access when running tests after upgrade and this fixed it

Copy link
Member

Choose a reason for hiding this comment

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

ok, got it. but this is quite unhandy for the future :( no change to get rid of this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't have another solution, without the change we're getitng errors like this on compile:

image

Copy link
Member

Choose a reason for hiding this comment

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

sad, but ok

import org.springframework.data.annotation.CreatedDate;
import org.springframework.data.annotation.LastModifiedDate;
import org.springframework.data.jpa.domain.support.AuditingEntityListener;

/** Represents a user */
@Entity
@Table(name = "user")
@Table(name = "`user`")
Copy link
Member

Choose a reason for hiding this comment

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

really?

Copy link
Contributor Author

@tkuzynow tkuzynow Jan 23, 2025

Choose a reason for hiding this comment

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

yes :) it was a fix for tests in H2.
But unfortunately this did not help for MariaDB, so eventually I needed to update the table name to _USER, as user is the reserved keyword in MariaDB, Hibernate in new version threw various exceptions when the table was named user or USER.

Copy link
Member

Choose a reason for hiding this comment

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

wanted always to ask for what is the reserved keyword, but now you say it, for MariaDB.
To be honest, I have no better idea than your rename suggestion :/

@@ -0,0 +1,10 @@
@FilterDefs({
Copy link
Member

Choose a reason for hiding this comment

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

oh this is ähm nice ;) so you can define that in the package-info that it matches for all classes inside?

Copy link
Member

Choose a reason for hiding this comment

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

what replaces that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

there was a tenantFilter defined in all entity classes, now we need to define this on package level, which for tenantFilter is actually a benefit :)

Copy link
Member

Choose a reason for hiding this comment

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

is this a must? interesting, then it would not be possible to have a filter only for dedicated classes.

Copy link
Contributor Author

@tkuzynow tkuzynow Jan 24, 2025

Choose a reason for hiding this comment

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

I think the problem was with repetition of filterDef in the context for multiple entities, so you can still configure it per entity but you cannot repeat same filter definition

@tkuzynow tkuzynow merged commit 4f607b2 into develop Jan 29, 2025
3 of 5 checks passed
@tkuzynow tkuzynow deleted the TSYSTEMS-211-upgrade-spring-boot branch January 29, 2025 11:56
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.

3 participants