Doctrine 1
  1. Doctrine 1
  2. DC-74

Condition of STATE_PROXY should be checked dynamically, not as a static state value

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.1.4
    • Fix Version/s: 1.2.0-BETA1
    • Component/s: Record
    • Labels:
      None

      Description

      This issue is continuation of issue #2488 from Trac. I have prepared Test case showing that the problem is seriously. Below original content.

      A record gets STATE_PROXY state at hydration when its properties are not fully loaded. Here is a condition (placed in Doctrine_Record::hydrate()):

      if ( ! $this->isModified() && count($this->_values) < $this->_table->getColumnCount()) {
         $this->_state = self::STATE_PROXY;
      }
      
      

      But at first setting a column value (whatever if the column value was loaded or not), the record gets STATE_DIRTY and "forgets" its proxy state.

      In effect a call to any not loaded column of the record causes returning null value. It should, of course, load remaining column values and change record state from STATE_PROXY to STATE_DIRTY. It doesn't load because current record state is already STATE_DIRTY.

      I'm attaching a patch, which fixes Doctrine_Record::load() in the described situation.

      In my opinion usage of STATE_PROXY must be rethinked. STATE_PROXY can mean:
      1. STATE_CLEAN + not fully loaded columns (proxy)
      2. STATE_DIRTY + not fully loaded columns (proxy)

      Currently it only points the first situation so there is not way to identify the second situation (and it is silently ignored).

      1. DC74.patch
        6 kB
        Jacek Dębowczyk
      2. DC74-1.2.diff
        6 kB
        Jacek Dębowczyk
      3. DC74TestCase.php
        3 kB
        Jacek Dębowczyk

        Activity

        Hide
        Jacek Dębowczyk added a comment -

        I'm including a patch.

        I think that the patch needs a little explanation. To solve the problem I see 2 solutions:
        1. create new state, eg. STATE_PROXY_MODIFIED being activated when a proxy record gets modified
        2. transform existing conditions to check proxy state dynamically using new Doctrine_Record::isInProxyState() method

        I chosen the second as a less invasive way. A record is in proxy state when it has any not loaded properties - this must be found out by the method. But... currently there is no way to find this. Doctrine_Null object in $record->_data means that property is not loaded yet OR it has null value.
        In the patch I divided real null value (simply stored as null in _data) from unknown (not loaded) value (stored as Doctrine_Null).

        Show
        Jacek Dębowczyk added a comment - I'm including a patch. I think that the patch needs a little explanation. To solve the problem I see 2 solutions: 1. create new state, eg. STATE_PROXY_MODIFIED being activated when a proxy record gets modified 2. transform existing conditions to check proxy state dynamically using new Doctrine_Record::isInProxyState() method I chosen the second as a less invasive way. A record is in proxy state when it has any not loaded properties - this must be found out by the method. But... currently there is no way to find this. Doctrine_Null object in $record->_data means that property is not loaded yet OR it has null value. In the patch I divided real null value (simply stored as null in _data) from unknown (not loaded) value (stored as Doctrine_Null).
        Hide
        Jonathan H. Wage added a comment -

        When I tried this patch, it broke the test suite. Can you try your patch again?

        Show
        Jonathan H. Wage added a comment - When I tried this patch, it broke the test suite. Can you try your patch again?
        Hide
        Jacek Dębowczyk added a comment -

        It breaks 2 test cases.

        The first is Doctrine_Ticket_1449_TestCase. I have appended following code to prepareData() in the test:

                $document->Attachments->getFirst()->getTable()->clear();
                $document->getTable()->clear();
        

        and added "a.document_id" column to select:

                $document = Doctrine_Query::create()
                    ->select('d.id, d.name, a.id, a.document_id')
        

        and the test passes. IMHO problem is in the test, what I noticed in DC-73.

        The second is Doctrine_Relation_OrderBy_TestCase. Currently there are identical queries asserted for lazy load $user->ChildrenUsers and $user->ParentUser. IMHO second query in the test (line 93) is wrong. Could you check this?

        Show
        Jacek Dębowczyk added a comment - It breaks 2 test cases. The first is Doctrine_Ticket_1449_TestCase. I have appended following code to prepareData() in the test: $document->Attachments->getFirst()->getTable()->clear(); $document->getTable()->clear(); and added "a.document_id" column to select: $document = Doctrine_Query::create() ->select('d.id, d.name, a.id, a.document_id') and the test passes. IMHO problem is in the test, what I noticed in DC-73 . The second is Doctrine_Relation_OrderBy_TestCase. Currently there are identical queries asserted for lazy load $user->ChildrenUsers and $user->ParentUser. IMHO second query in the test (line 93) is wrong. Could you check this?
        Hide
        Jacek Dębowczyk added a comment -

        I'm including the patch adapted to Doctrine-1.2.

        Show
        Jacek Dębowczyk added a comment - I'm including the patch adapted to Doctrine-1.2.

          People

          • Assignee:
            Jonathan H. Wage
            Reporter:
            Jacek Dębowczyk
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: