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

BUGFIX: Target workspace handling during indexing #26

Merged
merged 5 commits into from Feb 21, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 7 additions & 1 deletion Classes/Indexer/AbstractNodeIndexer.php
Expand Up @@ -52,6 +52,7 @@ abstract class AbstractNodeIndexer implements NodeIndexerInterface
* Called by the Flow object framework after creating the object and resolving all dependencies.
*
* @param integer $cause Creation cause
* @throws \Neos\Flow\Configuration\Exception\InvalidConfigurationTypeException
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually this method doesn't throw an exception

*/
public function initializeObject($cause)
{
Expand Down Expand Up @@ -91,6 +92,8 @@ protected function evaluateEelExpression($expression, NodeInterface $node, $prop
* @param string $fulltextExtractionExpression
* @param array $fulltextIndexOfNode
* @throws IndexingException
* @throws \Neos\ContentRepository\Exception\NodeException
* @throws \Neos\Eel\Exception
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These exceptions are thrown in called methods - we only annotate the exceptions thrown here (maybe you should reconfigure your phpStorm :))

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we only annotate the exceptions thrown here

Well, do we? Says who?

maybe you should reconfigure your phpStorm

Actually those warnings bother me, but I don't want to disable the inspection (since on actually throwing it definitely must be added). So I started to add them also when it's an "indirect throw", and found it quite helpful.

Anyway, let's handle that in https://discuss.neos.io/t/to-throws-or-not-to-throws-indirect-exceptions/3018 – ok?

*/
protected function extractFulltext(NodeInterface $node, $propertyName, $fulltextExtractionExpression, array &$fulltextIndexOfNode)
{
Expand Down Expand Up @@ -118,6 +121,9 @@ protected function extractFulltext(NodeInterface $node, $propertyName, $fulltext
* @param array $fulltextData
* @param \Closure $nonIndexedPropertyErrorHandler
* @return array
* @throws IndexingException
* @throws \Neos\ContentRepository\Exception\NodeException
* @throws \Neos\Eel\Exception
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No Exceptions thrown ...

*/
protected function extractPropertiesAndFulltext(NodeInterface $node, array &$fulltextData, \Closure $nonIndexedPropertyErrorHandler = null)
{
Expand Down Expand Up @@ -169,4 +175,4 @@ protected function isFulltextEnabled(NodeInterface $node)

return false;
}
}
}
33 changes: 25 additions & 8 deletions Classes/Indexer/NodeIndexingManager.php
Expand Up @@ -37,6 +37,11 @@ class NodeIndexingManager
*/
protected $targetWorkspaceNamesForNodesToBeIndexed = [];

/**
* @var array
*/
protected $targetWorkspaceNamesForNodesToBeRemoved = [];

/**
* the indexing batch size (from the settings)
*
Expand Down Expand Up @@ -71,28 +76,35 @@ public function injectSettings(array $settings)
* Schedule a node for indexing
*
* @param NodeInterface $node
* @param mixed $targetWorkspace In case this is triggered during publishing, a Workspace will be passed in
* @param Workspace $targetWorkspace In case this is triggered during publishing, a Workspace will be passed in
* @return void
*/
public function indexNode(NodeInterface $node, $targetWorkspace = null)
public function indexNode(NodeInterface $node, Workspace $targetWorkspace = null)
{
$this->nodesToBeRemoved->detach($node);
$this->nodesToBeIndexed->attach($node);
$this->targetWorkspaceNamesForNodesToBeIndexed[$node->getContextPath()] = $targetWorkspace instanceof Workspace ? $targetWorkspace->getName() : null;
// if this is triggered via afterNodePublishing, it could be a deletion, check and handle
if ($node->isRemoved() && $targetWorkspace !== null && $targetWorkspace->getBaseWorkspace() === null) {
$this->removeNode($node, $targetWorkspace);
} else {
$this->nodesToBeRemoved->detach($node);
$this->nodesToBeIndexed->attach($node);
$this->targetWorkspaceNamesForNodesToBeIndexed[$node->getContextPath()] = $targetWorkspace instanceof Workspace ? $targetWorkspace->getName() : null;

$this->flushQueuesIfNeeded();
$this->flushQueuesIfNeeded();
}
}

/**
* Schedule a node for removal of the index
*
* @param NodeInterface $node
* @param Workspace $targetWorkspace In case this is triggered during publishing, a Workspace will be passed in
* @return void
*/
public function removeNode(NodeInterface $node)
public function removeNode(NodeInterface $node, Workspace $targetWorkspace = null)
{
$this->nodesToBeIndexed->detach($node);
$this->nodesToBeRemoved->attach($node);
$this->targetWorkspaceNamesForNodesToBeRemoved[$node->getContextPath()] = $targetWorkspace instanceof Workspace ? $targetWorkspace->getName() : null;

$this->flushQueuesIfNeeded();
}
Expand Down Expand Up @@ -129,13 +141,18 @@ public function flushQueues()

/** @var NodeInterface $nodeToBeRemoved */
foreach ($this->nodesToBeRemoved as $nodeToBeRemoved) {
$this->nodeIndexer->removeNode($nodeToBeRemoved);
if (isset($this->targetWorkspaceNamesForNodesToBeRemoved[$nodeToBeRemoved->getContextPath()])) {
$this->nodeIndexer->removeNode($nodeToBeRemoved, $this->targetWorkspaceNamesForNodesToBeRemoved[$nodeToBeRemoved->getContextPath()]);
} else {
$this->nodeIndexer->removeNode($nodeToBeRemoved);
}
}

$this->nodeIndexer->flush();
$this->nodesToBeIndexed = new \SplObjectStorage();
$this->nodesToBeRemoved = new \SplObjectStorage();
$this->targetWorkspaceNamesForNodesToBeIndexed = [];
$this->targetWorkspaceNamesForNodesToBeRemoved = [];
};

