Conversation
Add persistenceconfiguration in Main. Add Entity in Film. Co-authored-by: Mattias Barth mattias.barth@gmail.com EdvinSandgren 229709012+EdvinSandgren@users.noreply.github.com johanhiths Johan.hietala@iths.se
Co-authored-by: Mattias Barth mattias.barth@gmail.com EdvinSandgren 229709012+EdvinSandgren@users.noreply.github.com johanhiths Johan.hietala@iths.se
Co-authored-by: Mattias Barth mattias.barth@gmail.com EdvinSandgren 229709012+EdvinSandgren@users.noreply.github.com johanhiths Johan.hietala@iths.se
Add ManyToOne in Film Class. Co-authored-by: Mattias Barth mattias.barth@gmail.com EdvinSandgren 229709012+EdvinSandgren@users.noreply.github.com johanhiths Johan.hietala@iths.se
Co-authored-by: Mattias Barth mattias.barth@gmail.com EdvinSandgren 229709012+EdvinSandgren@users.noreply.github.com johanhiths Johan.hietala@iths.se
Co-authored-by: Mattias Barth mattias.barth@gmail.com EdvinSandgren 229709012+EdvinSandgren@users.noreply.github.com johanhiths Johan.hietala@iths.se
Ok by group
…ain-class Co-authored-by: Mattias Barth mattias.barth@gmail.com
📝 WalkthroughWalkthroughThis pull request introduces a JPA-based persistence layer with two entity classes—Director and Film—featuring a one-to-many relationship. It adds a Main entry point that demonstrates entity creation, persistence, and retrieval operations through Hibernate. A GitHub Classroom deadline badge is also added to README.md. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In @src/main/java/Film.java:
- Around line 49-51: getAllFilms should validate the EntityManager parameter to
avoid a NullPointerException; at the start of the method (in
getAllFilms(EntityManager em) of class Film) add a null check for em and throw
an IllegalArgumentException (or another appropriate runtime exception) with a
clear message like "EntityManager must not be null" if it is null, otherwise
proceed to call em.createQuery(...).
- Around line 41-47: The static addFilm method on Film mixes persistence with
the entity and lacks null/empty validation; validate inputs (check em, title
non-null/non-empty, director non-null) and fail fast (throw
IllegalArgumentException) before creating/persisting the Film, and then move
this persistence logic out of the Film entity into a dedicated repository/DAO
(e.g., FilmRepository.addFilm or FilmDao.create) that owns the EntityManager
usage; keep the Film class as a plain entity and have the new repository handle
instantiation, validation, em.persist, and any logging.
- Around line 54-58: In getFilmsByDirector validate inputs at the top: check
that EntityManager em and Director director are not null and if either is null
throw an IllegalArgumentException (with a clear message like "em must not be
null" or "director must not be null") before using them; keep the rest of the
method unchanged so the query uses the validated parameters.
In @src/main/java/Main.java:
- Line 26: The director name has a typo: update the string passed to
director.setName from "Christopher Bolan" to the correct "Christopher Nolan" by
editing the call to director.setName in the code where the director variable is
set.
- Line 32: Replace the incorrect film title passed to Film.addFilm: change the
string "The Dirk Knight" to "The Dark Knight" in the Film.addFilm(em, "The Dirk
Knight", director) call (and any other occurrences of the same typo) so the
movie title is spelled correctly when creating the Film record.
- Around line 14-16: The code in Main.java currently hardcodes DB settings via
jdbcUrl, jdbcUsername, and jdbcPassword; replace these literal values with
configuration via environment variables or a config provider: read
System.getenv("DB_JDBC_URL"), System.getenv("DB_USERNAME"), and
System.getenv("DB_PASSWORD") (or use your existing config loader) and pass those
values into the .jdbcUrl/.jdbcUsername/.jdbcPassword calls, validating that
required variables exist and failing fast with a clear error if they are
missing; remove the hardcoded "jdbc:mysql://localhost:3306/film_database",
"root", and "root" literals and provide sensible defaults only when appropriate
and documented.
🧹 Nitpick comments (7)
src/main/java/Main.java (1)
45-56: Remove commented-out code.Dead code should be removed to improve readability. Version control preserves history if you need to refer back to it.
🧹 Proposed cleanup
- //If no Films in database, add some -// if (em.createQuery("select count(o) from Film o", Long.class) -// .getSingleResult() == 0) { -// Film film1 = new Film(); -// em.persist(film1); -// em.flush(); -// Film film2 = new Film(); -// em.persist(film2); -// } -// System.out.println("==== Using select query, N + 1 ===="); -// em.createQuery("from Film", Film.class) -// .getResultList().forEach(System.out::println); -src/main/java/Film.java (2)
46-46: Replace System.out.println with a logging framework.For better control over log levels and output formatting, consider using a logging framework like SLF4J with Logback.
12-14: Consider adding cascade and fetch configuration.Depending on your use case, you may want to specify cascade options (e.g.,
CascadeType.PERSIST,CascadeType.MERGE) and fetch type (e.g.,FetchType.LAZY) for the relationship.Example:
@ManyToOne(fetch = FetchType.LAZY) @JoinColumn(name = "director_id") private Director director;src/main/java/Director.java (4)
4-4: Remove unused import.The
Listimport is not used in this file.🧹 Proposed cleanup
import java.util.HashSet; -import java.util.List; import java.util.Set;
20-20: Remove redundant annotation attribute.The
nullable = trueattribute is redundant since columns are nullable by default in JPA. You can simply use@Columnor omit the annotation entirely.🧹 Proposed simplification
- @Column(nullable = true) private Integer yearOfDeath;
14-15: Consider specifying cascade and orphanRemoval options.For better control over the relationship lifecycle, consider adding cascade types and
orphanRemovalto automatically manage child entities when they're removed from the collection.Example:
@OneToMany(mappedBy = "director", cascade = CascadeType.ALL, orphanRemoval = true) private Set<Film> films = new HashSet<>();
23-29: Consider adding bidirectional synchronization helper methods.To maintain consistency in the bidirectional relationship, consider adding helper methods that update both sides of the association.
🔗 Example implementation
public void addFilm(Film film) { films.add(film); film.setDirector(this); } public void removeFilm(Film film) { films.remove(film); film.setDirector(null); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
README.mdsrc/main/java/Director.javasrc/main/java/Film.javasrc/main/java/Main.java
🧰 Additional context used
🧬 Code graph analysis (3)
src/main/java/Main.java (2)
src/main/java/Director.java (1)
Entity(7-71)src/main/java/Film.java (1)
Entity(5-61)
src/main/java/Film.java (1)
src/main/java/Director.java (1)
Entity(7-71)
src/main/java/Director.java (1)
src/main/java/Film.java (1)
Entity(5-61)
🔇 Additional comments (3)
README.md (1)
1-1: LGTM!The GitHub Classroom badge addition is appropriate for tracking assignment deadlines.
src/main/java/Film.java (1)
7-14: LGTM!The entity mapping is correctly configured with appropriate annotations for the primary key and the many-to-one relationship to Director.
src/main/java/Director.java (1)
10-15: LGTM!The entity mapping with the one-to-many relationship is correctly configured. The
mappedByattribute properly references the owner side of the bidirectional relationship.
| public static void addFilm(EntityManager em, String title, Director director) { | ||
| Film film = new Film(); | ||
| film.setTitle(title); | ||
| film.setDirector(director); | ||
| em.persist(film); | ||
| System.out.println("Film tillagd: " + title); | ||
| } |
There was a problem hiding this comment.
Add null validation and consider extracting to a repository class.
The method lacks null checks for parameters, which could lead to NullPointerException or constraint violations. Additionally, mixing data access logic with entity classes violates separation of concerns—consider moving CRUD operations to a dedicated repository or DAO class.
✅ Proposed fix with validation
public static void addFilm(EntityManager em, String title, Director director) {
+ if (em == null) {
+ throw new IllegalArgumentException("EntityManager cannot be null");
+ }
+ if (title == null || title.trim().isEmpty()) {
+ throw new IllegalArgumentException("Title cannot be null or empty");
+ }
+ if (director == null) {
+ throw new IllegalArgumentException("Director cannot be null");
+ }
Film film = new Film();
film.setTitle(title);
film.setDirector(director);
em.persist(film);
System.out.println("Film tillagd: " + title);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public static void addFilm(EntityManager em, String title, Director director) { | |
| Film film = new Film(); | |
| film.setTitle(title); | |
| film.setDirector(director); | |
| em.persist(film); | |
| System.out.println("Film tillagd: " + title); | |
| } | |
| public static void addFilm(EntityManager em, String title, Director director) { | |
| if (em == null) { | |
| throw new IllegalArgumentException("EntityManager cannot be null"); | |
| } | |
| if (title == null || title.trim().isEmpty()) { | |
| throw new IllegalArgumentException("Title cannot be null or empty"); | |
| } | |
| if (director == null) { | |
| throw new IllegalArgumentException("Director cannot be null"); | |
| } | |
| Film film = new Film(); | |
| film.setTitle(title); | |
| film.setDirector(director); | |
| em.persist(film); | |
| System.out.println("Film tillagd: " + title); | |
| } |
🤖 Prompt for AI Agents
In @src/main/java/Film.java around lines 41 - 47, The static addFilm method on
Film mixes persistence with the entity and lacks null/empty validation; validate
inputs (check em, title non-null/non-empty, director non-null) and fail fast
(throw IllegalArgumentException) before creating/persisting the Film, and then
move this persistence logic out of the Film entity into a dedicated
repository/DAO (e.g., FilmRepository.addFilm or FilmDao.create) that owns the
EntityManager usage; keep the Film class as a plain entity and have the new
repository handle instantiation, validation, em.persist, and any logging.
| public static List<Film> getAllFilms(EntityManager em) { | ||
| return em.createQuery("FROM Film", Film.class).getResultList(); | ||
| } |
There was a problem hiding this comment.
Add null validation for EntityManager.
The method should validate that the EntityManager parameter is not null to prevent NullPointerException.
✅ Proposed fix
public static List<Film> getAllFilms(EntityManager em) {
+ if (em == null) {
+ throw new IllegalArgumentException("EntityManager cannot be null");
+ }
return em.createQuery("FROM Film", Film.class).getResultList();
}🤖 Prompt for AI Agents
In @src/main/java/Film.java around lines 49 - 51, getAllFilms should validate
the EntityManager parameter to avoid a NullPointerException; at the start of the
method (in getAllFilms(EntityManager em) of class Film) add a null check for em
and throw an IllegalArgumentException (or another appropriate runtime exception)
with a clear message like "EntityManager must not be null" if it is null,
otherwise proceed to call em.createQuery(...).
| public static List<Film> getFilmsByDirector(EntityManager em, Director director) { | ||
| return em.createQuery("FROM Film f WHERE f.director = :director", Film.class) | ||
| .setParameter("director", director) | ||
| .getResultList(); | ||
| } |
There was a problem hiding this comment.
Add null validation for parameters.
Both EntityManager and Director parameters should be validated to prevent NullPointerException.
✅ Proposed fix
public static List<Film> getFilmsByDirector(EntityManager em, Director director) {
+ if (em == null) {
+ throw new IllegalArgumentException("EntityManager cannot be null");
+ }
+ if (director == null) {
+ throw new IllegalArgumentException("Director cannot be null");
+ }
return em.createQuery("FROM Film f WHERE f.director = :director", Film.class)
.setParameter("director", director)
.getResultList();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public static List<Film> getFilmsByDirector(EntityManager em, Director director) { | |
| return em.createQuery("FROM Film f WHERE f.director = :director", Film.class) | |
| .setParameter("director", director) | |
| .getResultList(); | |
| } | |
| public static List<Film> getFilmsByDirector(EntityManager em, Director director) { | |
| if (em == null) { | |
| throw new IllegalArgumentException("EntityManager cannot be null"); | |
| } | |
| if (director == null) { | |
| throw new IllegalArgumentException("Director cannot be null"); | |
| } | |
| return em.createQuery("FROM Film f WHERE f.director = :director", Film.class) | |
| .setParameter("director", director) | |
| .getResultList(); | |
| } |
🤖 Prompt for AI Agents
In @src/main/java/Film.java around lines 54 - 58, In getFilmsByDirector validate
inputs at the top: check that EntityManager em and Director director are not
null and if either is null throw an IllegalArgumentException (with a clear
message like "em must not be null" or "director must not be null") before using
them; keep the rest of the method unchanged so the query uses the validated
parameters.
| .jdbcUrl("jdbc:mysql://localhost:3306/film_database") | ||
| .jdbcUsername("root") | ||
| .jdbcPassword("root") |
There was a problem hiding this comment.
Remove hardcoded database credentials.
Hardcoded credentials pose a security risk and make the application less flexible. Consider using environment variables, a configuration file, or a secrets management system.
🔒 Proposed fix using environment variables
- .jdbcUrl("jdbc:mysql://localhost:3306/film_database")
- .jdbcUsername("root")
- .jdbcPassword("root")
+ .jdbcUrl(System.getenv().getOrDefault("DB_URL", "jdbc:mysql://localhost:3306/film_database"))
+ .jdbcUsername(System.getenv().getOrDefault("DB_USER", "root"))
+ .jdbcPassword(System.getenv().getOrDefault("DB_PASSWORD", ""))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .jdbcUrl("jdbc:mysql://localhost:3306/film_database") | |
| .jdbcUsername("root") | |
| .jdbcPassword("root") | |
| .jdbcUrl(System.getenv().getOrDefault("DB_URL", "jdbc:mysql://localhost:3306/film_database")) | |
| .jdbcUsername(System.getenv().getOrDefault("DB_USER", "root")) | |
| .jdbcPassword(System.getenv().getOrDefault("DB_PASSWORD", "")) |
🤖 Prompt for AI Agents
In @src/main/java/Main.java around lines 14 - 16, The code in Main.java
currently hardcodes DB settings via jdbcUrl, jdbcUsername, and jdbcPassword;
replace these literal values with configuration via environment variables or a
config provider: read System.getenv("DB_JDBC_URL"),
System.getenv("DB_USERNAME"), and System.getenv("DB_PASSWORD") (or use your
existing config loader) and pass those values into the
.jdbcUrl/.jdbcUsername/.jdbcPassword calls, validating that required variables
exist and failing fast with a clear error if they are missing; remove the
hardcoded "jdbc:mysql://localhost:3306/film_database", "root", and "root"
literals and provide sensible defaults only when appropriate and documented.
| emf.runInTransaction(em -> { | ||
| //Skapa regissör för test | ||
| Director director = new Director(); | ||
| director.setName("Christopher Bolan"); |
There was a problem hiding this comment.
Fix typo in director name.
"Christopher Bolan" should likely be "Christopher Nolan".
✏️ Proposed fix
- director.setName("Christopher Bolan");
+ director.setName("Christopher Nolan");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| director.setName("Christopher Bolan"); | |
| director.setName("Christopher Nolan"); |
🤖 Prompt for AI Agents
In @src/main/java/Main.java at line 26, The director name has a typo: update the
string passed to director.setName from "Christopher Bolan" to the correct
"Christopher Nolan" by editing the call to director.setName in the code where
the director variable is set.
| em.persist(director); | ||
|
|
||
| //Testa att lägga till filmer | ||
| Film.addFilm(em, "The Dirk Knight", director); |
There was a problem hiding this comment.
Fix typo in film title.
"The Dirk Knight" should likely be "The Dark Knight".
✏️ Proposed fix
- Film.addFilm(em, "The Dirk Knight", director);
+ Film.addFilm(em, "The Dark Knight", director);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Film.addFilm(em, "The Dirk Knight", director); | |
| Film.addFilm(em, "The Dark Knight", director); |
🤖 Prompt for AI Agents
In @src/main/java/Main.java at line 32, Replace the incorrect film title passed
to Film.addFilm: change the string "The Dirk Knight" to "The Dark Knight" in the
Film.addFilm(em, "The Dirk Knight", director) call (and any other occurrences of
the same typo) so the movie title is spelled correctly when creating the Film
record.
1 participant
Add crud-methods in Film-class. Add test code for crud-operations i Main-class
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.