Skip to content

Commit 46228ef

Browse files
frankmayerfrankmayer
committed
Fix the document ids to correctly use ArangoDB's _ids instead of only the _key
This is a backwards incompatible change, but necessary to be consistent with ArangoDB's API. Important related changes to this: - Document::getId() : Will return the correct id (collectionName/DocumentId) instead of the key. - DocumentHandler::getById(): will work as before, but it will accept a "real" document ID in addition to the key. If a real document ID is given, the collection data will be extracted from that string. That means that the first parameter `$collection` does not need to have a valid value, in that case. - UrlHelper::getDocumentIdFromLocation() : Will now return a "real" _id instead of what was essentially the `_key` Fixed Tests to adapt to the new id handling.
1 parent 6d7ec42 commit 46228ef

12 files changed

+412
-324
lines changed

‎CHANGELOG.md

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,16 @@
11
Release notes for the ArangoDB-PHP driver 3.2.0 (currently in development)
22
==========================================================================
3+
- Document ID's are now handled correctly. This is a necessary and ___backwards incompatible change___, in order to be consistent with ArangoDB's API.
4+
5+
Why: It was necessary to fix the document ids to correctly use ArangoDB's _ids instead of only the _key.
6+
7+
__Important incompatible changes related to this:__
8+
- Document::getId(): Will return the correct id (CollectionName/DocumentID) instead of the key (DocumentID).
9+
- UrlHelper::getDocumentIdFromLocation(): Will now return a "real" _id instead of what was essentially the `_key`
10+
11+
__Other changes related to this:__
12+
- DocumentHandler::getById(): Will work as before, but it will also accept a "real" document ID in addition to the key.
13+
If a real document ID is given, the collection data will be extracted from that string. That means that the first parameter `$collection` does not need to have a valid value, in that case.
314

415
- The namespace `\triagens\ArangoDb` was replaced with `\ArangoDBClient`.
516
For each class in the old namespace there is now a class alias that points

‎lib/ArangoDBClient/Document.php

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -704,18 +704,18 @@ public function getHandle()
704704
}
705705

706706
/**
707-
* Get the document id (if already known)
707+
* Get the document id (or document handle) if already known.
708708
*
709-
* Document ids are generated on the server only. Document ids are numeric but might be
710-
* bigger than PHP_INT_MAX. To reliably store a document id elsewhere, a PHP string should be used
709+
* It is a string and consists of the collection's name and the document key (_key attribute) separated by /.
710+
* Example: (collectionname/documentId)
711711
*
712-
* @return mixed - document id, might be NULL if document does not yet have an id
712+
* The document handle is stored in a document's _id attribute.
713+
*
714+
* @return mixed - document id, might be NULL if document does not yet have an id.
713715
*/
714716
public function getId()
715717
{
716-
@list(, $documentId) = explode('/', $this->_id, 2);
717-
718-
return $documentId;
718+
return $this->_id;
719719
}
720720

721721
/**

‎lib/ArangoDBClient/DocumentHandler.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,9 @@ public function has($collection, $documentId)
121121
*/
122122
public function getById($collection, $documentId, array $options = [])
123123
{
124+
if (strpos($documentId, '/') !== false) {
125+
@list($collection ,$documentId) = explode('/', $documentId);
126+
}
124127
$data = $this->getDocument(Urls::URL_DOCUMENT, $collection, $documentId, $options);
125128
$options['_isNew'] = false;
126129

‎lib/ArangoDBClient/UrlHelper.php

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,17 +34,17 @@ public static function getDocumentIdFromLocation($location)
3434

3535
if (0 === strpos($location, '/_db/')) {
3636
// /_db/<dbname>/_api/document/<collection>/<key>
37-
@list(, , , , , , $id) = explode('/', $location);
37+
@list(, , , , , $collectionName, $id) = explode('/', $location);
3838
} else {
3939
// /_api/document/<collection>/<key>
40-
@list(, , , , $id) = explode('/', $location);
40+
@list(, , , $collectionName, $id) = explode('/', $location);
4141
}
4242

4343
if (is_string($id)) {
4444
$id = urldecode($id);
4545
}
4646

47-
return $id;
47+
return $collectionName . '/' . $id;
4848
}
4949

