OPENJPA-2557: FinderCache contains incorrectly cached query with a NULL for a Primary Key.

git-svn-id: https://svn.apache.org/repos/asf/openjpa/trunk@1662615 13f79535-47bb-0310-9956-ffa450edef68

openjpa-jdbc/src/main/java/org/apache/openjpa/jdbc/kernel/FinderQueryImpl.java

@@ -72,7 +72,17 @@ public class FinderQueryImpl
             return null;
         SQLBuffer buffer = impl.getSQL();
         Column[] pkCols = mapping.getPrimaryKeyColumns();
-        boolean canCache = pkCols.length == buffer.getParameters().size();
+
+        //OPENJPA-2557: Typically the number of pkCols (length) should match the number (size) of 
+        //parameters.  However, there are a few cases (e.g. when extra parameters are needed for
+        //discriminator data) where the pkCols length may be different than the parameters.   
+        //If we find the number of pkCols is equal to the number of parameters, we need to do
+        //one last check to verify that the buffers columns match the pkCols exactly.
+        boolean canCache = (pkCols.length == buffer.getParameters().size());
+        for(int i=0; i < pkCols.length  && canCache; i++){
+            canCache = canCache && buffer.getColumns().contains(pkCols[i]);            
+        }
+        
         return (canCache)
             ? new FinderQueryImpl(mapping, impl, buffer) : null;
     }

openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/sqlcache/discrim/AbstractExtValue.java

@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.    
+ */
+package org.apache.openjpa.persistence.jdbc.sqlcache.discrim;
+
+import java.io.Serializable;
+
+import javax.persistence.DiscriminatorColumn;
+import javax.persistence.DiscriminatorType;
+import javax.persistence.Entity;
+import javax.persistence.Id;
+
+@Entity
+@DiscriminatorColumn(discriminatorType = DiscriminatorType.INTEGER, name = "EXTDISCR")
+public class AbstractExtValue implements Serializable {
+
+    private static final long serialVersionUID = 7753311252101420833L;
+    
+    @Id
+	private String code;
+
+	public String getCode() {
+		return this.code;
+	}
+
+	public void setCode(String code) {
+		this.code = code;
+	}
+
+    @Override
+    public int hashCode() {
+        final int prime = 31;
+        int result = 1;
+        result = prime * result + ((code == null) ? 0 : code.hashCode());
+        return result;
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+        if (this == obj)
+            return true;
+        if (obj == null)
+            return false;
+        if (getClass() != obj.getClass())
+            return false;
+        AbstractExtValue other = (AbstractExtValue) obj;
+        if (code == null) {
+            if (other.code != null)
+                return false;
+        } else if (!code.equals(other.code))
+            return false;
+        return true;
+    }
+}

openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/sqlcache/discrim/ComposedPK.java

@@ -0,0 +1,88 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.    
+ */
+package org.apache.openjpa.persistence.jdbc.sqlcache.discrim;
+
+import java.io.Serializable;
+
+import javax.persistence.Embeddable;
+
+@Embeddable
+public class ComposedPK implements Serializable {
+    
+    private static final long serialVersionUID = -7415701271873221026L;
+
+    private Short field1;
+
+    private Integer field2;
+    
+    public ComposedPK(){}
+
+    public Short getField1() {
+        return field1;
+    }
+
+    public void setField1(Short field1) {
+        this.field1 = field1;
+    }
+
+    public Integer getField2() {
+        return field2;
+    }
+
+    public void setField2(Integer field2) {
+        this.field2 = field2;
+    }
+
+    public ComposedPK(Short field1, Integer field2) {
+        this.field1 = field1;
+        this.field2 = field2;
+    }
+
+    @Override
+    public int hashCode() {
+        final int prime = 31;
+        int result = 1;
+        result = prime * result + ((field1 == null) ? 0 : field1.hashCode());
+        result = prime * result + ((field2 == null) ? 0 : field2.hashCode());
+        return result;
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+        if (this == obj)
+            return true;
+        if (obj == null)
+            return false;
+        if (getClass() != obj.getClass())
+            return false;
+        ComposedPK other = (ComposedPK) obj;
+        if (field1 == null) {
+            if (other.field1 != null)
+                return false;
+        } else if (!field1.equals(other.field1))
+            return false;
+        if (field2 == null) {
+            if (other.field2 != null)
+                return false;
+        } else if (!field2.equals(other.field2))
+            return false;
+        return true;
+    }
+    
+}

openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/sqlcache/discrim/ExtValue1.java

@@ -0,0 +1,31 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.    
+ */
+package org.apache.openjpa.persistence.jdbc.sqlcache.discrim;
+
+import java.io.Serializable;
+
+import javax.persistence.DiscriminatorValue;
+import javax.persistence.Entity;
+
+@Entity
+@DiscriminatorValue("9")
+public class ExtValue1 extends AbstractExtValue implements Serializable {
+
+    private static final long serialVersionUID = -6800461627353149172L;
+}

openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/sqlcache/discrim/TestFinderCacheWithNulls.java