if ($this->nodeIndexer instanceof BulkNodeIndexerInterface) {
Expand Down
8 changes: 4 additions & 4 deletions Classes/Package.php
Expand Up @@ -54,13 +54,13 @@ public function registerIndexingSlots(Bootstrap $bootstrap)
$settings = $configurationManager->getConfiguration(ConfigurationManager::CONFIGURATION_TYPE_SETTINGS, $this->getPackageKey());
if (isset($settings['realtimeIndexing']['enabled']) && $settings['realtimeIndexing']['enabled'] === true) {
// handle changes to nodes
$bootstrap->getSignalSlotDispatcher()->connect(Node::class, 'nodeAdded', Indexer\NodeIndexingManager::class, 'indexNode');
$bootstrap->getSignalSlotDispatcher()->connect(Node::class, 'nodeUpdated', Indexer\NodeIndexingManager::class, 'indexNode');
$bootstrap->getSignalSlotDispatcher()->connect(Node::class, 'nodeRemoved', Indexer\NodeIndexingManager::class, 'removeNode');
$bootstrap->getSignalSlotDispatcher()->connect(Node::class, 'nodeAdded', Indexer\NodeIndexingManager::class, 'indexNode', false);
$bootstrap->getSignalSlotDispatcher()->connect(Node::class, 'nodeUpdated', Indexer\NodeIndexingManager::class, 'indexNode', false);
$bootstrap->getSignalSlotDispatcher()->connect(Node::class, 'nodeRemoved', Indexer\NodeIndexingManager::class, 'removeNode', false);
// all publishing calls (Workspace, PublishingService) eventually trigger this - and publishing is triggered in various ways
$bootstrap->getSignalSlotDispatcher()->connect(Workspace::class, 'afterNodePublishing', Indexer\NodeIndexingManager::class, 'indexNode', false);
// make sure we always flush at the end, regardless of indexingBatchSize
$bootstrap->getSignalSlotDispatcher()->connect(PersistenceManager::class, 'allObjectsPersisted', Indexer\NodeIndexingManager::class, 'flushQueues');
$bootstrap->getSignalSlotDispatcher()->connect(PersistenceManager::class, 'allObjectsPersisted', Indexer\NodeIndexingManager::class, 'flushQueues', false);
}
}
}