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

3.x matching() overwrites contain() #5584

Closed
davidyell opened this issue Jan 6, 2015 · 19 comments
Closed

3.x matching() overwrites contain() #5584

davidyell opened this issue Jan 6, 2015 · 19 comments
Assignees
Milestone

Comments

@davidyell
Copy link
Contributor

Scenario
A recent update to the core seems to have broken some of my finds. The affect being to me at least that using matching() whilst also having a contain() in your find will cause only the tables in matching() to be returned in the query, and not the ones in contain().

TL;DR
Here are two examples of my find, with an extra matching() call and without.
https://gist.github.com/davidyell/6b3d029815accca7197c
https://gist.github.com/davidyell/962a5615037385cfeeb3

I also have another find which suffers from the same issue.

So if you contain Posts and Tags but then using matching('Posts') the return will be Posts.tags = null, unless you add another matching('Posts.Tags')

@markstory markstory added this to the 3.0.0 milestone Jan 6, 2015
@markstory markstory self-assigned this Jan 6, 2015
@markstory
Copy link
Member

I can take a look at this.

@davidyell
Copy link
Contributor Author

Cool, thanks Mark. @lorenzo said he might also take a look. Just a note that the only code difference in the Gists is on ln 33.

@markstory
Copy link
Member

Is it not also a problem that all you containments for Innings.Matches.* were dropped?

@markstory
Copy link
Member

I tried to add a failing test case for this, but it passed..

    /**
     * Checks that matching and contain can be called for nested associations
     *
     * @see https://github.com/cakephp/cakephp/issues/5584
     * @return void
     */
    public function testFindMatchingAndContainOverwrite()
    {
        $authors = TableRegistry::get('Authors');
        $authors->hasMany('Articles');

        $articles = TableRegistry::get('Articles');
        $articles->belongsToMany('Tags');
        $articles->hasMany('Comments');

        $query = $authors->find()
            ->contain(['Articles' => ['Comments', 'Tags']])
            ->matching('Articles.Tags', function ($q) {
                return $q->where(['Tags.name' => 'tag1']);
            });
        $result = $query->first();

        $this->assertNotEmpty($result->articles[0]->tags);
        $this->assertNotEmpty($result->articles[0]->comments);
    }

Both the articles.comments and articles.tags associations were populated correctly. Is there something I'm missing?

@davidyell
Copy link
Contributor Author

Yes, you're missing the count() and group() which could also be causing problems perhaps? It might just be that my find needs to be updated, however I have not changed the find code. It worked in 1c4aa96 but when I updated my associations dropped.

If the actual context helps in any way, I'm calling this custom finder from a Cell.
https://github.com/davidyell/Cricketeer/blob/develop/src/View/Cell/StatisticsCell.php#L23-L27
https://github.com/davidyell/Cricketeer/blob/develop/src/Model/Table/BowlersTable.php#L92-L124

Could it be to do with autoFields(true)?

@markstory
Copy link
Member

I tried updating the test case code to be

        $authors = TableRegistry::get('Authors');
        $authors->hasMany('Articles');

        $articles = TableRegistry::get('Articles');
        $articles->belongsToMany('Tags');
        $articles->hasMany('Comments');

        $query = $authors->find();
        $query->contain(['Articles' => ['Comments', 'Tags']])
            ->matching('Articles.Tags', function ($q) {
                return $q->where(['Tags.name' => 'tag1']);
            })
            ->select(['total' => $query->func()->count('*')])
            ->order(['Articles.id' => 'DESC'])
            ->autoFields(true);
        $result = $query->first();

        $this->assertNotEmpty($result->articles[0]->tags);
        $this->assertNotEmpty($result->articles[0]->comments);

And it still passed. Do you have any behaviors, mapReduce, result formatters or beforeFind that could be altering the query results?

@davidyell
Copy link
Contributor Author

You didn't include a group() does that matter? Could I be cutting the data out of my result set that way?

MapReduce, Not that I'm aware of, I certainly haven't made any. It's just a Cell which calls that find. Although I'm not using first() as it should be returning a collection of items.

