Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Versionable Behaviour : bug with multiple identical related objects #876

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ruben-podadera
Copy link

Hello,

I think I might found a bug with the versionable behaviour. It happens when a versionable table links twice an other versionable table. I said I might because it could also be that I'm not using the versionable behaviour as I should. In fact I'm having 2 issues, as I will explain.

I have a lease contract, which is signed between two companies, a renter, and a tenant. I'm working within a symfony2 project. I have the following schema :

In one bundle, the company :

<?xml version="1.0" encoding="UTF-8"?>
<database name="default" namespace="Api\UserBundle\Propel" defaultIdMethod="native">
    <table name="company">

        <column name="id" type="integer" required="true" primaryKey="true" autoIncrement="true" />
        <column name="name" type="varchar" size="255" required="true" primaryString="true" />

        <behavior name="versionable"></behavior>
        <behavior name="timestampable"></behavior>
    </table>
</database>

In an other bundle, the contract :

<?xml version="1.0" encoding="UTF-8"?>
<database name="default" namespace="Api\ContractBundle\Propel" defaultIdMethod="native">
    <table name="contract">
        <column name="id" type="integer" required="true" primaryKey="true" autoIncrement="true" />

        <column name="renter_company_id" type="integer" required="true"/>
        <foreign-key foreignTable="company" phpName="RenterCompany" onDelete="cascade" onUpdate="cascade">
            <reference local="renter_company_id" foreign="id" />
        </foreign-key>

        <column name="tenant_company_id" type="integer" required="false"/>
        <foreign-key foreignTable="company" phpName="TenantCompany" onDelete="setnull" onUpdate="cascade">
            <reference local="tenant_company_id" foreign="id" />
        </foreign-key>

        <behavior name="versionable">
            <parameter name="log_created_at" value="true" />
        </behavior>
        <behavior name="timestampable"></behavior>
    </table>
</database>

This schema results in 4 tables : contract, contract_version, company and company_version.

First issue

$tenant_company = new Company();
$tenant_company->setName("TenantCompany");
$tenant_company->save();

$renter_company = new Company();
$renter_company->setName("RenterCompany");
$renter_company->save();

$contract = new Contract();
$contract->setRenterCompany($renter_company);
$contract->setTenantCompany($tenant_company);

$contract->enforceVersioning();
$contract->save();

$contract->toVersion(1);

Console :

PHP Fatal error:  Call to a member function getId() on a non-object in src/Api/UserBundle/Propel/om/BaseCompany.php on line 2478

The bug comes from the fact that there is no company version created. I have fixed it by overloading the enforceVersioning method in the Contract.php :

// Contract.php

...
public function enforceVersioning()
{
    // used to trigger the modified column status of propel
    $this->getRenterCompany()->setUpdatedAt(strtotime("yesterday"))->setUpdatedAt(time())->enforceVersioning();
    $this->getTenantCompany()->setUpdatedAt(strtotime("yesterday"))->setUpdatedAt(time())->enforceVersioning();

    return parent::enforceVersioning();
}
...

Ok, now it works. I assume that I'm doing something wrong in the way I'm doing my work for this point because I have read that the versionable behaviour handles itself the related tables. Obviously I have not found what so I'm doing this bugfix in my code. For the second bug, I really believe there is an issue with the behaviour, that's why I creating this pull request. The purpose og this pull request is not to fix this issue, bug the second one.

Second issue

$tenant_company = new Company();
$tenant_company->setName("TenantCompany");
$tenant_company->save();

$renter_company = new Company();
$renter_company->setName("RenterCompany");
$renter_company->save();

$contract = new Contract();
$contract->setRenterCompany($renter_company);
$contract->setTenantCompany($tenant_company);

$contract->enforceVersioning();
$contract->save();

$contract->enforceVersioning();
$contract->save();

$contract->toVersion(2);

echo "Tenant Company : " . $contract->getTenantCompany()->getName() . "\n";
echo "Renter Company : " . $contract->getRenterCompany()->getName() . "\n";

Console :

Tenant Company : TenantCompany
Renter Company : TenantCompany

This is the content of company_version table after the code execution :

id renter_company_id tenant_company_id version renter_company_id_version tenant_company_id_version
1 2 1 2 2 2

The bug is in the populateFromVersion of BaseCompany.

The request made to retreive related contract_versions where the company belongs as the tenant is made as so :

SELECT *  FROM `contract_version` WHERE (contract_version.id=1 AND contract_version.version=2)

So we have one contract.

The request made to retreive related contract_versions where the company belongs as the renter is made as so :

SELECT * FROM `contract_version` WHERE (contract_version.id=1 AND contract_version.version=2)

So we have one contract.

Both requests are identical, and they sould not.

With this pull request, here are the requests generated in populateFromVersion :

as a renter :

SELECT * FROM `contract_version` WHERE (contract_version.id=1 AND contract_version.version=2) AND contract_version.renter_company_id=1

as a tenant :

SELECT * FROM `contract_version` WHERE (contract_version.id=1 AND contract_version.version=2) AND contract_version.tenant_company_id=1

So the Company is either found as a tenant company or a renter company, but not for both. The output is now :

Tenant Company : TenantCompany
Renter Company : RenterCompany

@ruben-podadera
Copy link
Author

Anyone ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant