ManyToMany relationship validator not validating

[migueldemoura]

Hey,

I've been tinkering with the ManyToMany relationship and stumbled upon a problem:

It seems that since I'm using an array, the validator doesn't work, i.e., no exception is thrown when inserting invalid values in the courses.

Now, I'm not sure I have the "glue" Entity properly implemented, or if this is the expected behavior, but I think it makes sense to have this check.

Trace:

$config = new Spot\Config();
$config->addConnection('mysql', [
    'driver'   => $settings['driver'],
    'host'     => $settings['host'],
    'dbname'   => $settings['database'],
    'user'     => $settings['username'],
    'password' => $settings['password'],
    'charset'  => $settings['charset']
    ]);
$database = new Spot\Locator($config);

$courseMapper = $database->mapper('Entities\Course');
$proposalMapper = $database->mapper('Entities\Proposal');

$course = $courseMapper->create([
    'name' => 'Something'
]); // I will assume this gets the id 1

$proposal = $proposalMapper->build([]);
$proposal->relation('courses', [2]); // Should throw validation exception or
$proposalMapper->save($proposal); // Should throw validation exception

Source (simplified):

Proposal Entity

<?php
namespace Entities;

use Spot\Entity;
use Spot\MapperInterface;
use Spot\EntityInterface;

class Proposal extends Entity
{
    protected static $table = 'proposals';

    public static function fields()
    {
        return [
            'id' => ['type' => 'integer', 'autoincrement' => true, 'primary' => true],
        ];
    }

    public static function relations(MapperInterface $mapper, EntityInterface $entity)
    {
        return [
            'courses' => $mapper->hasManyThrough(
                $entity,
                'Entities\Course',
                'Entities\ProposalCourse',
                'course_id',
                'proposal_id'
            )
        ];
    }
}

Course Entity

<?php
namespace Entities;

use Spot\Entity;

class Course extends Entity
{
    protected static $table = 'courses';

    public static function fields()
    {
        return [
            'id'       => ['type' => 'integer', 'autoincrement' => true, 'primary' => true],
            'name' => ['type' => 'text'],
        ];
    }
}

ProposalCourse Entity

<?php
namespace Entities;

use Spot\Entity;

class ProposalCourse extends Entity
{
    protected static $table = 'proposalscourses';

    public static function fields()
    {
        return [
            'id'                 => ['type' => 'integer', 'autoincrement' => true, 'primary' => true],
            'proposal_id' => ['type' => 'integer'],
            'course_id'    => ['type' => 'integer'],
        ];
    }
}

Edit: added replication code. Edit2: fix typo in course id number. Edit3: add missing 'course_id' to ProposalCourse and remove field 'courses' from Proposal. Create relation using the relation method.

[tuupola]

Example code which triggers the bug?

[migueldemoura]

@tuupola, my bad, forgot to paste that in. Edited the issue description!

[tuupola]

I did not test the code, but you are defining courses field to be an array:

    public static function fields()
    {
        return [
            'id'           => ['type' => 'integer', 'autoincrement' => true, 'primary' => true],
            'courses' => ['type' => 'array'],
        ];
    }

You are also passing it an array:

    $proposal = $proposalMapper->create([
        'courses' => [1]
    ]);

So my logic says there should not be validation error.

However you seem also to be defining a relation named courses. I am bit surprised Spot did not throw an exception on this. Having a field and relation with same name is not the best idea. Rename or remove the courses field and see what happens.

[migueldemoura]

The error I was expecting has to do with the referenced course not existing, not with the var type (which is correct). Is this out of spot's scope?

Also, 'courses' => [1] should read 'courses' => [2], 2 being a non-existing course id (edited the description).

I'll change the relationship name to something else as well.

[nebulousGirl]

Here is what I see:

You are missing a course_id field in your ProposalCourse entity to complete the HasManyThroughRelation. You should not add a field courses in your Proposal entity. It can conflict with the relation name. To add a course to a proposal you need to instantiate a proposal and explicitly set the relation by doing the following: $proposal->relation('courses', [$course]);

[migueldemoura]

@nebulousGirl, thanks the response.

I've followed your instructions for this case and nothing is saved now - the through entity is empty. Is that entity properly defined? Not only that but the validator doesn't get triggered on invalid input (non existing ids). I've updated the sample code.

To add a course to a proposal you need to instantiate a proposal and explicitly set the relation by doing the following: $proposal->relation('courses', [$course]);

Is this recommended for the other relations?

[tuupola]

Not only that but the validator doesn't get triggered on invalid input (non existing ids).

Is it somewhere documented that validator should check for the id's? IIRC it does not.

[tuupola]

@nebulousGirl Did not even know $proposal->relation('courses', [$course]) is possible. Does it also set the foreign id's in ProposalCourse?

What I tend to do is create the join table entry manually. Something like:

$spot->mapper("Spot\Entity\ProposalCourse")->create([
     "course_id" => $course->id
     "proposal_id" => $proposal->id
]);

[migueldemoura]

@tuupola

Not only that but the validator doesn't get triggered on invalid input (non existing ids).

Is it somewhere documented that validator should check for the id's? IIRC it does not.

You're correct, there's no documentation stating such feature. But I expected it to work like that since inserting invalid ids would cause an exception by itself database-wise (foreign id not existing).

The issue seems to be that either my through entity or the instantiation is incorrect.

@nebulousGirl, am I correct?

[migueldemoura]

Any update on this?

[tuupola]

Quickly looking your code is wrong. You are trying to add an integer as relation when AFAIK you should be adding a Course entity as relation.

$proposal->relation('courses', [2]);

[migueldemoura]

Thanks for the help!

I changed that expression to $proposal->relation('courses', [$course]); and still get the same result - the proposalscourses table is empty. Do I need to create the entries manually?

[migueldemoura]

I peeked around the code and found the reason: you need to pass ['relations' => true] to the $options param for the mapper to save the relations. An exception is also thrown when passing invalid data, as expected.

Are PR's being accepted? I can improve the docs for this type of relation.

[nebulousGirl]

SpotORM used to be solely managed by @vlucas. Now that the project has an organisation, I think it is valid to expect PRs to be accepted faster, especially for documentation since it does not affect the stability of the package.