HHH-2350 : second-level cache broken for non-inverse bidirectional one-to-many relation

git-svn-id: https://svn.jboss.org/repos/hibernate/core/trunk@20013 1b8cb986-b30d-0410-93ca-fae66ebed9b2

core/src/main/java/org/hibernate/type/ManyToOneType.java

@@ -88,10 +88,11 @@ public class ManyToOneType extends EntityType {
 	}
 
 	public boolean isAlwaysDirtyChecked() {
-		// If we have <tt>not-found="ignore"</tt> association mapped to a
+		// always need to dirty-check, even when non-updateable;
-		// formula, we always need to dirty check it, so we can update the
+		// this ensures that when the association is updated,
-		// second-level cache
+		// the entity containing this association will be updated
-		return ignoreNotFound;
+		// in the cache
+		return true;
 	}
 
 	public boolean isOneToOne() {

testsuite/src/test/java/org/hibernate/test/onetomany/AbstractRecursiveBidirectionalOneToManyTest.java

@@ -27,6 +27,7 @@ package org.hibernate.test.onetomany;
 import java.util.ArrayList;
 
 import org.hibernate.CacheMode;
+import org.hibernate.Hibernate;
 import org.hibernate.Session;
 import org.hibernate.Transaction;
 import org.hibernate.testing.junit.functional.FunctionalTestCase;
@@ -65,36 +66,43 @@ public abstract class AbstractRecursiveBidirectionalOneToManyTest extends Functi
 		return new String[] { "onetomany/Node.hbm.xml" };
 	}	
 
-    public void testOneToManyMoveElement() {
+	public void testOneToManyMoveElement() {
-        init();
+		init();
-        transformMove();
+		transformMove();
-        check();
+		check( false );
 		delete();
-    }
+	}
+
+	public void testOneToManyMoveElementWithDirtySimpleProperty() {
+		init();
+		transformMoveWithDirtySimpleProperty();
+		check( true );
+		delete();
+	}
 
 	public void testOneToManyReplaceList() {
 		init();
 		transformReplace();
-		check();
+		check( false );
 		delete();
 	}
 
-    void init() {
+	void init() {
 
 		Session s = openSession();
 		Transaction tx = s.beginTransaction();
 
-	    Node node1 = new Node( 1 );
+		Node node1 = new Node( 1, "node1" );
-	    Node node2 = new Node( 2 );
+		Node node2 = new Node( 2, "node2" );
-		Node node3 = new Node( 3 );
+		Node node3 = new Node( 3, "node3" );
 
-	    node1.addSubNode( node2 );
+		node1.addSubNode( node2 );
-	    node2.addSubNode( node3 );
+		node2.addSubNode( node3 );
 
-	    s.save(node1);
+		s.save(node1);
 
 		tx.commit();
-        s.close();
+		s.close();
 	}
 
 	void transformMove() {
@@ -113,6 +121,23 @@ public abstract class AbstractRecursiveBidirectionalOneToManyTest extends Functi
 		s.close();
 	}
 
+	void transformMoveWithDirtySimpleProperty() {
+
+		Session s = openSession();
+		Transaction tx = s.beginTransaction();
+
+		Node node3 = (Node) s.load(Node.class, new Integer(3));
+		Node node2 = node3.getParentNode();
+		Node node1 = node2.getParentNode();
+
+		node2.removeSubNode( node3 );
+		node1.addSubNode( node3 );
+		node3.setDescription( "node3-updated" );
+
+		tx.commit();
+		s.close();
+	}
+
 	void transformReplace() {
 
 		Session s = openSession();
@@ -131,21 +156,32 @@ public abstract class AbstractRecursiveBidirectionalOneToManyTest extends Functi
 		s.close();
 	}
 
-	void check() {
+	void check(boolean simplePropertyUpdated) {
-
 		Session s = openSession();
 		Transaction tx = s.beginTransaction();
-		Node node3 = (Node) s.get(Node.class, new Integer(3));
+		Node node3 = (Node) s.get( Node.class, new Integer(3) );
 
 		// fails with 2nd level cache enabled
-		assertEquals(1, node3.getParentNode().getId().intValue());
+		assertEquals( 1, node3.getParentNode().getId().intValue() );
+		assertEquals( ( simplePropertyUpdated ? "node3-updated" : "node3" ), node3.getDescription() );
+		assertTrue( node3.getSubNodes().isEmpty() );
+
+		Node node1 = node3.getParentNode();
+		assertNull( node1.getParentNode() );
+		assertEquals( 2, node1.getSubNodes().size() );
+		assertEquals( 2, ( ( Node ) node1.getSubNodes().get( 0 ) ).getId().intValue() );
+		assertEquals( "node1", node1.getDescription() );
+
+		Node node2 = ( Node ) node1.getSubNodes().get( 0 );
+		assertSame( node1, node2.getParentNode() );
+		assertTrue( node2.getSubNodes().isEmpty() );
+		assertEquals( "node2", node2.getDescription() );
 
 		tx.commit();
 		s.close();
 	}
 
 	void delete() {
-
 		Session s = openSession();
 		Transaction tx = s.beginTransaction();
 		Node node1 = ( Node ) s.get(  Node.class, new Integer( 1 ) );

testsuite/src/test/java/org/hibernate/test/onetomany/AbstractVersionedRecursiveBidirectionalOneToManyTest.java

@@ -0,0 +1,75 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * Copyright (c) 2010, Red Hat Middleware LLC or third-party contributors as
+ * indicated by the @author tags or express copyright attribution
+ * statements applied by the authors.  All third-party contributions are
+ * distributed under license by Red Hat Middleware LLC.
+ *
+ * This copyrighted material is made available to anyone wishing to use, modify,
+ * copy, or redistribute it subject to the terms and conditions of the GNU
+ * Lesser General Public License, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this distribution; if not, write to:
+ * Free Software Foundation, Inc.
+ * 51 Franklin Street, Fifth Floor
+ * Boston, MA  02110-1301  USA
+ *
+ */
+package org.hibernate.test.onetomany;
+
+import java.util.ArrayList;
+
+import org.hibernate.CacheMode;
+import org.hibernate.Hibernate;
+import org.hibernate.Session;
+import org.hibernate.Transaction;
+import org.hibernate.testing.junit.functional.FunctionalTestCase;
+import org.hibernate.test.readonly.VersionedNode;
+
+/**
+ * @author Burkhard Graves, Gail Badner
+ */
+
+public abstract class AbstractVersionedRecursiveBidirectionalOneToManyTest extends AbstractRecursiveBidirectionalOneToManyTest {
+
+    /*
+	 *  What is done:
+	 *    ___                   ___
+	 *   |   |                 |   |
+	 *    -> 1                  -> 1
+	 *       |   -transform->     / \
+	 *       2                   2   3
+	 *       |
+	 *     	 3
+	 *
+	 */
+
+	public AbstractVersionedRecursiveBidirectionalOneToManyTest(String str) {
+		super(str);
+	}
+
+	public String[] getMappings() {
+		return new String[] { "onetomany/VersionedNode.hbm.xml" };
+	}
+
+	void check(boolean simplePropertyUpdated) {
+		super.check( simplePropertyUpdated );
+		Session s = openSession();
+		Transaction tx = s.beginTransaction();
+		Node node1 = ( Node ) s.get( Node.class, new Integer( 1 ) );
+		Node node2 = ( Node ) s.get( Node.class, new Integer( 2 ) );
+		Node node3 = ( Node ) s.get( Node.class, new Integer( 3 ) );
+		assertEquals( 1, node1.getVersion() );
+		assertEquals( 1, node2.getVersion() );
+		assertEquals( 1, node3.getVersion() );
+		tx.commit();
+		s.close();
+	}
+}

testsuite/src/test/java/org/hibernate/test/onetomany/Node.hbm.xml

@@ -8,6 +8,7 @@
         <id name="id" column="id" type="java.lang.Integer">
             <!--<generator class="native"/> -->
         </id>
+        <property name="description"/>
         <many-to-one name="parentNode"
                      class="Node"
                      column="node_id"
@@ -15,7 +16,6 @@
                      not-null="false"
                      insert="false"
                      update="false"
-
                      />
         <list name="subNodes" cascade="all">
             <key column="node_id"/>

testsuite/src/test/java/org/hibernate/test/onetomany/Node.java

@@ -31,15 +31,17 @@ import java.util.ArrayList;
 public class Node implements Serializable {
 
 	private Integer id;
-
+	private long version;
 	private Node parentNode;
+	private String description;
 	private List subNodes = new ArrayList();
 
 	public Node() {
 	}
 
-	public Node(int id) {
+	public Node(int id, String description) {
 		setId( id );
+		setDescription( description );
 	}
 
     public Integer getId() {
@@ -50,6 +52,22 @@ public class Node implements Serializable {
 		this.id = id;
 	}
 
+	public long getVersion() {
+		return version;
+	}
+
+	public void setVersion(long version) {
+		this.version = version;
+	}
+
+	public String getDescription() {
+		return description;
+	}
+
+	public void setDescription(String description) {
+		this.description = description;
+	}
+
 	public Node getParentNode() {
 		return parentNode;
 	}

testsuite/src/test/java/org/hibernate/test/onetomany/RecursiveBidirectionalOneToManyCacheTest.java

@@ -61,24 +61,4 @@ public class RecursiveBidirectionalOneToManyCacheTest extends AbstractRecursiveB
 	public static Test suite() {
 		return new FunctionalTestClassTestSuite( RecursiveBidirectionalOneToManyCacheTest.class );
 	}
-
-    public void testOneToManyMoveElement() {
-		reportSkip( "non-inverse one-to-many known to fail with 2nd-level cache", "cache support");
-    }
-
-	// HHH-2350
-	public void testOneToManyMoveElementFailureExpected() {
-		super.testOneToManyMoveElement();
-	}
-
-	public void testOneToManyReplaceList() {
-		reportSkip( "non-inverse one-to-many known to fail with 2nd-level cache", "cache support");
-	}
-
-	// HHH-2350
-	public void testOneToManyReplaceListFailureExpected() {
-		super.testOneToManyReplaceList();
-	}
-
-
 }

testsuite/src/test/java/org/hibernate/test/onetomany/RecursiveVersionedBidirectionalOneToManyCacheTest.java

@@ -0,0 +1,64 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * Copyright (c) 2010, Red Hat Middleware LLC or third-party contributors as
+ * indicated by the @author tags or express copyright attribution
+ * statements applied by the authors.  All third-party contributions are
+ * distributed under license by Red Hat Middleware LLC.
+ *
+ * This copyrighted material is made available to anyone wishing to use, modify,
+ * copy, or redistribute it subject to the terms and conditions of the GNU
+ * Lesser General Public License, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this distribution; if not, write to:
+ * Free Software Foundation, Inc.
+ * 51 Franklin Street, Fifth Floor
+ * Boston, MA  02110-1301  USA
+ *
+ */
+package org.hibernate.test.onetomany;
+
+import junit.framework.Test;
+
+import org.hibernate.CacheMode;
+import org.hibernate.testing.junit.functional.FunctionalTestClassTestSuite;
+
+/**
+ * @author Burkhard Graves, Gail Badner
+ */
+
+public class RecursiveVersionedBidirectionalOneToManyCacheTest extends AbstractVersionedRecursiveBidirectionalOneToManyTest {
+
+    /*
+	 *  What is done:
+	 *    ___                   ___
+	 *   |   |                 |   |
+	 *    -> 1                  -> 1
+	 *       |   -transform->     / \
+	 *       2                   2   3
+	 *       |
+	 *     	 3
+	 *
+	 * Commenting out
+	 * @Cache(usage=CacheConcurrencyStrategy.READ_WRITE)
+	 * in Node.java makes the assertion true (in check() below).
+	 */
+
+	public RecursiveVersionedBidirectionalOneToManyCacheTest(String str) {
+		super(str);
+	}
+
+	protected CacheMode getSessionCacheMode() {
+			return CacheMode.NORMAL;
+	}
+
+	public static Test suite() {
+		return new FunctionalTestClassTestSuite( RecursiveVersionedBidirectionalOneToManyCacheTest.class );
+	}
+}

testsuite/src/test/java/org/hibernate/test/onetomany/RecursiveVersionedBidirectionalOneToManyNoCacheTest.java

@@ -0,0 +1,68 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * Copyright (c) 2010, Red Hat Middleware LLC or third-party contributors as
+ * indicated by the @author tags or express copyright attribution
+ * statements applied by the authors.  All third-party contributions are
+ * distributed under license by Red Hat Middleware LLC.
+ *
+ * This copyrighted material is made available to anyone wishing to use, modify,
+ * copy, or redistribute it subject to the terms and conditions of the GNU
+ * Lesser General Public License, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY
+ * or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public License
+ * for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this distribution; if not, write to:
+ * Free Software Foundation, Inc.
+ * 51 Franklin Street, Fifth Floor
+ * Boston, MA  02110-1301  USA
+ *
+ */
+package org.hibernate.test.onetomany;
+
+import junit.framework.Test;
+
+import org.hibernate.CacheMode;
+import org.hibernate.testing.junit.functional.FunctionalTestClassTestSuite;
+
+/**
+ * @author Burkhard Graves, Gail Badner
+ */
+
+public class RecursiveVersionedBidirectionalOneToManyNoCacheTest extends AbstractVersionedRecursiveBidirectionalOneToManyTest {
+
+    /*
+	 *  What is done:
+	 *    ___                   ___
+	 *   |   |                 |   |
+	 *    -> 1                  -> 1
+	 *       |   -transform->     / \
+	 *       2                   2   3
+	 *       |
+	 *     	 3
+	 *
+	 * Commenting out
+	 * @Cache(usage=CacheConcurrencyStrategy.READ_WRITE)
+	 * in Node.java makes the assertion true (in check() below).
+	 */
+
+	public RecursiveVersionedBidirectionalOneToManyNoCacheTest(String str) {
+		super(str);
+	}
+
+	public String getCacheConcurrencyStrategy() {
+			return null;
+	}
+
+	protected CacheMode getSessionCacheMode() {
+			return CacheMode.IGNORE;
+	}
+
+	public static Test suite() {
+		return new FunctionalTestClassTestSuite( RecursiveVersionedBidirectionalOneToManyNoCacheTest.class );
+	}
+}

testsuite/src/test/java/org/hibernate/test/onetomany/VersionedNode.hbm.xml

@@ -0,0 +1,27 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!DOCTYPE hibernate-mapping PUBLIC "-//Hibernate/Hibernate Mapping DTD 3.0//EN" "http://hibernate.sourceforge.net/hibernate-mapping-3.0.dtd">
+
+<hibernate-mapping package="org.hibernate.test.onetomany">
+    <class name="Node"
+           table="Node">
+        <cache usage="read-write" region="Node.entities"/>
+        <id name="id" column="id" type="java.lang.Integer">
+            <!--<generator class="native"/> -->
+        </id>
+        <version name="version" column="vers" type="long" />
+        <property name="description"/>
+        <many-to-one name="parentNode"
+                     class="Node"
+                     column="node_id"
+                     lazy="proxy"
+                     not-null="false"
+                     insert="false"
+                     update="false"
+                     />
+        <list name="subNodes" cascade="all">
+            <key column="node_id"/>
+            <list-index column="idx"/>
+            <one-to-many class="Node"/>
+        </list>
+    </class>
+</hibernate-mapping>