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

Object of class Closure could not be converted to int #121

Closed
phileon opened this issue Jan 22, 2018 · 10 comments
Closed

Object of class Closure could not be converted to int #121

phileon opened this issue Jan 22, 2018 · 10 comments

Comments

@phileon
Copy link

phileon commented Jan 22, 2018

Hi @baopham,

thank you for this great work!

I do not understand the meaning of this passage in the code:
https://github.com/baopham/laravel-dynamodb/blob/master/src/DynamoDbQueryBuilder.php#L851

every() needs an int param, so it comes to this notice:

Object of class Closure could not be converted to int in /..../vendor/laravel/framework/src/Illuminate/Support/Collection.php on line 250

Last lines of stack trace:

PHP 17. BaoPham\DynamoDb\DynamoDbQueryBuilder->find() /.../vendor/laravel/framework/src/Illuminate/Database/Eloquent/Model.php:3561

PHP 18. BaoPham\DynamoDb\DynamoDbQueryBuilder->isMultipleIds() /.../vendor/baopham/dynamodb/src/DynamoDbQueryBuilder.php:416

PHP 19. Illuminate\Support\Collection->every() /.../vendor/baopham/dynamodb/src/DynamoDbQueryBuilder.php:866

Best regards
Jan

@baopham
Copy link
Owner

baopham commented Jan 23, 2018

Hi Jan

Could you share the code that got this exception?

Thanks

@phileon
Copy link
Author

phileon commented Jan 23, 2018

Hi @baopham ,

I have put together the important parts for you. At the end, the desired result comes, nevertheless there is this notice message.

class DynamoDbModel extends \BaoPham\DynamoDb\DynamoDbModel
{
    const CREATED_AT = 'createdAt';
    const UPDATED_AT = 'updatedAt';

    /**
     * DynamoDbModel constructor.
     *
     * @param array $attributes
     *
     * @throws \Exception
     */
    public function __construct(array $attributes = [])
    {
        parent::__construct($attributes);

        $this->__set('systemHash', config('app.systemHash'));

        if (empty($this->primaryKey)) {
            throw new \Exception('primaryKey not set');
        }

        // set composite key
        $this->compositeKey = [$this->primaryKey, 'systemHash'];

        // expand table name by dev for local development
        if(config('isDeveloperSystem') == true) {
            $this->table .= '-dev';
        }
    }

    /**
     * Add global where for systemHash
     * Therefore, each table must have the model id as the primary key and plentyIdHash as sorting key
     *
     * @return DynamoDbQueryBuilder
     *
     * @throws \BaoPham\DynamoDb\NotSupportedException
     */
    public function newQuery()
    {
        $builder = new DynamoDbQueryBuilder($this);
        $builder->where('systemHash', $this->__get('systemHash'));

        foreach ($this->getGlobalScopes() as $identifier => $scope) {
            $builder->withGlobalScope($identifier, $scope);
        }

        return $builder;
    }
}

class Report extends DynamoDbModel
{
    /**
     * @internal
     * @var string
     */
    protected $table = 'report';

    /**
     * composite key is set by DynamoDbModel
     *
     * @internal
     * @var string
     */
    protected $primaryKey = 'id';

    /**
     * Indexes.
     * [
     *     '<simple_index_name>' => [
     *          'hash' => '<index_key>'
     *     ],
     *     '<composite_index_name>' => [
     *          'hash' => '<index_hash_key>',
     *          'range' => '<index_range_key>'
     *     ],
     * ]
     *
     * @internal
     * @var array
     */
    protected $dynamoDbIndexKeys = [
        'reportName-active-index' => [
            'hash' => 'reportName',
            'range' => 'active'
        ],
    ];

    /**
     * The attributes that are mass assignable.
     * @internal
     * @var array
     */
    protected $fillable = [
        'reportName',
        'reportType',
        'active',
        'ttl',
        'filterParams'
    ];

    /**
     * @return bool
     */
    public function isActive():bool
    {
        if($this->active=='Y') {
            return true;
        }

        return false;
    }
}

abstract from repository class, which causes the error:

public function getReport(string $reportId): Report
{
    $report = new Report();
    $result = $report->find($reportId);

    if($result instanceof Report) {
        return $result;
    } else {

        $this->getLogger(__METHOD__)->debug('module_report::debug.reportNotFound', ['id' => $reportId]);

        return $report;
    }
}

@baopham
Copy link
Owner

baopham commented Jan 23, 2018

Nice. Thanks for the details!
Could you show me the output of $report->getKeyNames()?

@phileon
Copy link
Author

phileon commented Jan 23, 2018

Thanks for your quick reply!

Array
(
    [0] => id
    [1] => systemHash
)

@phileon
Copy link
Author

phileon commented Jan 23, 2018

Maybe usefull

        $report = new Report();
        $result = $report->find($reportId);

        print_r($report->toArray());

Output

Array
(
    [systemHash] => 1000
    [id] => fb97f991-3a8a-564f-b39a-0ba1118fea74
)

@baopham
Copy link
Owner

baopham commented Jan 23, 2018

Ok, although I could not reproduce but there are a few things I don't get:

  1. You want to set a global where when a new query is created. But at that point, $this->__get('systemHash') may not be set, correct?
  2. Because your model is a composite model, find requires a full key, which means it should have been: $report->find(['id' => $reportId, 'systemHash' => <something>]).

Because find is supposed to give you the unique item based on the key, the id being passed to find needs to be a full key. If you don't have the full key, consider where and get instead.

@phileon
Copy link
Author

phileon commented Jan 23, 2018

Thanks, if I call find() with all keys, it comes to the same issue.

Look an this:

    protected function isMultipleIds($id)
    {
        $keys = collect($this->model->getKeyNames());

        // could be ['id' => 'foo'], ['id1' => 'foo', 'id2' => 'bar']
        $single = $keys->every(function ($name) use ($id) {
            return isset($id[$name]);
        });

        if ($single) {
            return false;
        }

        // could be ['foo', 'bar'], [['id1' => 'foo', 'id2' => 'bar'], ...]
        return $this->model->hasCompositeKey() ? is_array(array_first($id)) : is_array($id);
    }

The only problem is, that $keys->every() needs an int and gets a bool because of
return isset($id[$name]);

Therefore, this code is never executed (In any case, I never get to this point at debug):

function ($name) use ($id) {
            return isset($id[$name]);
        }

The result does not change for me: I get the desired result from dynamo. I just wanted to help improve the project.

@baopham
Copy link
Owner

baopham commented Jan 23, 2018

🤔 I think we may be looking at different versions of Collection because based on the docs, the return value should be bool

@phileon
Copy link
Author

phileon commented Jan 23, 2018

OMG

Currently we uses laravel 5.3.x: https://laravel.com/docs/5.3/collections#method-every
In laravel 5.5 it will work in your way.

Good to know! So please mention this in the readme and maybe check your versions in composer.json 😉

@baopham
Copy link
Owner

baopham commented Jan 23, 2018

Nice. Thanks for helping debug this. I'll probably switch to something else more compatible.

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

No branches or pull requests

2 participants