Hello,

I'm sending the patch I cooked up in attachment. I hope you approve of
it. All the tests pass (at least all the ones that passed before the
patch was applied).

Basically, what this patch does is to lookup the concrete class before
materializing the corresponding object. This lookup avoids needing to
materialize each column in the method

   DescriptorRepository.getFieldDescriptorsForMultiMappedTable()

This is in my opinion a cleaner solution for materializing objects
(which avoids the issues I previously stated). However, when generating
the sql statements, all the fields must be known, so the original
behavior must be maintained for the SqlSelectStatement classes. 

Cheers,
Luis Cruz


On Sat, 2004-08-21 at 12:21, Armin Waibel wrote:
> Hi Luis,
>
> I think this method returns all fields to support interfaces and
> abstract classes too. But I agree you are right, if the ClassDescritor
> belongs to a "normal" class only the associated fields should be
> returned. But I'm not familiar with this stuff (side-effects?), so did
> you write a patched version of DescriptorRepository? Do all junit
> tests pass?
> 
> regards,
> Armin
>
> On Wed, 2004-08-18 at 16:50, Luis Cruz wrote:
> > Hello,
> > 
> > We are working with OJB 1.0 release and we found a bug a few weeks ago.
> > Only now have we found time to determine the origin of the bug so here
> > goes.
> > 
> > In the DescriptorRepository class, in particular, in the method:
> > 
> >    getFieldDescriptorsForMultiMappedTable
> > 
> > the following snip of code is used to obtain the field descriptors of
> > the target class descriptor:
> > 
> >    synchronized (m_multiMappedTableMap)
> >    {
> >       retval =
> > getAllMappedColumns(getClassesMappedToSameTable(targetCld));
> >       m_multiMappedTableMap.put(targetCld.getClassNameOfObject(),
> > retval);
> >    }
> > 
> > Basically this code is retrieving all the field descriptors of all the
> > class descriptors mapped to the same table. This is a bug is it not? If
> > I'm missing something I would greatly appreciate an explanation.
> > 
> > As a result of this behavior, OJB latter materializes all the columns of
> > the table for objects which are mapped on the same table, even columns
> > that are not mapped to the object being materialized. A more detailed
> > description of the collateral damage induced by this behavior can be
> > found in a previous mail I submitted; the subject of the mail was:
> > 
> >    Possible bug Mapping All Classes on the Same Table
> > 
> > I think a simple fix for this bug can be simply getting all mapped
> > columns for the target class descriptor instead of getting all mapped
> > columns for all the classes mapped to the same table, but I'll leave
> > this up to you experts.
> > 
> > Best regards,
> > Luis Cruz

Index: DescriptorRepository.java
===================================================================
RCS file: /home/cvspublic/db-ojb/src/java/org/apache/ojb/broker/metadata/DescriptorRepository.java,v
retrieving revision 1.50.2.2
diff -u -r1.50.2.2 DescriptorRepository.java
--- DescriptorRepository.java	4 Aug 2004 00:31:51 -0000	1.50.2.2
+++ DescriptorRepository.java	23 Aug 2004 10:20:37 -0000
@@ -17,6 +17,7 @@
 
 import java.io.Serializable;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.Iterator;
@@ -178,11 +179,33 @@
     }
 
     /**
+     * @return field descriptors for a class.
+     */
+    public FieldDescriptor[] getFieldDescriptorsForMultiMappedTable(ClassDescriptor targetCld)
+    {
+        if(m_multiMappedTableMap == null)
+        {
+            m_multiMappedTableMap = new HashMap();
+        }
+        FieldDescriptor[] retval = (FieldDescriptor[]) m_multiMappedTableMap.get(targetCld.getClassNameOfObject());
+        if (retval == null)
+        {
+            synchronized (m_multiMappedTableMap)
+            {
+                ClassDescriptor[] clds = new ClassDescriptor[] {targetCld};
+                retval = getAllMappedColumns(Arrays.asList(clds));
+                m_multiMappedTableMap.put(targetCld.getClassNameOfObject(), retval);
+            }
+        }
+        return retval;
+    }
+
+    /**
      * @return all field descriptors for a class that belongs to a set of classes mapped
      * to the same table, otherwise the select queries produced won't contain the necessary
      * information to materialize extents mapped to the same class.
      */
