bosnadev / repository

Laravel Repositories is a package for Laravel 5 which is used to abstract the database layer. This makes applications much easier to maintain.
https://bosnadev.com
823 stars 233 forks source link

In a loop, repository always returns me the same record #39

Open ghost opened 9 years ago

ghost commented 9 years ago

Hi,

I'm facing a weird problem when fetching records from the DB using the find or findWhere methods.

$offersToInsert is an array of objects sent via AJAX.

[
    {
        id: 2,
        title: "red",
        type: "tag",
        inDB: 1
    },
    {
        id: null,
        title: "green",
        type: "tag",
        inDB: 0
    },
    {
        id: 3,
        title: "Simon's",
        type: "brand",
        inDB: 1
    }
];

For each offer in the array, if "inDB" is true, I retrieve the the corresponding tag from the DB. This is where the problem occurs:

$tag = $this->tagRepo->find($tagData['id']);

Although I've made sure the $tagData['id'] value is different on each iteration, this line always returns me the same record (the first one it retrieved when processing the first iteration in the loop).

But if I change the "find" method in my TagRepository from this:

/**
 * @param $id
 * @param array $columns
 * @return mixed
 */
public function find($id, $columns = array('*')) {
    $this->applyCriteria();
    $this->newQuery()->eagerLoadRelations();
    return $this->model->find($id, $columns);
}

to this (by removing the newQuery():

/**
 * @param $id
 * @param array $columns
 * @return mixed
 */
public function find($id, $columns = array('*')) {
    $this->applyCriteria();
    $this->eagerLoadRelations();
    return $this->model->find($id, $columns);
}

it works perfectly (even though it might create some effects somewhere ?)

If I directly call the model it works just fine but defeats the purpose of a repository.

$tag = Tag::find($tagData['id']);

Here is the (simplified) code I have in a Job class.

public function __construct($offersToInsert)
{
    $this->offersToInsert = $offersToInsert;
}

public function handle(TagRepository $tagRepo)
    {
        $this->tagRepo = $tagRepo;

        $insertedOffers = [];
        foreach ($this->offersToInsert as $key => $offer) {
            $tagsToBeAttached = [];

            foreach ($offer['tags'] as $key => $tagData) {
                if($tagData['inDB'] === 0) {
                    $existingTag = $this->tagRepo
                                        ->findWhere([
                                                        'title_fr' => $tagData['title'],
                                                        'title_en' => $tagData['title']
                                                    ],
                                                    ['id', 'counter'],
                                                    true
                                        )
                                        ->all();

                    if($existingTag) {
                        $existingTag[0]->increment('counter');
                        $tagsToBeAttached[] = $existingTag[0]->id;
                    }
                    else {
                        $newTag = [
                            'tag_type_id' => 1,
                            'title_en' => $tagData['title'],
                            'title_fr' => $tagData['title'],
                            'counter' => 1,
                        ];
                        $newTag = $this->tagRepo->create($newTag);
                        $tagsToBeAttached[] = $newTag->id;
                    }

                }
                else {
                    // Works if I remove the "newQuery()" call inside the "find" method of the repo.
                    $tag = $this->tagRepo->find($tagData['id']);

                    // Works just fine.
                    //$tag = Tag::find($tagData['id']);
                }
            }

        }

        // Return whatever...
    }

I'm not really familiar with the purpose of the newQuery method, but does someone see something wrong in my code ? Can I safely remove the newQuery() calls everywhere ?

Thanks a lot for your precious time !

ghost commented 9 years ago

Does anyone have a clue on this ?