@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.    
+ */
+package org.apache.openjpa.persistence.jdbc.sqlcache.discrim;
+
+import javax.persistence.EntityManager;
+
+import org.apache.openjpa.jdbc.conf.JDBCConfiguration;
+import org.apache.openjpa.jdbc.meta.ClassMapping;
+import org.apache.openjpa.kernel.FetchConfiguration;
+import org.apache.openjpa.kernel.FinderCache;
+import org.apache.openjpa.persistence.test.SingleEMFTestCase;
+
+
+public class TestFinderCacheWithNulls extends SingleEMFTestCase {
+    private FetchConfiguration fetchCfg;
+    private FinderCache fndrCache;
+    private ClassMapping clsMapping_UserData;
+    private ClassMapping clsMapping_AbstractExtValue;
+
+    @Override
+    public void setUp() throws Exception {
+        super.setUp(AbstractExtValue.class, ComposedPK.class, ExtValue1.class, UserData.class);
+    }
+    
+    public void test() {
+
+        init();
+        initData();
+        
+        EntityManager em = this.emf.createEntityManager();
+        
+        assertNull(fndrCache.get(clsMapping_UserData, fetchCfg));
+        
+        UserData usrData=em.find(UserData.class, new ComposedPK(Short.valueOf("2"), null));
+        assertNull(usrData);
+        //FinderCache should be empty.  That is, since the previous find contained a NULL,
+        //the cache shouldn't not contain the finder SQL.  However, prior to OPENJPA-2557, 
+        //the finder cache contain the finder SQL with the NULL value.  With this 
+        //JIRA, the cache should not contain the finder.
+        assertNull(fndrCache.get(clsMapping_UserData, fetchCfg));
+        em.clear();
+        
+        usrData=em.find(UserData.class, new ComposedPK(Short.valueOf("2"), 3));       
+        //Prior to OPENJPA-2557, the UserData would not have been found because the previous 
+        //find with a NULL would have been cached.  
+        assertNotNull(usrData);
+        assertNull(fndrCache.get(clsMapping_UserData, fetchCfg));  
+        em.clear();
+
+        ExtValue1 ev1 = em.find(ExtValue1.class, "A");       
+        assertNotNull(ev1);
+        assertNotNull(fndrCache.get(clsMapping_AbstractExtValue, fetchCfg));
+        em.clear();
+        
+        fndrCache.invalidate(clsMapping_AbstractExtValue);
+        assertNull(fndrCache.get(clsMapping_AbstractExtValue, fetchCfg));
+
+        AbstractExtValue aev = em.find(AbstractExtValue.class, "A");       
+        assertNotNull(aev);
+        assertNotNull(fndrCache.get(clsMapping_AbstractExtValue, fetchCfg));
+
+        em.close();
+    }
+    
+    public void init(){
+        EntityManager em = emf.createEntityManager();
+        
+        JDBCConfiguration conf = (JDBCConfiguration) emf.getConfiguration();        
+        clsMapping_UserData = conf.getMappingRepositoryInstance().getMapping(UserData.class, null, true);
+        clsMapping_AbstractExtValue = conf.getMappingRepositoryInstance().
+                                           getMapping(AbstractExtValue.class, null, true);
+        
+        fetchCfg = ((org.apache.openjpa.persistence.EntityManagerImpl) em).getBroker().getFetchConfiguration();
+
+        fndrCache = ((JDBCConfiguration) emf.getConfiguration()).getFinderCacheInstance();
+        
+        em.close();
+    }
+
+    public void initData() {
+        EntityManager em = emf.createEntityManager();
+        
+        ExtValue1 extValue1 = new ExtValue1();
+        extValue1.setCode("A");
+        em.getTransaction().begin();
+        em.persist(extValue1);
+        em.flush();        
+        
+        ComposedPK pK = new ComposedPK((short) 2, 3);
+        UserData userData = new UserData();
+        userData.setPk(pK);
+        userData.setExtValue(extValue1);
+        em.persist(userData);
+        em.getTransaction().commit();
+        em.close();
+    }
+}

openjpa-persistence-jdbc/src/test/java/org/apache/openjpa/persistence/jdbc/sqlcache/discrim/UserData.java

@@ -0,0 +1,88 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.    
+ */
+package org.apache.openjpa.persistence.jdbc.sqlcache.discrim;
+
+import java.io.Serializable;
+
+import javax.persistence.EmbeddedId;
+import javax.persistence.Entity;
+import javax.persistence.JoinColumn;
+import javax.persistence.ManyToOne;
+import javax.persistence.Table;
+
+@Entity
+@Table(name = "TB1")
+public class UserData implements Serializable {
+
+    private static final long serialVersionUID = 158985341763605994L;
+
+    @EmbeddedId
+    private ComposedPK pk;
+
+    @ManyToOne
+    @JoinColumn(name = "EXT_USR", insertable = false, updatable = false)
+    private ExtValue1 extValue;
+
+    public ExtValue1 getExtValue() {
+        return extValue;
+    }
+
+    public void setExtValue(ExtValue1 val) {
+        this.extValue = val;
+    }
+
+    public ComposedPK getPk() {
+        return pk;
+    }
+
+    public void setPk(ComposedPK pk) {
+        this.pk = pk;
+    }
+
+    @Override
+    public int hashCode() {
+        final int prime = 31;
+        int result = 1;
+        result = prime * result + ((extValue == null) ? 0 : extValue.hashCode());
+        result = prime * result + ((pk == null) ? 0 : pk.hashCode());
+        return result;
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+        if (this == obj)
+            return true;
+        if (obj == null)
+            return false;
+        if (getClass() != obj.getClass())
+            return false;
+        UserData other = (UserData) obj;
+        if (extValue == null) {
+            if (other.extValue != null)
+                return false;
+        } else if (!extValue.equals(other.extValue))
+            return false;
+        if (pk == null) {
+            if (other.pk != null)
+                return false;
+        } else if (!pk.equals(other.pk))
+            return false;
+        return true;
+    }
+}