-    public FieldDescriptor[] getFieldDescriptorsForMultiMappedTable(ClassDescriptor targetCld)
+    public FieldDescriptor[] getAllFieldDescriptorsForMultiMappedTable(ClassDescriptor targetCld)
     {
         if(m_multiMappedTableMap == null)
         {
Index: RowReaderDefaultImpl.java
===================================================================
RCS file: /home/cvspublic/db-ojb/src/java/org/apache/ojb/broker/accesslayer/RowReaderDefaultImpl.java,v
retrieving revision 1.30.2.1
diff -u -r1.30.2.1 RowReaderDefaultImpl.java
--- RowReaderDefaultImpl.java	9 Aug 2004 07:51:26 -0000	1.30.2.1
+++ RowReaderDefaultImpl.java	23 Aug 2004 10:21:18 -0000
@@ -24,6 +24,7 @@
 import org.apache.ojb.broker.PersistenceBrokerException;
 import org.apache.ojb.broker.metadata.ClassDescriptor;
 import org.apache.ojb.broker.metadata.FieldDescriptor;
+import org.apache.ojb.broker.metadata.JdbcType;
 import org.apache.ojb.broker.util.ClassHelper;
 import org.apache.ojb.broker.util.logging.LoggerFactory;
 
@@ -171,7 +172,23 @@
         }
         else
         {
-            fields = m_cld.getRepository().getFieldDescriptorsForMultiMappedTable(m_cld);
+            FieldDescriptor concreteClassFD = m_cld.getOjbConcreteClassField();
+            ClassDescriptor classDescriptor = m_cld;
+            if (concreteClassFD != null)
+            {
+            	try
+				{
+            		JdbcType jdbcType = concreteClassFD.getJdbcType();
+            		String val = (String) jdbcType.getObjectFromColumn(rs, concreteClassFD.getColumnName());
+            		classDescriptor = m_cld.getRepository().getDescriptorFor(val);
+				}
+            	catch (SQLException e)
+				{
+            		classDescriptor = m_cld;
+				}
+            }
+
+        	fields = m_cld.getRepository().getFieldDescriptorsForMultiMappedTable(classDescriptor);
         }
         readValuesFrom(rs, row, fields);
     }
Index: SqlSelectByPkStatement.java
===================================================================
RCS file: /home/cvspublic/db-ojb/src/java/org/apache/ojb/broker/accesslayer/sql/SqlSelectByPkStatement.java,v
retrieving revision 1.8
diff -u -r1.8 SqlSelectByPkStatement.java
--- SqlSelectByPkStatement.java	4 Apr 2004 23:53:32 -0000	1.8
+++ SqlSelectByPkStatement.java	23 Aug 2004 10:22:37 -0000
@@ -58,7 +58,7 @@
      */
 	protected List appendListOfColumnsForSelect(ClassDescriptor cld, StringBuffer buf)
     {
-        FieldDescriptor[] fieldDescriptors = cld.getRepository().getFieldDescriptorsForMultiMappedTable(cld);
+        FieldDescriptor[] fieldDescriptors = cld.getRepository().getAllFieldDescriptorsForMultiMappedTable(cld);
         ArrayList columnList = new ArrayList();
 
         if (fieldDescriptors != null)
Index: SqlSelectStatement.java
===================================================================
RCS file: /home/cvspublic/db-ojb/src/java/org/apache/ojb/broker/accesslayer/sql/SqlSelectStatement.java,v
retrieving revision 1.22.2.1
diff -u -r1.22.2.1 SqlSelectStatement.java
--- SqlSelectStatement.java	26 Jul 2004 15:49:51 -0000	1.22.2.1
+++ SqlSelectStatement.java	23 Aug 2004 10:21:51 -0000
@@ -80,7 +80,7 @@
      */
     protected List appendListOfColumnsForSelect(ClassDescriptor cld, StringBuffer buf)
     {
-        FieldDescriptor[] fieldDescriptors = cld.getRepository().getFieldDescriptorsForMultiMappedTable(cld);
+        FieldDescriptor[] fieldDescriptors = cld.getRepository().getAllFieldDescriptorsForMultiMappedTable(cld);
         int fieldDescriptorLength = fieldDescriptors.length;
         ArrayList columnList = new ArrayList();
         int i = 0;

---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]

Reply via email to