DocumentHandler -> update pass $document twice to updateById

[elaineli]

public function update(Document $document, array $options = [])
{
$documentId = $this->getDocumentId($document);
return $this->updateById($document, $documentId, $document, $options);
}

It seems the first arg to call to updateById should be $collection, and the update function does not accept $collection. Am I missing something?

[jsteemann]

I think it works as it is now, but you are right that it is misleading as it is and should be fixed.
I am using current devel for explanaition:

DocumentHandler.php (at the very bottom) has an update function, which passes $document as the first parameter to updateById as you say.

The first call parameter of updateById is indeed $collection, but it is not type-enforced. So the function will accept any type here. It will pass the value of $collection (which a document in our case) to the function patch as second parameter.

The second parameter of patch is also not typed, and patch will finally pass that value into a function named makeCollection, which is implemented in Handler.php.

makeCollection, at the very top of the screenshot, will accept inputs of class type Collection or Document. That's the reason why it works, even though a document is passed into update and not a collection.

I will leave this ticket open so it will be fixed.

[elaineli]

Thank you.
When i try calling update($document), i am getting this error.
ArangoDBClient\ServerException: collection must be given in URL path or query parameter 'collection' must be specified
Not sure why it is working as is.

Seems to me all the CRUD functions should require a $collection argument to be consistent. It will be great to get fixed.

[jsteemann]

I think right now an update attempt will fail for cases in which $document does not contain the collection id, e.g.:

$documentHandler = new DocumentHandler("test");
$document = Document::createFromArray([ "_key" => "testKey" ]);
$documentHandler->update($document);

This will fail with the exact same error message that you posted. This is because there is an attempt to pull out the collection name from the document, but there is none present.
Changing the calling code as follows (so it includes the "_id" values in the document) will fix it:

$documentHandler = new DocumentHandler("test");
$document = Document::createFromArray([ "_key" => "testKey", "_id" => "test/testKey" ]);
$documentHandler->update($document);

Yet another way is to call updateById directly using the collection name, as it is a public function:

$documentHandler = new DocumentHandler("test");
$document = Document::createFromArray([ "_key" => "testKey" ]);
$documentHandler->updateById("test", "testKey", $document);

Similar functions exist for replacing (replaceById) and removing (removeById) already. I think these should do what you would like to see.

The problem with changing the signature of DocumentHandler::update() or other crud methods that currently do not require a collection is that this would make them all downwards-imcompatible.