The implementation of the Cursor's current() method always returns the current->file, breaking queryBuilder api
Closed this issue · 8 comments
Hello,
I was trying to build a query to select all upload ids when I encountered strange behaviour.
The implementation of the Cursor's current() method always returns the current->file if current is an instance of MongoGridFSFile, then it makes a new GridFSFile of with the data in current.
This transformation does not respect the queryBuilder methods in the example below, and therefore I'm now unable to make a queryBuilder query that selects just the ids of all upload objects.
I'm wondering why this choice was made and if there is a solution to do what I want with the queryBuilder without changing the current() implementation or reverting to raw queries.
$queryBuilder = $this->createQueryBuilder();
return $queryBuilder
->hydrate(false)
->select('_id')
->getQuery()
->execute();
I'm now unable to make a queryBuilder query that selects just the ids of all upload objects.
How does the problem manifest itself? Is the resulting GridFSFile incomplete because the original query projected only the _id field?
I'm wondering why this choice was made and if there is a solution to do what I want with the queryBuilder without changing the current() implementation or reverting to raw queries.
I don't recall why this design decision was made, but I expect it was done in the interest of user convenience. I could see it conflict if the user already had a "file" field in their fs.files document; however, removing this functionality in a minor version would likely cause more headaches as a BC break.
Having looked at #253, I think a compromise would be to respect the query's projection before deciding to inject a GridFSFile object to the file property. If the file field is excluded either directly or indirectly (i.e. by only selecting other fields), we can skip the behavior. Would that work for you?
@0xPr0xy could you explain what issues arise from this? I created the following test:
public function testGridFSProjection()
{
$gridFS = $this->conn->selectDatabase(self::$dbName)->getGridFS();
$document = array('foo' => 'bar');
$gridFS->storeFile(__FILE__, $document);
$cursor = $gridFS->createQueryBuilder()
->select('_id')
->getQuery()
->execute();
$item = $cursor->getSingleResult();
}
When running the query with a projection, I get the following item:
array(2) {
'_id' =>
class MongoId#42 (1) {
public $$id =>
string(24) "56eb9c19e78a69d2c40041aa"
}
'file' =>
class Doctrine\MongoDB\GridFSFile#41 (4) {
private $mongoGridFSFile =>
class MongoGridFSFile#43 (3) {
public $file =>
array(1) {
...
}
protected $gridfs =>
class MongoGridFS#36 (5) {
...
}
public $flags =>
int(0)
}
private $filename =>
NULL
private $bytes =>
NULL
private $isDirty =>
bool(false)
}
}
Without any projection, the result is quite similar:
array(8) {
'_id' =>
class MongoId#42 (1) {
public $$id =>
string(24) "56eb9c19e78a69d2c40041aa"
}
'foo' =>
string(3) "bar"
'filename' =>
string(70) "/tmp/testFile.php"
'uploadDate' =>
class MongoDate#41 (2) {
public $sec =>
int(1458281497)
public $usec =>
int(884000)
}
'length' =>
int(13775)
'chunkSize' =>
int(261120)
'md5' =>
string(32) "e1f72ed8d389646c9d58e4d493b4bf7e"
'file' =>
class Doctrine\MongoDB\GridFSFile#47 (4) {
private $mongoGridFSFile =>
class MongoGridFSFile#43 (3) {
public $file =>
array(7) {
...
}
protected $gridfs =>
class MongoGridFS#36 (5) {
...
}
public $flags =>
int(0)
}
private $filename =>
NULL
private $bytes =>
NULL
private $isDirty =>
bool(false)
}
}
Either way, the result is an array with all properties that are included in the projection, plus an extra "file" property that contains a Doctrine\MongoDB\GridFSFile instance that can be consumed further. Isn't that what you would expect?
@alcaeus Your tests demonstrate the problem I'm having:
The query builder api documentation states the following:
You can limit the fields that are returned in the results by using the select() method:
In the results only the data from the username and password will be returned.
$qb = $dm->createQueryBuilder('User')
->select('username', 'password');
$query = $qb->getQuery();
$users = $query->execute();
This is not working correctly since the file property can't be excluded from the result set.
I would expect that if I only select the _id field, I will not get the file field.
I have tested whether or not hydrate(false) and exclude('file') will work, but even with select('_id') hydrate(false) and exlude('file) I am unable to NOT get back the file field.
I hope I explained the issue clear enough.
@0xPr0xy ah, I see. Apart from being different than stated in the documentation, does this cause problems for you? Just trying to figure out whether this blocks the upcoming releases or if it can wait a bit ;)
@jmikola is there any way we can detect projections in the cursor without having access to the query? The file array could contain any number of fields, so we can't exactly check for the presence of fields to detect a projection.
@alcaeus No it does not present a blocking issue, since I can simply iterate the result and unset the file index. Which then results in the expected projection, (just the ids). This however does decrease performance and that is not desired.
is there any way we can detect projections in the cursor without having access to the query? The file array could contain any number of fields, so we can't exactly check for the presence of fields to detect a projection.
@alcaeus: AFAIK, the projection (which is taken in the constructor) is only exposed via MongoCursior::info().
I took a crack at fixing this, there is just one small problem. As silly as it sounds, removing the file field from the result array at this time constitutes a BC break: people might have come to expect that the file field would always be present, even when limiting the other fields returned by the query. I'm not sure fixing this is worth the BC break, especially considering that this library will most likely die along with the old MongoDB driver.