entityConverter and mgr.merge and a workaround

[deanhiller2000]

I have fought this problem for years and thought I might post a work around that someone helped me come up with recently.  This problem occurs because of a bad design of hibernate where it validates constraints too early(ie. it won't wait until the transaction commit which is really what I want)

     //BIG BIG NOTE:  We wanted to have this code
     //newNode = mgr.merge(newNode);
     //script.addNode(newNode);
     //
     //Unfortunately, then you get a ConstraintViolationException on mgr.merge because
     //you are merging a node without a script and script is required!!!!
     //Then we move to this code....
     //script.addNode(newNode);
     //newNode =mgr.merge(newNode);
     //
     //This code then fails later on when script.getImmutableNodes is called as
     //all nodes in the List MUST have an id and adding the newNode before the merge
     //causes a node without an id to be added to the script.  Only the newNode
     //returned from mgr.merge has an id.  Realize the newNode returned from mgr.merge
     //IS A DIFFERENT instance than the newNode passed into mgr.merge so basically
     //the script has the wrong instance of newNode with this code and an exception
     //happens later when the List of entities is passed through the entityConverter
     //The other problem is you should not be giving transient instances of Node to
     //the script as you will have merge problems later sometimes.
     public void save() {     
          Node temp = newNode;
          script.addNode(temp);
          newNode = mgr.merge(temp);
          
                //remove the transient node and add the
                //merged node
          script.removeNode(temp);
          script.addNode(newNode);
     }

I believe in hibernate this is not a problem, but in EJB3, it tends to be a problem.  Also, if anyone has a better solution, I would not mind hearing it :).
Dean

[gjeudy]

You are aware that merge does 2 things ? It makes your entity managed and also synchronize memory state with db state which can result in an UPDATE statement fired at the database at flush time. Since hibernate UPDATE all updateable properties it cannot allow to fire an UPDATE for a property that is null but is set to be non-nullable.

So to me it makes perfect sense.

Have you read Java Persistence with Hibernate book? Excellent read and should leave you less wondering on whats going on.

[gjeudy]

Just a hint, you might want to look at cascading relationships and see if that helps to solve your problem.

[deanhiller2000]

I forgot to mention....This is a long running application transaction...ie. FlushModeType=MANUAL so at the end I call a mgr.flush()....This means there are no database updates :) and so this code is still a problem.

yes, I have read the hibernate book.  In fact, I find myself getting really good at looking down in the hibernate 1st level cache to debug problems now and figuring out why my code doesn't work.  I don't see how anyone can use hibernate and debug it without fully understanding where the variables are for the 1st level cache(and they are buried pretty deep)....I think some people just hack and hack on it.  anyways...off my soap box.

later,
Dean

[gjeudy]

Dean,

ok good to know you've read the book.

Your problem explanation is a bit confusing to me, can you give the hibernate mappings of script, node and what contains the script.addNode() method ?

I assume we are dealing with a bidirectional relationship and you wire both ends inside script.addNode() ?

Do you use invert=true on one side of the relationship? I usually put invert=true on the side where you have the collection, so in your case it would be the script -> node relationship that you need to invert.

The inverted side (which is a very bad name to describe what it does) just means that this side of the relationship is ignored for the relationship management logic but it does not cancel any cascading attribute setup (I know this is confusing to me as well). But anyways I digress have you tried:

script.addNode(newNode);
  script=mgr.merge(script);

Setup a cascade="merge" property in script -> node relationship; this way the merge(script) should automatically merge the newNode as well.

[deanhiller2000]

