propelorm/Propel3

Referrers inconsistancy between Query and EntityMap

cristianoc72 opened this issue · 6 comments

In https://github.com/propelorm/Propel3/blob/master/src/Propel/Generator/Builder/Om/Component/EntityMap/BuildRelationsMethod.php#L45 the name of the referrer relation is plural.

In https://github.com/propelorm/Propel3/blob/master/src/Propel/Generator/Builder/Om/Component/Query/JoinMethods.php#L44 joins are built considering the name of the referrer relation as singular.

So writing

<?php
    AuthorQuery::create()->joinBook();

causes an exception thrown by EntityMap, because it looks for a book relation while it has a books relation.
Writing

<?php
    AuthorQuery::create()->joinBooks();

causes a Query exception because joinBooks method doesn't exists.

We should choose one between singular or plural.
Imho we should implement our decision in Propel\Generator\Builder\Om\Component\RelationTrait, by adding getRelationName and getReferrerName methods, to avoid similar issues for the future.

marcj commented

Semantically the method should be named "joinBooks" as it joins in one-many relation ("books" is the referrer relation). So actually I guess the error lies simply in the method https://github.com/propelorm/Propel3/blob/master/src/Propel/Generator/Builder/Om/Component/Query/JoinMethods.php#L44 :)

marcj commented

Also the property is named plural as it contains a list instead of a single object. We could however, change relation "name" (which is used in joins) to always being singular to avoid confusion. Then https://github.com/propelorm/Propel3/blob/master/src/Propel/Generator/Builder/Om/Component/EntityMap/BuildRelationsMethod.php#L45 this line is the issue. But I'm currently not sure whether this will break something as we use the plural name sometimes in the code (not only for the property) to get the relation (irrc).

I'm checking the second solution, because also in Propel2 the join method is "joinBook" (even if I'm agree with you: it should be named "joinBooks").
By now, it doesn't break anything, in agnostic tests.

I was wrong: the previous solution broke some formatter methods. The referrer relation name now is plural and the name of the join methods is plural, too.
E.g. AuthorQuery::create()->joinBooks().

marcj commented

nice :)

Close via #32