nextras / orm

Orm with clean object design, smart relationship loading and powerful collections.
https://nextras.org/orm
MIT License
309 stars 59 forks source link

Entity: allow relationship defined only on one side #29

Closed hrach closed 8 years ago

hrach commented 10 years ago

I was thinking about this and came to the opinion, this would be probably wont-fix. It could bring many unexpected behavior, which would be hardly debuggable. See:

class Post {}
/**
 * @property Post $post {m:1 PostsRepository}
 */
class Comment {}

$this->post = ...
$comment = new Comment();
$comment->post = $this->post;

$this->orm->posts->persistAndFlush($this->post);
// comment is not stored...
// no exception, no error....

Also, it makes sense mainly for 1:M relationship on the m side.

Opinions?

JanTvrdik commented 10 years ago

+1 for now won't fix + possible reconsideration in the future

f3l1x commented 9 years ago

I know it's a won't fix. But, any idea? I think it could be great.

hrach commented 9 years ago

Idea about what? But you are right, let's reopen it and move it to 1.2 :)

f3l1x commented 9 years ago

I like Nextras\Orm but I think this one is only ugly thing.. :smiley:

We/you should fix this thing. What problems do you think could came across?

hrach commented 9 years ago

I have already mentioned one few comments above.

f3l1x commented 9 years ago
/**
 * @property Comment[] {1:m Comment}
 */
class Post {}

class Comment {}

Is it the way you suggest? Remove required relation in Comment, but optional?

hrach commented 9 years ago

No, on the other side, as in the exaple above. The comment wrongly mention the "M" side, but it should be the otherwise.

f3l1x commented 9 years ago

And how IDE suggest you comments in Post? Through {virtual}?

hrach commented 9 years ago

@f3l1x in no way. This is not a good example of one sided relationship. This is typical usecase when it doesn't make sense.

f3l1x commented 9 years ago

I see. I guess you may have chance defined relation on both side but in case of 1:m not required. Does it make sense? Or do you have any other idea?

hrach commented 9 years ago

Sorry, I don't understand what do you mean.

f3l1x commented 9 years ago

I have updated my previous comment.

vitkutny commented 8 years ago

@hrach exception při použití one sided relací

Nextras\Orm\InvalidModifierDefinitionException Modifier {1:1} in Ytnuk\Menu\Entity::$title property has unknown arguments: oneSided.

opraveno úpravou MetadataParser.php:307

            } else {
                $targetProperty = NULL;
                unset($args['oneSided']);
            }
hrach commented 8 years ago

Fixed. Thanks! I should write some tests... :)