From 7dfb4448a1c6b69f7c221022142088aa101afa63 Mon Sep 17 00:00:00 2001 From: dantleech Date: Sun, 16 Nov 2014 11:39:03 +0100 Subject: [PATCH] Fix issue with double indexing --- src/Transport/Fs/Client.php | 54 ++++--- .../Fs/Search/Adapter/ZendSearchAdapter.php | 20 ++- tests/Transport/Fs/ClientTest.php | 44 ------ tests/Transport/Fs/FunctionalTestCase.php | 35 +++++ .../Fs/Search/Adapter/AdapterTestCase.php | 135 ++++++++++++++++++ .../Search/Adapter/ZendSearchAdapterTest.php | 14 ++ tests/bootstrap.php | 2 + 7 files changed, 235 insertions(+), 69 deletions(-) delete mode 100644 tests/Transport/Fs/ClientTest.php create mode 100644 tests/Transport/Fs/FunctionalTestCase.php create mode 100644 tests/Transport/Fs/Search/Adapter/AdapterTestCase.php create mode 100644 tests/Transport/Fs/Search/Adapter/ZendSearchAdapterTest.php diff --git a/src/Transport/Fs/Client.php b/src/Transport/Fs/Client.php index 5cd34fa..d6eecfd 100644 --- a/src/Transport/Fs/Client.php +++ b/src/Transport/Fs/Client.php @@ -98,29 +98,6 @@ public function __construct($factory, $parameters = array()) $this->registerEventSubscribers(); } - private function getFilesystemAdapter($adapterName) - { - if ($adapterName instanceof AdapterInterface) { - return $adapterName; - } - - if (null === $adapterName) { - return new LocalAdapter($this->path); - } - - switch ($adapterName) { - case 'local': - return new LocalAdapter($this->path); - case 'array': - return new ArrayAdapter(); - } - - throw new \InvalidArgumentException(sprintf( - 'Unknown filesystem adapter "%s", must be one of "%s"', - implode('", "', array('local', 'array')) - )); - } - private function getSearchAdapter() { $this->searchAdapter = new ZendSearchAdapter($this->path, $this->nodeTypeManager, $this->zendHideDestructException); @@ -715,4 +692,35 @@ private function validatePath($workspaceName, $path) )); } } + + /** + * Return the filesystem adapter indicated by $adapterName + * Defaults to LocalAdapter when null. + * + * @param string|AdapterInterface $adapterName + * @return AdapterInterface + * @throws InvalidArgumentException + */ + private function getFilesystemAdapter($adapterName) + { + if ($adapterName instanceof AdapterInterface) { + return $adapterName; + } + + if (null === $adapterName) { + return new LocalAdapter($this->path); + } + + switch ($adapterName) { + case 'local': + return new LocalAdapter($this->path); + case 'array': + return new ArrayAdapter(); + } + + throw new \InvalidArgumentException(sprintf( + 'Unknown filesystem adapter "%s", must be one of "%s"', + implode('", "', array('local', 'array')) + )); + } } diff --git a/src/Transport/Fs/Search/Adapter/ZendSearchAdapter.php b/src/Transport/Fs/Search/Adapter/ZendSearchAdapter.php index 1df8b0f..b079fb5 100644 --- a/src/Transport/Fs/Search/Adapter/ZendSearchAdapter.php +++ b/src/Transport/Fs/Search/Adapter/ZendSearchAdapter.php @@ -65,6 +65,8 @@ public function __construct($path, NodeTypeManagerInterface $nodeTypeManager = n public function index($workspace, $path, Node $node) { $index = $this->getIndex($workspace); + $this->removeExisting($index, $node); + $document = new Document(); $nodeName = PathHelper::getNodeName($path); $localNodeName = $nodeName; // PathHelper::getLocalNodeName($path); @@ -127,7 +129,6 @@ public function index($workspace, $path, Node $node) $document->addField(Field::Text($propertyName, $value)); break; } - }; $index->addDocument($document); @@ -203,7 +204,6 @@ public function query($workspace, QueryObjectModelInterface $qom) ); } - $properties[$selectorName] = array(); foreach ($document->getFieldNames() as $fieldName) { $field = $document->getField($fieldName); @@ -318,4 +318,20 @@ private function normalizeValue($value) return $value; } + + /** + * Remove any existing references to this node in the index + * + * @param Index $index Zend search index object + * @param Node $node Jackalope FS node object + */ + private function removeExisting(Index $index, Node $node) + { + $internalUuid = $node->getPropertyValue(Storage::INTERNAL_UUID); + $hits = $index->find(str_replace(':', "\\:", Storage::INTERNAL_UUID) . ':' . $internalUuid); + + foreach ($hits as $hit) { + $index->delete($hit->id); + } + } } diff --git a/tests/Transport/Fs/ClientTest.php b/tests/Transport/Fs/ClientTest.php deleted file mode 100644 index e5ce6d2..0000000 --- a/tests/Transport/Fs/ClientTest.php +++ /dev/null @@ -1,44 +0,0 @@ -fs = $this->prophesize('Jackalope\Transport\Fs\Filesystem\Filesystem'); - $this->client = new Client($factory, array('path' => __DIR__ . '/../../data'), $this->fs->reveal()); - } - - public function testGetNode() - { - $yamlData = <<fs->exists('/workspaces/default/foo/node.yml')->willReturn(true); - $this->fs->read('/workspaces/default/foo/node.yml')->willReturn($yamlData); - $this->fs->ls('/workspaces/default/foo')->willReturn(array('dirs' => array(), 'files' => array())); - $res = $this->client->getNode('/foo'); - - $this->assertEquals('Date', $res->{':jcr:created'}); - $this->assertEquals('admin', $res->{'jcr:createdBy'}); - } -} diff --git a/tests/Transport/Fs/FunctionalTestCase.php b/tests/Transport/Fs/FunctionalTestCase.php new file mode 100644 index 0000000..95a1f5b --- /dev/null +++ b/tests/Transport/Fs/FunctionalTestCase.php @@ -0,0 +1,35 @@ +path = __DIR__ . '/../../data'; + + $fs = new Filesystem(); + + if (file_exists($this->path)) { + $fs->remove($this->path); + } + + $parameters = array_merge(array( + 'path' => $this->path, + ), $parameters); + + $factory = new RepositoryFactoryFilesystem(); + $repository = $factory->getRepository($parameters); + $credentials = new SimpleCredentials('admin', 'admin'); + $session = $repository->login($credentials); + + return $session; + } +} diff --git a/tests/Transport/Fs/Search/Adapter/AdapterTestCase.php b/tests/Transport/Fs/Search/Adapter/AdapterTestCase.php new file mode 100644 index 0000000..7296011 --- /dev/null +++ b/tests/Transport/Fs/Search/Adapter/AdapterTestCase.php @@ -0,0 +1,135 @@ +session = $this->getSession(array( + 'filesystem.adapter' => 'array' + )); + $this->nodeTypeManager = $this->session->getWorkspace()->getNodeTypeManager(); + $this->queryManager = $this->session->getWorkspace()->getQueryManager(); + $this->adapter = $this->getAdapter(); + } + + abstract protected function getAdapter(); + + public function provideQuery() + { + return array( + array('simple', 'SELECT * FROM [nt:unstructured]', 2), + array('simple', 'SELECT * FROM [nt:unstructured] WHERE field1 = "value 1"', 1) + ); + } + + /** + * @dataProvider provideQuery + */ + public function testQuery($nodeDataName, $query, $expectedNbResults) + { + $this->indexNodeData($nodeDataName); + + $res = $this->adapter->query('workspace', $this->queryToQOM($query)); + $this->assertCount($expectedNbResults, $res); + } + + /** + * @dataProvider provideQuery + */ + public function testQueryDoubleIndex($nodeDataName, $query, $expectedNbResults) + { + $this->indexNodeData($nodeDataName); + $this->indexNodeData($nodeDataName); + + $res = $this->adapter->query('workspace', $this->queryToQOM($query)); + $this->assertCount($expectedNbResults, $res); + } + + public function testQueryNewIndex() + { + $this->indexNodeData('simple'); + $this->indexNodeData('article'); + + $query = 'SELECT * FROM [nt:unstructured] WHERE title = "Article Title"'; + $res = $this->adapter->query('workspace', $this->queryToQOM($query)); + $this->assertCount(1, $res); + } + + protected function getNodeData($name) + { + switch ($name) { + case 'article': + return array( + array( + '/node/node-new', + array( + Storage::INTERNAL_UUID => 'article', + 'jcr:primaryType' => 'nt:unstructured', + 'title' => 'Article Title', + 'body' => 'This is the article body', + ), + ), + ); + case 'simple': + default: + return array( + array( + '/node/node1', + array( + Storage::INTERNAL_UUID => 'simple1', + 'jcr:primaryType' => 'nt:unstructured', + 'field1' => 'value 1', + 'field2' => 'value 2', + ), + ), + array( + '/node/node2', + array( + Storage::INTERNAL_UUID => 'simple2', + 'jcr:primaryType' => 'nt:unstructured', + 'field1' => 'value 3', + 'field2' => 'value 4', + ), + ), + ); + } + } + + protected function indexNodeData($nodeDataName) + { + $nodeData = $this->getNodeData($nodeDataName); + + foreach ($nodeData as $nodeDatum) { + list($path, $properties) = $nodeDatum; + $node = new Node(); + + foreach ($properties as $key => $value) { + $node->setProperty($key, $value); + } + + $this->adapter->index('workspace', $path, $node); + } + } + + protected function queryToQOM($query) + { + $parser = new Sql2ToQomQueryConverter($this->queryManager->getQOMFactory()); + $qom = $parser->parse($query); + + return $qom; + } +} + + diff --git a/tests/Transport/Fs/Search/Adapter/ZendSearchAdapterTest.php b/tests/Transport/Fs/Search/Adapter/ZendSearchAdapterTest.php new file mode 100644 index 0000000..43344ef --- /dev/null +++ b/tests/Transport/Fs/Search/Adapter/ZendSearchAdapterTest.php @@ -0,0 +1,14 @@ +path, $this->nodeTypeManager); + } +} + diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 1cf58a1..f03fe60 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -39,7 +39,9 @@ require_once __DIR__ . '/../vendor/phpcr/phpcr-api-tests/inc/AbstractLoader.php'; require_once __DIR__ . '/../vendor/phpcr/phpcr-api-tests/inc/FixtureLoaderInterface.php'; require_once __DIR__ . '/ImplementationLoader.php'; +require_once __DIR__ . '/Transport/Fs/FunctionalTestCase.php'; require_once __DIR__ . '/Transport/Fs/Filesystem/Adapter/AdapterTestCase.php'; +require_once __DIR__ . '/Transport/Fs/Search/Adapter/AdapterTestCase.php'; function dodefine($name, $value) {