JPA 2.1 right way (using eclipseLink) of updating an entity of my database?

Question

I would like to know if what i'm doing to read my database and update it, its the way to do it and its a good practice.

When i run my program i load my database into memory:

 public static void open() {
  EntityManagerFactory emf = Persistence.createEntityManagerFactory("htTeamManagerPU");
  EntityManager em = emf.createEntityManager();
  TypedQuery<Season> query = em.createQuery("select s from Season s",Season.class);
  seasons = query.getResultList(); }

By doing that i have in memory my database of players from a particular season.

Now i have my playerList which is a list of Players, and i want to update it in memory and after that update that player in my database. Can i do it differently as i already have the player in memory, i still have to do the find of the player and make the same changes i already did in memory before ?? seems that i'm doing the same changes twice...

public void finishTraining(int id){
  EntityManagerFactory emf = Persistence.createEntityManagerFactory("htTeamManagerPU");
  EntityManager em = emf.createEntityManager();
  em.getTransaction().begin();
   for (Player player: playerList){
       if (player.getPlayerId()==id) {
           player.setIsTeam((short)0);
           player.setIsToSell((short)1);
           //no i find the player in my database with id primary key
           Player p1 = em.find(Player.class, id);
           //make the same changes i already did in memory
           p1.setIsTeam((short)0);
           p1.setIsToSell((short)1);
           //submit my changes? there is another way of doing this?
           em.getTransaction().commit();

       }
   } 
   em.close();
   emf.close();
}

This is working fine and updating my databse. What i want to know if this is the right and best way to do it. Thanks

Answer 1

First of all you are not doing twice the same changes.

During the EntityManager life (it's a simplification because there are transaction's timeout concepts and more things) it monitors what is happening with the entities that retrieves so, if modified, their changes will be translated to database but once EntityManager life ends, all monitoring ends too. Problem is that you are used to implicit operations (in your code there is no EntityManager's merge, delete or remove function so it is the own EntityManager who cares about what to do with changes) instead of explicit (the use of EntityManager's merge, remove or create methods declared by programmer) so you probably believed that once entity was modified it was commited automatically but not: changes were handled by the EntityManager

Due to the fact that playerList was probably modified out of a EntityManager lifecycle no one cares about it's entities so changes are not refreshed at database and it causes your problem.

This lack of refresh is completely normal in the case i have described. For unhandled entities it's completely normal the process of get a entity manager, begin a transaction and so on.

About your question "What i want to know if this is the right and best way to do it", i have explained the concept but there are some code improvements i can advise. As example i would avoid implicit EntityManager operations to avoid confusion; using explicit method's i would change your method that way:

public void modifiedFinishTraining(int id){

/*This three lines are mandatory. You have to get an EntityManager to get access to     database and start a transaction to store your changes*/
EntityManagerFactory emf = Persistence.createEntityManagerFactory("htTeamManagerPU");
EntityManager em = emf.createEntityManager();
em.getTransaction().begin();

/*case 1: use merge method. If you want to update your player or create if it is not at database, use it and avoid the em.find(Player.class, id) step*/
for (Player player: playerList){

if (player.getPlayerId()==id) {
//This way you avoid the step of find by be careful: merge updates existing or creates a new one.
em.merge(player);
}

}
/*case 2: If you want to check if exists and you dont want to create it in other case  do it*/
for (Player player: playerList){
  if (player.getPlayerId()==id) {
      //you find the player in database
       Player p1 = em.find(Player.class, player.getPlayerId());
       //check if player exists and if true, update           
       if(p1  != null){
         p1.setIsTeam((short)0);
         p1.setIsToSell((short)1);

         em.merge(p1);
       }
   }
}

//this would be done at the end of the operation to commit all changes at the same time
em.getTransaction().commit();   
em.close();
emf.close();
/*the upper three lines are mandatory. You must commit your changes because transaction is manually handled and once commited you have to close all to free resources*/

}