5050
/**
@@ -62,6 +62,10 @@ public static function buildUrl($baseUrl, array $parts = [])
6262
$url = $baseUrl;
6363

6464
foreach ($parts as $part) {
65+
if (strpos($part, '/') !== false) {
66+
@list(,$part) = explode('/', $part);
67+
}
68+
6569
$url .= '/' . urlencode($part);
6670
}
6771

‎tests/CollectionExtendedTest.php

Lines changed: 240 additions & 196 deletions
Large diffs are not rendered by default.

‎tests/CustomDocumentClassTest.php

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ class CustomDocumentClassTest extends
2323

2424
protected static $testsTimestamp;
2525

26+
protected $collection;
27+
protected $collectionHandler;
28+
2629
public function __construct($name = null, array $data = [], $dataName = '')
2730
{
2831
parent::__construct($name, $data, $dataName);
@@ -59,18 +62,18 @@ public function testGetCustomDocumentWithHandler()
5962

6063
$document->someAttribute = 'someValue';
6164

62-
$documentId = $documentHandler->save($collection->getId(), $document);
65+
$documentId = $documentHandler->save($collection->getName(), $document);
6366

6467
$documentHandler->setDocumentClass(CustomDocumentClass1::class);
65-
$resultingDocument1 = $documentHandler->get($collection->getId(), $documentId);
68+
$resultingDocument1 = $documentHandler->get($collection->getName(), $documentId);
6669
static::assertInstanceOf(CustomDocumentClass1::class, $resultingDocument1, 'Retrieved document isn\'t made with provided CustomDocumentClass1!');
6770

6871
$documentHandler->setDocumentClass(CustomDocumentClass2::class);
69-
$resultingDocument2 = $documentHandler->get($collection->getId(), $documentId);
72+
$resultingDocument2 = $documentHandler->get($collection->getName(), $documentId);
7073
static::assertInstanceOf(CustomDocumentClass2::class, $resultingDocument2, 'Retrieved document isn\'t made with provided CustomDocumentClass2!');
7174

7275
$documentHandler->setDocumentClass(Document::class);
73-
$resultingDocument = $documentHandler->get($collection->getId(), $documentId);
76+
$resultingDocument = $documentHandler->get($collection->getName(), $documentId);
7477
static::assertInstanceOf(Document::class, $resultingDocument, 'Retrieved document isn\'t made with provided Document!');
7578
static::assertNotInstanceOf(CustomDocumentClass1::class, $resultingDocument, 'Retrieved document is made with CustomDocumentClass1!');
7679
static::assertNotInstanceOf(CustomDocumentClass2::class, $resultingDocument, 'Retrieved document is made with CustomDocumentClass2!');
@@ -105,7 +108,7 @@ public function testGetCustomDocumentWithStatement()
105108

106109
$document->someAttribute = 'anotherValue';
107110

108-
$documentHandler->save($collection->getId(), $document);
111+
$documentHandler->save($collection->getName(), $document);
109112

110113
$statement = new Statement(
111114
$connection, [
@@ -141,7 +144,7 @@ public function testGetCustomDocumentWithExport()
141144

142145
$document->someAttribute = 'exportValue';
143146

144-
$documentHandler->save($collection->getId(), $document);
147+
$documentHandler->save($collection->getName(), $document);
145148

146149
$export = new Export($connection, $collection->getName(), [
147150
'batchSize' => 5000,
@@ -178,18 +181,18 @@ public function testGetCustomDocumentWithBatch()
178181
$document1 = Document::createFromArray(
179182
['someAttribute' => 'someValue', 'someOtherAttribute' => 'someOtherValue']
180183
);
181-
$docId1 = $documentHandler->save($this->collection->getId(), $document1);
184+
$docId1 = $documentHandler->save($this->collection->getName(), $document1);
182185
$document2 = Document::createFromArray(
183186
['someAttribute' => 'someValue2', 'someOtherAttribute' => 'someOtherValue2']
184187
);
185-
$docId2 = $documentHandler->save($this->collection->getId(), $document2);
188+
$docId2 = $documentHandler->save($this->collection->getName(), $document2);
186189

187190
$batch = new Batch($connection);
188191
$batch->setDocumentClass(CustomDocumentClass1::class);
189192
$batch->startCapture();
190193

191-
$documentHandler->getById($this->collection->getId(), $docId1);
192-
$documentHandler->getById($this->collection->getId(), $docId2);
194+
$documentHandler->getById($this->collection->getName(), $docId1);
195+
$documentHandler->getById($this->collection->getName(), $docId2);
193196

194197
$batch->process();
195198
$result = $batch->getPart(0)->getProcessedResponse();

‎tests/DocumentBasicTest.php

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,9 @@ public function testCreateAndDeleteDocumentWithId()
7373
$document = Document::createFromArray([ "_key" => "me" ]);
7474
$documentHandler = new DocumentHandler($connection);
7575

76-
$documentId = $documentHandler->save($collection->getId(), $document);
76+
$documentId = $documentHandler->save($collection->getName(), $document);
7777

78-
$resultingDocument = $documentHandler->get($collection->getId(), $documentId);
78+
$resultingDocument = $documentHandler->get($collection->getName(), $documentId);
7979

8080
$key = $resultingDocument->getKey();
8181
static::assertSame("me", $key);
@@ -99,9 +99,9 @@ public function testCreateAndDeleteDocument()
9999

100100
$document->someAttribute = 'someValue';
101101

102-
$documentId = $documentHandler->save($collection->getId(), $document);
102+
$documentId = $documentHandler->save($collection->getName(), $document);
103103

104-
$resultingDocument = $documentHandler->get($collection->getId(), $documentId);
104+
$resultingDocument = $documentHandler->get($collection->getName(), $documentId);
105105

106106
$resultingAttribute = $resultingDocument->someAttribute;
107107
static::assertSame(
@@ -366,9 +366,9 @@ public function testCreateAndDeleteDocumentWithArray()
366366

367367
$documentArray = ['someAttribute' => 'someValue'];
368368

369-
$documentId = $documentHandler->save($collection->getId(), $documentArray);
369+
$documentId = $documentHandler->save($collection->getName(), $documentArray);
370370

371-
$resultingDocument = $documentHandler->get($collection->getId(), $documentId);
371+
$resultingDocument = $documentHandler->get($collection->getName(), $documentId);
372372

373373
$resultingAttribute = $resultingDocument->someAttribute;
374374
static::assertSame(
@@ -390,16 +390,16 @@ public function testCreateGetAndDeleteDocumentWithRevision()
390390

391391
$documentArray = ['someAttribute' => 'someValue'];
392392

393-
$documentId = $documentHandler->save($collection->getId(), $documentArray);
393+
$documentId = $documentHandler->save($collection->getName(), $documentArray);
394394

395-
$document = $documentHandler->get($collection->getId(), $documentId);
395+
$document = $documentHandler->get($collection->getName(), $documentId);
396396

397397
/**
398398
* lets get the document in a wrong revision
399399
*/
400400
try {
401401
$documentHandler->get(
402-
$collection->getId(), $documentId, [
402+
$collection->getName(), $documentId, [
403403
'ifMatch' => true,
404404
'revision' => 12345
405405
]
@@ -410,7 +410,7 @@ public function testCreateGetAndDeleteDocumentWithRevision()
410410

411411
try {
412412
$documentHandler->get(
413-
$collection->getId(), $documentId, [
413+
$collection->getName(), $documentId, [
414414
'ifMatch' => false,
415415
'revision' => $document->getRevision()
416416
]
@@ -419,7 +419,7 @@ public function testCreateGetAndDeleteDocumentWithRevision()
419419
}
420420
static::assertEquals('Document has not changed.', $exception304->getMessage());
421421

422-
$resultingDocument = $documentHandler->get($collection->getId(), $documentId);
422+
$resultingDocument = $documentHandler->get($collection->getName(), $documentId);
423423

424424
$resultingAttribute = $resultingDocument->someAttribute;
425425
static::assertSame(
@@ -431,7 +431,7 @@ public function testCreateGetAndDeleteDocumentWithRevision()
431431
$documentHandler->replace($resultingDocument);
432432

433433
$oldRevision = $documentHandler->get(
434-
$collection->getId(), $documentId,
434+
$collection->getName(), $documentId,
435435
['revision' => $resultingDocument->getRevision()]
436436
);
437437
static::assertEquals($oldRevision->getRevision(), $resultingDocument->getRevision());
@@ -449,30 +449,30 @@ public function testCreateHeadAndDeleteDocumentWithRevision()
449449

450450
$documentArray = ['someAttribute' => 'someValue'];
451451

452-
$documentId = $documentHandler->save($collection->getId(), $documentArray);
453-
$document = $documentHandler->get($collection->getId(), $documentId);
452+
$documentId = $documentHandler->save($collection->getName(), $documentArray);
453+
$document = $documentHandler->get($collection->getName(), $documentId);
454454

455455
try {
456-
$documentHandler->getHead($collection->getId(), $documentId, '12345', true);
456+
$documentHandler->getHead($collection->getName(), $documentId, '12345', true);
457457
} catch (\Exception $e412) {
458458
}
459459

460460
static::assertEquals(412, $e412->getCode());
461461

462462
try {
463-
$documentHandler->getHead($collection->getId(), 'notExisting');
463+
$documentHandler->getHead($collection->getName(), 'notExisting');
464464
} catch (\Exception $e404) {
465465
}
466466

467467
static::assertEquals(404, $e404->getCode());
468468

469469

470-
$result304 = $documentHandler->getHead($collection->getId(), $documentId, $document->getRevision(), false);
470+
$result304 = $documentHandler->getHead($collection->getName(), $documentId, $document->getRevision(), false);
471471
static::assertEquals('"' . $document->getRevision() . '"', $result304['etag']);
472472
static::assertEquals(0, $result304['content-length']);
473473
static::assertEquals(304, $result304['httpCode']);
474474

475-
$result200 = $documentHandler->getHead($collection->getId(), $documentId, $document->getRevision(), true);
475+
$result200 = $documentHandler->getHead($collection->getName(), $documentId, $document->getRevision(), true);
476476
static::assertEquals('"' . $document->getRevision() . '"', $result200['etag']);
477477
static::assertNotEquals(0, $result200['content-length']);
478478
static::assertEquals(200, $result200['httpCode']);
@@ -530,7 +530,7 @@ public function testHasDocumentReturnsFalseIfDocumentDoesNotExist()
530530
$connection = $this->connection;
531531
$collection = $this->collection;
532532
$documentHandler = new DocumentHandler($connection);
533-
static::assertFalse($documentHandler->has($collection->getId(), 'just_a_stupid_document_id_which_does_not_exist'));
533+
static::assertFalse($documentHandler->has($collection->getName(), 'just_a_stupid_document_id_which_does_not_exist'));
534534
}
535535

536536

@@ -544,9 +544,9 @@ public function testHasDocumentReturnsTrueIfDocumentExists()
544544
$document = new Document();
545545
$document->someAttribute = 'someValue';
546546

547-
$documentHandler->save($collection->getId(), $document);
547+
$documentHandler->save($collection->getName(), $document);
548548

549-
static::assertTrue($this->collectionHandler->has($collection->getId()));
549+
static::assertTrue($this->collectionHandler->has($collection->getName()));
550550
}
551551

552552

0 commit comments

Comments
 (0)