The enquiries stemmed from an error being thrown in my view for timeAgoInWords() as unknown method on ln33 of my bowlers.ctp, because the entity wasn't present in the data. I'm not doing anything funky in my view before display.

However the data pasted in the gists from the OP are directly dumped before rendering the view.

What would be the recommendation from here? Should I try refactoring/rewriting the find code? It took quite a while to get working the first time around. Or perhaps try writing a test case myself using some imported data?

@ndm2
Copy link
Contributor

ndm2 commented Jan 8, 2015

Maybe this has something to do with the type of associations, I stumbled over something similar with belongsTo ones:

$comments = TableRegistry::get('Comments');
$comments->belongsTo('Articles');

$articles = TableRegistry::get('Articles');
$articles->belongsToMany('Tags');

$result = $comments
    ->find()
    ->matching('Articles.Tags', function($q) {
        return $q->where(['tags.id' => 2]);
    })
    ->contain('Articles')
    ->first();

$this->assertEquals(1, $result->id);
$this->assertEquals(1, $result->_matchingData['Articles']->id);
$this->assertEquals(2, $result->_matchingData['Tags']->id);
$this->assertNotNull($result->article);

This is basically the same as in QueryTest::testMatchingWithContain(), it's just that the association with articles is belongsTo instead of hasMany. According to the matching data everything is found as expected, but $result->article is null.

@davidyell
Copy link
Contributor Author

@ndm2 Yes, that's what I am experiencing.

I'll show my Table relationships if that helps.
Bowlers belongsTo Players
Bowlers belongsTo Innings
Innings hasMany Matches
Matches belongsTo Formats
Matches belongsTo Venues
Innings hasMany Wickets
Wickets belongsTo Players as PlayerBowledWicket

I have an ERD if that helps visualise it.

@markstory
Copy link
Member

Thanks @ndm, I'll take a look.

@lorenzo
Copy link
Member

lorenzo commented Jan 15, 2015

Closing as the PR that solved this problem was merged

@lorenzo lorenzo closed this as completed Jan 15, 2015
@davidyell
Copy link
Contributor Author

Great, thanks for the update. I will revert my finds and do some testing tomorrow. Cheers @lorenzo

@davidyell
Copy link
Contributor Author

I've reset my code to when I opened this ticket and am still hitting the same issue. I guess I need to refactor my find.

https://github.com/davidyell/Cricketeer/blob/develop/src/Model/Table/BowlersTable.php#L92-L124

@davidyell
Copy link
Contributor Author

Right, so I've made my own test for this and it seems to be working. So I guess I have an error in my application code somewhere.

@nilsonpena
Copy link

@davidyell did you find any error in your code? I'm experiencing the same error here, but I'm using cake 3.4.7

@Oracizan
Copy link

Oracizan commented Aug 3, 2018

I don't comment often on Github issues, so please excuse me if this is not the correct place to write this.

I think that this issue is still present (as of 3.5.4).

If I only use where(), matching(), and contain() in a query, the data is present under both _matchingData and the entity property. If I use select() in a query (i.e. selecting fields from both the association and the primary table), however, only _matchingData will be set.

I have not had time to create a test case, but the general code to reproduce the issue would be along the lines of this:

Works

$user = $this->Users->find()
    ->contain('UserEmails')
    ->matching('UserEmails', function ($q) {
        return $q->where(['email' => 'alice@example.com']);
    })
    ->first();

Fails

$user = $this->Users->find()
    ->contain('UserEmails')
    ->select(['Users.id', 'UserEmails.id'])
    ->matching('UserEmails', function ($q) {
        return $q->where(['email' => 'alice@example.com']);
    })
    ->first();

@davidyell
Copy link
Contributor Author

davidyell commented Aug 3, 2018 via email

@Oracizan
Copy link

Oracizan commented Aug 3, 2018

My mistake - though I did not include it in above code snippet, I do actually select the matched field in my code. The entity property (user_emails in the above case) is still not set.

I will try to put together a test case to better illustrate this. Thank you for the quick reply.

@Oracizan
Copy link

Oracizan commented Aug 3, 2018

I cannot reproduce the issue with a more general test case, so it must be something on my end. Apologies, and thanks once more for your time.

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

No branches or pull requests

6 participants