I don't want to cascade this one as it could get into a loop of cascades.  yes, you got it right with the add.  I followed Gavin's pattern on addScript from his first book.  Here is the methods I have in ScriptModel.java...(notice get/set is private so client can't muck with List)....Node.java is below that. 

     @OneToMany(mappedBy="script", cascade=CascadeType.MERGE)
     private List<Node> getNodes() {
          if(nodes == null)
               nodes = new ArrayList<Node>();
          return nodes;
     }
     
     @SuppressWarnings("all")
     private void setNodes(List<Node> c) {
          this.nodes = c;
     }
     
     @Transient
     public List<Node> getImmutableNodes() {
          //we don't want users modifying the collection.  They should
          //call addRequirement or removeRequirement instead.  This is read-only
          return Collections.unmodifiableList(getNodes());
     }
     
     public void addNode(Node childProject) {
          if(childProject == null)
               throw new IllegalArgumentException("childProject may not be null and was");
          else if(childProject.getScript() != null) {
               //make sure the old parent removes this childProject(after all, this
            //is one to many, not many to many!!!!
               childProject.getScript().removeNode(childProject);
          }
          
          childProject.setScript(this);
          getNodes().add(childProject);
     }
     
     public void removeNode(Node childProject) {
          if(childProject == null)
               throw new IllegalArgumentException("requirement may not be null and was");
          
          childProject.setScript(null);
          getNodes().remove(childProject);
     }     

Here is Node.java(I thought invertable was only for the xml files.  I could not find where I could do that with annotations..

     @ManyToOne
     @JoinColumn(nullable=false)
     protected ScriptModel getScript() {
          return script;
     }
     protected void setScript(ScriptModel scriptModel) {
          this.script = scriptModel;
     }

so, is there a better solution then the one we are using when
1. we don't want to cascade
2. we need to make sure we end up with a script that has a node that has an id so entityConverter does not fail.

thanks,
Dean

[deanhiller2000]

heh heh, I guess I got confused.  We had cascade=merge already on this one(I have to be careful as I am dealing with a map(not java map), but map in the nodes to mutliple nodes sense and am trying to be careful of not cascading to the point it is circular.

ALSO, big note, we had alady done script = mgr.merge(script) earlier so the script is NOT transient in our use case while the node is transient and we are trying to attach it.

[deanhiller2000]

hmmmmm, so if I have already done mgr.merge(script), can I do that again every time I add a node then?  I need the first mgr.merge(script) as that is the first line in the editScript(ScriptModel script) method such that we start getting data from the script to display the current script, then I can just keep calling mgr.merge(script) instead of mgr.merge(node).

is that true?

very interesting.
dean

[deanhiller2000]

SWEET, that worked, BUT HYPOTHETICALLY, what if I could not do a cascade.  What then?  Is there a better solution than what I was doing?

thanks,
Dean

[deanhiller2000]

oops, I am not sure if this will be an issue, but this behavior is interesting....

@In(required=false)
@Out
private Node newNode;

@In
@Out
private ScriptModel script;

public void save() {
   script.add(newNode);
   script = mgr.merge(script);
}

NOTE that the script.getAllNodes().get(0).getId() has an id now BUT newNode.getId() is null so newNode is still transient and we are outjecting it to be used....it looks like my solution is the only way to go if I need that newNode later...the two lins of code above are not that good...they don't set the id of the newNode and associate it with the EntityManager.

[gjeudy]

I don't see why you fear having a circular cascade.. I think hibernate can handle such circular cascade setup.

In any case I'm just saying if you merge(script) and have cascade="merge" your nodes will always be synced with database including new instances with no ids persisted or at least the id eagerly initialized.

mappedBy is the semantic equivalent of invert="true"

[gjeudy]

just to comment on your last post. Your code will work if you keep the cascade="merge". Didnt you have this in your last test (code sample) ?

[deanhiller2000]

I think then that the code has to be this, correct????....

script.add(newNode);
script = mgr.merge(script);
int lastOne = script.geNodes().size();
newNode = script.getNodes().get(lastOne-1);
   //This replaces the newNode which has no id with the one that 
   // has an id.

We just ran into a case where newNode was being used in the next page and you don't want to be using the transient instance when there is a non-transient one already in the cache....that would be bad...very bad.  This is kind of ugly too....not much better than the previous solution, but a little better maybe.

[gjeudy]

ah I see what you mean. it just occurred to me, why are you using merge if your usecase is for node creation only ?

  script.addNode(newNode);
  mgr.persist(newNode);

wouldnt that work ? persist() will make your newNode instance managed and should have the id populated no need to retrieve last node from getNodes() anymore.

[deanhiller2000]

We have two uses cases.  One is for node creation only but the other is for creation and edits so we can use the wizard for add and edit and the add and edit, the variable is actually node, not newNode and that node ends up without an id which is bad, so we have to set the node to the non-transient instance right now....kinda ugly.