castor
  1. castor
  2. CASTOR-2713 Refactoring SQLStatementStore
  3. CASTOR-2718

Refactor and shift the Storage of extended classes to the ClassMolder

    Details

    • Type: Sub-task Sub-task
    • Status: Closed Closed
    • Priority: Trivial Trivial
    • Resolution: Fixed
    • Affects Version/s: 1.3
    • Fix Version/s: 1.3.1
    • Component/s: JDO queries
    • Labels:
      None
    • Number of attachments :
      2
    1. patch-c2718-20090523.txt
      11 kB
      AHMAD HASSAN
    2. patch-C2718-20090523.txt
      11 kB
      Ralf Joachim

      Activity

      Hide
      AHMAD HASSAN added a comment - - edited

      The comment is for reviewing the logic. The following code is moved from SQLengine to ClassMolder. As you can see, in SQLEngine this code was implemented in a way that it stores the records recursively.

         // Must store record in parent table first.
              // All other dependents are stored independently.
              SQLEngine extended = _engine.getExtends();
              if (extended != null) {
                  // | quick and very dirty hack to try to make multiple class on the same table work
                  ClassDescriptor extDesc = extended.getDescriptor();
                  if (!new ClassDescriptorJDONature(extDesc).getTableName().equals(_mapTo)) {
                      extended.store(conn, identity, newentity, oldentity);
                  }
              }

      New code which I written in ClassMolder is different. We cannot use the recursive mechanism in ClassMolder because it is at different layer. The new code looks like

          // Stores record in all the tables hierarchy       
              Connection conn = tx.getConnection(oid.getMolder().getLockEngine());
              _persistence.store(conn, oid.getIdentity(), newentity, oldentity);
      
              ClassMolder molder = this; 
              ClassMolder extendedMolder = molder._extends;
              
              while (extendedMolder != null) { 
                  ClassDescriptor extendedDescriptor = extendedMolder.getClassDescriptor();
                  ClassDescriptor descriptor = molder.getClassDescriptor();
                  if (!new ClassDescriptorJDONature(extendedDescriptor).getTableName().equals(
                          new ClassDescriptorJDONature(descriptor).getTableName())) {
                      extendedMolder._persistence.store(conn, oid.getIdentity(), newentity, oldentity);
                  }
               
                  molder = extendedMolder;
                  extendedMolder = extendedMolder._extends;
              }

      Comments?

      Best Regards, Ahmad

      Show
      AHMAD HASSAN added a comment - - edited The comment is for reviewing the logic. The following code is moved from SQLengine to ClassMolder. As you can see, in SQLEngine this code was implemented in a way that it stores the records recursively. // Must store record in parent table first. // All other dependents are stored independently. SQLEngine extended = _engine.getExtends(); if (extended != null ) { // | quick and very dirty hack to try to make multiple class on the same table work ClassDescriptor extDesc = extended.getDescriptor(); if (! new ClassDescriptorJDONature(extDesc).getTableName().equals(_mapTo)) { extended.store(conn, identity, newentity, oldentity); } } New code which I written in ClassMolder is different. We cannot use the recursive mechanism in ClassMolder because it is at different layer. The new code looks like // Stores record in all the tables hierarchy Connection conn = tx.getConnection(oid.getMolder().getLockEngine()); _persistence.store(conn, oid.getIdentity(), newentity, oldentity); ClassMolder molder = this ; ClassMolder extendedMolder = molder._extends; while (extendedMolder != null ) { ClassDescriptor extendedDescriptor = extendedMolder.getClassDescriptor(); ClassDescriptor descriptor = molder.getClassDescriptor(); if (! new ClassDescriptorJDONature(extendedDescriptor).getTableName().equals( new ClassDescriptorJDONature(descriptor).getTableName())) { extendedMolder._persistence.store(conn, oid.getIdentity(), newentity, oldentity); } molder = extendedMolder; extendedMolder = extendedMolder._extends; } Comments? Best Regards, Ahmad
      Hide
      Ralf Joachim added a comment -

      Using a loop to walk the extends chain is a good replacement for the recursion.

      Show
      Ralf Joachim added a comment - Using a loop to walk the extends chain is a good replacement for the recursion.
      Hide
      AHMAD HASSAN added a comment -

      A much improved version which I think will be more efficient.

      //ClassMolder Code
      
        //Stores record in all the tables hierarchy by looping over extended hierarchy
              //Gets Connection reference
              Connection conn = tx.getConnection(oid.getMolder().getLockEngine());
              //Gets the reference of extended ClassMolder
              ClassMolder extendedMolder = this._extends;
              //Gets the table name through ClassDescriptorJDONature
              String tableName = new ClassDescriptorJDONature(
                      this.getClassDescriptor()).getTableName();
              //Gets the table name through ClassDescriptorJDONature of extended ClassMolder
              String extendedTableName;
              
              _persistence.store(conn, oid.getIdentity(), newentity, oldentity);
              while (extendedMolder != null) {
                  extendedTableName = new ClassDescriptorJDONature(
                          extendedMolder.getClassDescriptor()).getTableName();
                  if (!extendedTableName.equals(tableName)) {
                      extendedMolder._persistence.store(conn, oid.getIdentity(), newentity, oldentity);
                  }
               
                  tableName = extendedTableName;
                  extendedMolder = extendedMolder._extends;
              }
      
      
      Show
      AHMAD HASSAN added a comment - A much improved version which I think will be more efficient. //ClassMolder Code //Stores record in all the tables hierarchy by looping over extended hierarchy //Gets Connection reference Connection conn = tx.getConnection(oid.getMolder().getLockEngine()); //Gets the reference of extended ClassMolder ClassMolder extendedMolder = this ._extends; //Gets the table name through ClassDescriptorJDONature String tableName = new ClassDescriptorJDONature( this .getClassDescriptor()).getTableName(); //Gets the table name through ClassDescriptorJDONature of extended ClassMolder String extendedTableName; _persistence.store(conn, oid.getIdentity(), newentity, oldentity); while (extendedMolder != null ) { extendedTableName = new ClassDescriptorJDONature( extendedMolder.getClassDescriptor()).getTableName(); if (!extendedTableName.equals(tableName)) { extendedMolder._persistence.store(conn, oid.getIdentity(), newentity, oldentity); } tableName = extendedTableName; extendedMolder = extendedMolder._extends; }
      Hide
      AHMAD HASSAN added a comment -

      Patch for Review. Due to lack of exact specific information for some of the attributes, I left some of the javadoc comments deliberately.

      Regards, Ahmad

      Show
      AHMAD HASSAN added a comment - Patch for Review. Due to lack of exact specific information for some of the attributes, I left some of the javadoc comments deliberately. Regards, Ahmad
      Hide
      Ralf Joachim added a comment -

      Based on Ahmad's patch to loop over extends hierarchy I found a possibility to further simplify code of ClassMolder. Patch of SQLSatetmentStore is contained without any change.

      Show
      Ralf Joachim added a comment - Based on Ahmad's patch to loop over extends hierarchy I found a possibility to further simplify code of ClassMolder. Patch of SQLSatetmentStore is contained without any change.

        People

        • Assignee:
          AHMAD HASSAN
          Reporter:
          AHMAD HASSAN
        • Votes:
          0 Vote for this issue
          Watchers:
          0 Start watching this issue

          Dates

          • Created:
            Updated:
            Resolved: