Doctrine 1

synchronizeWithArray does not properly set foreign key validation

Details

  • Type: Bug Bug
  • Status: Resolved Resolved
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: 1.2.2, 1.2.3
  • Fix Version/s: 1.2.3
  • Component/s: Relations, Validators
  • Labels:
    None

Description

I've discovered a variation of the problem tested in tests/Validator/ForeignKeysTestCase.php. This happens when you synchronizeWithArray and a foreign relation is set - isValid will trigger the isnull validator.

This is probably better explained through two new test cases. I've included them below. The first test case passes. However, the second test case (testSynchronizedForeignKeyIsValidIfForeignRelationIsSet) fails.

tests/Validator/ForeignKeysTestCase.php - also attached as patch
// Place in tests/Validator/ForeignKeysTestCase.php
    public function testSynchronizedForeignKeyIsValidIfLocalRelationIsSet()
    {
        $person = new TestPerson();
        $person->synchronizeWithArray(array('Addresses' => array(array())));

        $address = $person->Addresses[0];
        $table = $address->getTable();
        
        $errors = $table->validateField('person_id', $address->person_id, $address);
        $this->assertEqual(0, $errors->count());
    }

    public function testSynchronizedForeignKeyIsValidIfForeignRelationIsSet()
    {
        $address = new TestAddress();
        $address->synchronizeWithArray(array('Person' => array()));

        $table = $address->getTable();
        $errors = $table->validateField('person_id', $address->person_id, $address);
        $this->assertEqual(0, $errors->count());
    }

I've discovered a workaround, if you reassign the value it will work.

$address->synchronizeWithArray(array('Person' => array()));
$address->Person = $address->Person;

A quick and likely terrible (or wrong?) fix is to have the synchronizeWithArray function do it for you. I've attached a patch does just that.

Activity

Hide
Permalink
Jeff Chu added a comment -

Just as a note - I was looking at this further and noticed that doing this also fails:

public function testGetForeignKeyIsValidIfForeignRelationIsSet()
{
    $address = new TestAddress();
    $address->Person;

    $table = $address->getTable();
    $errors = $table->validateField('person_id', $address->person_id, $address);
    $this->assertEqual(0, $errors->count());
}

But oddly enough, the following works:

$address->Person;
$address->Person = $address->Person;

I think this has to do with the inconsistencies in whether get should create a real relation or fake it until it's actually set with a setter. From what I can tell, this all stems from the support for the following behavior:

$address = new Address();
$address->Person->first_name = "Bob";

This behavior is taken advantage of from within synchronizeWithArray:

$this->$key->synchronizeWithArray($value);

However, because it doesn't create a real relation this way - the original issue comes up. Updating record's get to create a real relation requires us to update Doctrine_Record's _get to use coreSetRelated (instead of directly modifying $this->_references). However, doing this will conflict directly with test Ticket 1072.

What is the intended behavior of all of this?

Show
Jeff Chu added a comment - Just as a note - I was looking at this further and noticed that doing this also fails: public function testGetForeignKeyIsValidIfForeignRelationIsSet() { $address = new TestAddress(); $address->Person; $table = $address->getTable(); $errors = $table->validateField('person_id', $address->person_id, $address); $ this ->assertEqual(0, $errors->count()); } But oddly enough, the following works: $address->Person; $address->Person = $address->Person; I think this has to do with the inconsistencies in whether get should create a real relation or fake it until it's actually set with a setter. From what I can tell, this all stems from the support for the following behavior: $address = new Address(); $address->Person->first_name = "Bob" ; This behavior is taken advantage of from within synchronizeWithArray: $ this ->$key->synchronizeWithArray($value); However, because it doesn't create a real relation this way - the original issue comes up. Updating record's get to create a real relation requires us to update Doctrine_Record's _get to use coreSetRelated (instead of directly modifying $this->_references). However, doing this will conflict directly with test Ticket 1072. What is the intended behavior of all of this?

People

  • Assignee:
    Jonathan H. Wage
    Reporter:
    Jeff Chu
Vote (0)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: