-
Notifications
You must be signed in to change notification settings - Fork 2
Add tutorial using composite Id with Data REST #252
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @rashidi, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces a new, self-contained tutorial demonstrating best practices for integrating Spring Data REST with entities that utilize composite primary keys. The changes encompass new entity definitions, custom ID converters, and comprehensive integration tests, providing a clear example for developers facing similar challenges.
Highlights
- New Tutorial Project: I've added a new tutorial project,
data-rest-composite-id
, which demonstrates how to implement and expose entities with composite IDs through Spring Data REST. - Entity Modeling with Composite IDs: The tutorial showcases two entity types,
Author
andBook
, both utilizing@EmbeddedId
for their primary keys.Author
uses a simpleLong
wrapped in an embeddedId
class, whileBook
uses a more complexIsbn
class that parses an ISBN string into its components. - Spring Data REST ID Conversion: To properly handle composite IDs in the REST API, I've implemented
BackendIdConverter
for bothAuthor.Id
andBook.Isbn
. Additionally, anAuthorIdReferencedConverter
is registered to manage conversions when referencingAuthor
entities by their composite ID. - Comprehensive Testing: The new tutorial includes integration tests (
CreateAuthorTests
,CreateBookTests
,GetBookTests
) to validate the creation and retrieval of entities with composite IDs via the exposed REST endpoints, utilizing Testcontainers for a robust testing environment. - Documentation Updates: The main
README.adoc
and the project's navigation (docs/modules/ROOT/nav.adoc
) have been updated to include the new 'Spring Data REST with Composite ID' tutorial, with a dedicateddata-rest-composite-id.adoc
file providing detailed explanations.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request adds a new tutorial demonstrating how to use Spring Data REST with composite IDs. The changes include a new Spring Boot module with entities, repositories, converters, and tests, along with corresponding documentation.
The review identified several issues:
- A mis-generated Liquibase file with duplicate changesets.
- Potential
NullPointerException
s inequals
,hashCode
, andtoString
methods in the entity classes due to lack of null-safety. - A critical typo in a test configuration property.
- Use of a shaded dependency in tests, which is not a best practice.
I've provided suggestions to fix these issues to improve code quality, correctness, and maintainability.
*/ | ||
@Import(TestcontainersConfiguration.class) | ||
@Testcontainers | ||
@SpringBootTest(webEnvironment = RANDOM_PORT, properties = "spring.jpa.hibernate.Ïddl-auto=create-drop") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a typo in the properties
attribute. Ïddl-auto
should be ddl-auto
. The character Ï
is a non-standard ASCII character that was likely inserted by mistake. This will cause the property to be ignored, which could lead to unexpected test behavior or failures.
@SpringBootTest(webEnvironment = RANDOM_PORT, properties = "spring.jpa.hibernate.Ïddl-auto=create-drop") | |
@SpringBootTest(webEnvironment = RANDOM_PORT, properties = "spring.jpa.hibernate.ddl-auto=create-drop") |
public final boolean equals(Object o) { | ||
return o instanceof Id another && this.id.equals(another.id); | ||
} | ||
|
||
@Override | ||
public int hashCode() { | ||
return id.hashCode(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The equals
and hashCode
methods in the Id
class can throw a NullPointerException
if this.id
is null
. This can happen if an Author
entity is new and has not yet been persisted. It's safer to use java.util.Objects
to handle potential nulls.
public final boolean equals(Object o) { | |
return o instanceof Id another && this.id.equals(another.id); | |
} | |
@Override | |
public int hashCode() { | |
return id.hashCode(); | |
} | |
@Override | |
public final boolean equals(Object o) { | |
return o instanceof Id another && java.util.Objects.equals(this.id, another.id); | |
} | |
@Override | |
public int hashCode() { | |
return java.util.Objects.hashCode(id); | |
} |
data-rest-composite-id/src/main/java/zin/rashidi/datarest/compositeid/book/Book.java
Outdated
Show resolved
Hide resolved
public String toString() { | ||
return String.format("%d%d%d%d%d", prefix, group, registrant, publication, check); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The toString()
method is susceptible to a NullPointerException
because the %d
format specifier in String.format
does not handle null
Integer
values. This can happen if an Isbn
object is created via the default constructor and its fields are not initialized. Using the %s
specifier is a safer alternative as it handles nulls gracefully.
public String toString() { | |
return String.format("%d%d%d%d%d", prefix, group, registrant, publication, check); | |
} | |
@Override | |
public String toString() { | |
return String.format("%s%s%s%s%s", prefix, group, registrant, publication, check); | |
} |
import org.springframework.context.annotation.Import; | ||
import org.springframework.test.web.servlet.assertj.MockMvcTester; | ||
import org.testcontainers.junit.jupiter.Testcontainers; | ||
import org.testcontainers.shaded.org.apache.commons.lang3.math.NumberUtils; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using a shaded dependency from Testcontainers (org.testcontainers.shaded.org.apache.commons.lang3.math.NumberUtils
) is not recommended. Relying on shaded dependencies is risky as they might be changed or removed in future versions of Testcontainers without notice. It's better practice to declare a direct dependency on Apache Commons Lang3.
import org.testcontainers.shaded.org.apache.commons.lang3.math.NumberUtils; | |
import org.apache.commons.lang3.math.NumberUtils; |
Job Summary for GradleGradle Build :: gradle-build
|
Job Summary for GradleGradle Build :: gradle-build
|
Job Summary for GradleGradle Build :: gradle-build
|
|
No description provided.