-
Notifications
You must be signed in to change notification settings - Fork 19
Johan #13
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
base: main
Are you sure you want to change the base?
Johan #13
Changes from 20 commits
cd58553
f043eb4
86c2ac2
5c33218
c5b0d5f
3f3262f
5125f3a
bc6883c
4d51d1f
be0e7b4
5427500
0135fb8
cc6a16e
719fa14
27cc14b
d253f70
438577a
b27871c
41fcca0
02a6b0e
0066a05
9bf2aca
55889b0
887f1e1
ad87eed
daa00c8
effd2b1
d330599
01bf50a
3c10c42
9b8ceaa
109988c
57b7d4e
b043f55
e2f2c57
42d834d
cfde7f8
ba087fb
a6de7d4
a1beeb5
e91d463
e9fb1ae
4929c8f
1ea47d7
af12454
cff7dee
3d148c3
5dff5c6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| import jakarta.persistence.*; | ||
|
|
||
| import java.util.HashSet; | ||
| import java.util.List; | ||
| import java.util.Set; | ||
|
|
||
| @Entity | ||
| public class Director { | ||
|
|
||
| @Id | ||
| @GeneratedValue(strategy = GenerationType.IDENTITY) | ||
| private Long id; | ||
|
|
||
| @OneToMany(mappedBy = "director") | ||
| private Set<Film> films = new HashSet<>(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Incomplete relationship mapping compared to This version lacks the 🤖 Prompt for AI Agents |
||
|
|
||
| private String name; | ||
| private String country; | ||
| private int birthYear; | ||
| @Column(nullable = true) | ||
| private Integer yearOfDeath; | ||
|
|
||
|
|
||
|
|
||
| public Set<Film> getFilms() { | ||
| return films; | ||
| } | ||
|
|
||
| public void setFilms(Set<Film> films) { | ||
| this.films = films; | ||
| } | ||
|
|
||
| public Integer getYearOfDeath() { | ||
| return yearOfDeath; | ||
| } | ||
|
|
||
| public void setYearOfDeath(Integer yearOfDeath) { | ||
| this.yearOfDeath = yearOfDeath; | ||
| } | ||
|
|
||
| public int getBirthYear() { | ||
| return birthYear; | ||
| } | ||
|
|
||
| public void setBirthYear(int birthYear) { | ||
| this.birthYear = birthYear; | ||
| } | ||
|
|
||
| public String getCountry() { | ||
| return country; | ||
| } | ||
|
|
||
| public void setCountry(String country) { | ||
| this.country = country; | ||
| } | ||
|
|
||
| public String getName() { | ||
| return name; | ||
| } | ||
|
|
||
| public void setName(String name) { | ||
| this.name = name; | ||
| } | ||
|
|
||
| public void setId(Long id) { | ||
| this.id = id; | ||
| } | ||
|
|
||
| public Long getId() { | ||
| return id; | ||
| } | ||
|
|
||
|
|
||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| import jakarta.persistence.*; | ||
|
|
||
| @Entity | ||
| public class Film { | ||
| @Id | ||
| @GeneratedValue(strategy = GenerationType.IDENTITY) | ||
| private Long id; | ||
|
|
||
| private String title; | ||
| @ManyToOne | ||
| @JoinColumn(name = "director_id") | ||
| private Director director; | ||
|
|
||
| public Director getDirector() { | ||
| return director; | ||
| } | ||
|
|
||
| public void setDirector(Director director) { | ||
| this.director = director; | ||
| } | ||
|
|
||
| public String getTitle() { | ||
| return title; | ||
| } | ||
|
|
||
| public void setTitle(String title) { | ||
| this.title = title; | ||
| } | ||
|
|
||
| public void setId(Long id) { | ||
| this.id = id; | ||
| } | ||
|
|
||
| public Long getId() { | ||
| return id; | ||
| } | ||
|
|
||
|
|
||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,42 @@ | ||||||||||||||
| import jakarta.persistence.Entity; | ||||||||||||||
| import jakarta.persistence.EntityManagerFactory; | ||||||||||||||
| import jakarta.persistence.Persistence; | ||||||||||||||
| import jakarta.persistence.PersistenceConfiguration; | ||||||||||||||
| import org.hibernate.jpa.HibernatePersistenceConfiguration; | ||||||||||||||
|
|
||||||||||||||
| import java.util.List; | ||||||||||||||
|
|
||||||||||||||
| public class Main { | ||||||||||||||
| public static void main(String[] args) { | ||||||||||||||
|
|
||||||||||||||
|
|
||||||||||||||
| final PersistenceConfiguration cfg = new HibernatePersistenceConfiguration("emf") | ||||||||||||||
| .jdbcUrl("jdbc:mysql://localhost:3306/film_database") | ||||||||||||||
| .jdbcUsername("root") | ||||||||||||||
| .jdbcPassword("root") | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hardcoded database credentials pose a security risk. Database credentials should not be committed to source control. Consider using environment variables, configuration files excluded from version control, or a secrets management solution. ♻️ Suggested approach using environment variables- .jdbcUrl("jdbc:mysql://localhost:3306/film_database")
- .jdbcUsername("root")
- .jdbcPassword("root")
+ .jdbcUrl(System.getenv("DATABASE_URL"))
+ .jdbcUsername(System.getenv("DATABASE_USER"))
+ .jdbcPassword(System.getenv("DATABASE_PASSWORD"))📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||
| .property("hibernate.hbm2ddl.auto", "update") | ||||||||||||||
| .property("hibernate.show_sql", "true") | ||||||||||||||
| .property("hibernate.format_sql", "true") | ||||||||||||||
| .property("hibernate.highlight_sql", "true") | ||||||||||||||
| .managedClasses(Film.class, Director.class); | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Duplicate entity definitions exist across packages.
🤖 Prompt for AI Agents |
||||||||||||||
| try (EntityManagerFactory emf = cfg.createEntityManagerFactory()) { | ||||||||||||||
| emf.runInTransaction(em -> { | ||||||||||||||
| //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); | ||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Seeded Film entities have no meaningful data and lack a The 🤖 Prompt for AI Agents |
||||||||||||||
|
|
||||||||||||||
| }); | ||||||||||||||
|
|
||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| package org.example; | ||
|
|
||
| import jakarta.persistence.GeneratedValue; | ||
| import jakarta.persistence.GenerationType; | ||
| import jakarta.persistence.Id; | ||
| import jakarta.persistence.MappedSuperclass; | ||
|
|
||
| @MappedSuperclass | ||
| public abstract class BaseEntity { | ||
|
|
||
| @Id | ||
| @GeneratedValue(strategy = GenerationType.IDENTITY) | ||
| private Long id; | ||
|
|
||
| public Long getId() { | ||
| return id; | ||
| } | ||
|
|
||
| } |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,90 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||
| package org.example; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| import jakarta.persistence.EntityManager; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| import java.util.Optional; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| public class BaseRepositoryImpl<T extends org.example.BaseEntity> implements Repository<T> { | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| protected EntityManager em; | ||||||||||||||||||||||||||||||||||||||||||||||||
| protected Class<T> entityClass; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| public BaseRepositoryImpl(EntityManager em, Class<T> entityClass) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| this.em = em; | ||||||||||||||||||||||||||||||||||||||||||||||||
| this.entityClass = entityClass; | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+7
to
+19
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. EntityManager can be null when methods are called. The Consider either:
🐛 Option 1: Require EntityManager in constructor- public BaseRepositoryImpl(Class<T> entityClass) {
+ public BaseRepositoryImpl(Class<T> entityClass, EntityManager em) {
this.entityClass = entityClass;
+ this.em = em;
}
-
- public void setEntityManager(EntityManager em) {
- this.em = em;
- }📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| public T save(T entity) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| if (entity == null) { | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| throw new IllegalArgumentException("Entity cannot be null"); | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
| if (entity.getId() == null) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| em.persist(entity); | ||||||||||||||||||||||||||||||||||||||||||||||||
| return entity; | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
| else { | ||||||||||||||||||||||||||||||||||||||||||||||||
| return em.merge(entity); | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| public EntityManager getEntityManager() { | ||||||||||||||||||||||||||||||||||||||||||||||||
| return em; | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||||||||||||||||||||||||
| public Optional<T> findById(Long id) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| return Optional.ofNullable(em.find(entityClass, id)); | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||||||||||||||||||||||||
| public void delete(T entity) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| em.remove(em.contains(entity) ? entity : em.merge(entity)); | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||||||||||||||||||||||||
| public void deleteById(Long id) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| T entity = em.find(entityClass, id); | ||||||||||||||||||||||||||||||||||||||||||||||||
| if (entity != null) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| em.remove(entity); | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||||||||||||||||||||||||
| public Iterable<T> findAll() { | ||||||||||||||||||||||||||||||||||||||||||||||||
| return em.createQuery( | ||||||||||||||||||||||||||||||||||||||||||||||||
| "select e from " + entityClass.getSimpleName() + " e", entityClass | ||||||||||||||||||||||||||||||||||||||||||||||||
| ).getResultList(); | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||||||||||||||||||||||||
| public boolean existsById(Long id) { | ||||||||||||||||||||||||||||||||||||||||||||||||
| Long count = em.createQuery( | ||||||||||||||||||||||||||||||||||||||||||||||||
| "select count(e) from " + entityClass.getSimpleName() + " e where e.id = :id", | ||||||||||||||||||||||||||||||||||||||||||||||||
| Long.class | ||||||||||||||||||||||||||||||||||||||||||||||||
| ).setParameter("id", id).getSingleResult(); | ||||||||||||||||||||||||||||||||||||||||||||||||
| return count > 0; | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||||||||||||||||||||||||
| public long count() { | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| return em.createQuery("select count(e) from " + entityClass.getSimpleName() + " e", Long.class).getSingleResult(); | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||||||||||||||||||||||||
| public void flush() { | ||||||||||||||||||||||||||||||||||||||||||||||||
| em.flush(); | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| @Override | ||||||||||||||||||||||||||||||||||||||||||||||||
| public void clear() { | ||||||||||||||||||||||||||||||||||||||||||||||||
| em.clear(); | ||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,6 @@ | ||||||||||||||||||
| package org.example; | ||||||||||||||||||
|
|
||||||||||||||||||
| public class BusinessException extends Throwable { | ||||||||||||||||||
| public BusinessException(String s) { | ||||||||||||||||||
| } | ||||||||||||||||||
|
Comment on lines
+3
to
+5
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Exception message is discarded; consider extending Two issues:
🐛 Proposed fix-public class BusinessException extends Throwable {
- public BusinessException(String s) {
- }
+public class BusinessException extends Exception {
+ public BusinessException(String message) {
+ super(message);
+ }
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||
| } | ||||||||||||||||||
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.
🧩 Analysis chain
🏁 Script executed:
Repository: ithsjava25/ithsjava25-java-project-jpa-maven-java-template
Length of output: 844
🏁 Script executed:
Repository: ithsjava25/ithsjava25-java-project-jpa-maven-java-template
Length of output: 959
🏁 Script executed:
Repository: ithsjava25/ithsjava25-java-project-jpa-maven-java-template
Length of output: 200
Critical: Duplicate
Directorentity will cause JPA conflicts.This
Directorclass in the default package duplicatessrc/main/java/org/example/Director.java. Having two@Entityclasses with the same name will cause JPA/Hibernate mapping conflicts and unpredictable behavior. The same issue exists withFilm.javain the default package duplicatingsrc/main/java/org/example/Film.java.Additionally, the unused import
java.util.List(line 4) should be removed.Both files in the default package should be deleted. The properly organized entities in the
org.examplepackage should be used instead.🤖 Prompt for AI Agents