Skip to content

Commit

Permalink
Optimizing external calls on article-import-all
Browse files Browse the repository at this point in the history
Before the change, the static cache was not working very well as each article being imported was causing 5 requests to the /articles/:id/version API. After the change, the number of calls is reduced to 2, one in the service and only one from the NodePresave object.

Moreover, passing the article object on the stack is safer than keeping it around as a global static field, which may distribute stale data in long-running processes
  • Loading branch information
giorgiosironi committed Feb 9, 2017
1 parent ecc6159 commit 55e6738
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 31 deletions.
10 changes: 6 additions & 4 deletions src/modules/jcms_article/jcms_article.module
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,10 @@ function jcms_article_node_presave(EntityInterface $entity) {
return NULL;
}
$node_presave = \Drupal::service('jcms_article.hooks.node_presave');
$node_presave->addJsonFields($entity);
$node_presave->setPublishedStatus($entity);
$node_presave->setStatusDate($entity);
$node_presave->setSubjectTerms($entity);
// TODO: retrieve with getArticleById() and pass it in
$article = $node_presave->getArticleById($entity->label());
$node_presave->addJsonFields($entity, $article);
$node_presave->setPublishedStatus($entity, $article);
$node_presave->setStatusDate($entity, $article);
$node_presave->setSubjectTerms($entity, $article);
}
36 changes: 9 additions & 27 deletions src/modules/jcms_article/src/Hooks/NodePresave.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,6 @@ final class NodePresave {
*/
private $fetchArticleVersions;

/**
* @var string
*/
private static $articleData;

/**
* NodePresave constructor.
*
Expand All @@ -41,35 +36,26 @@ public function __construct(FetchArticleVersions $fetch_article_versions) {
* @return \Drupal\jcms_article\Entity\ArticleVersions|null
*/
public function getArticleById($id) {
// If there is no stored article data.
if (!self::$articleData) {
self::$articleData = $this->fetchArticleVersions->getArticleVersions($id);
}
// If there is stored article data but the ID doesn't match the request ID.
elseif (self::$articleData->getJsonObject()->id != $id) {
self::$articleData = $this->fetchArticleVersions->getArticleVersions($id);
}
return self::$articleData;
return $this->fetchArticleVersions->getArticleVersions($id);
}

/**
* Adds the JSON fields to the node.
*/
public function addJsonFields(EntityInterface $entity) {
public function addJsonFields(EntityInterface $entity, $article) {
if ($entity->get('field_article_json')->getValue()) {
$this->updateJsonParagraph($entity);
$this->updateJsonParagraph($entity, $article);
}
else {
$this->createJsonParagraph($entity);
$this->createJsonParagraph($entity, $article);
}
}

/**
* Sets the status date (the date article became VOR or POA) on the node.
*/
public function setStatusDate(EntityInterface $entity) {
public function setStatusDate(EntityInterface $entity, $article) {
$id = $entity->label();
$article = $this->getArticleById($id);
// Set the published date if there's a published version.
$version = $article->getLatestPublishedVersionJson() ?: '';
if (!$version) {
Expand All @@ -86,9 +72,8 @@ public function setStatusDate(EntityInterface $entity) {
/**
* Sets the published status of the node.
*/
public function setPublishedStatus(EntityInterface $entity) {
public function setPublishedStatus(EntityInterface $entity, $article) {
$id = $entity->label();
$article = $this->getArticleById($id);
// If there's a published version, set to published.
$status = $article->getLatestPublishedVersionJson() ? 1 : 0;
$entity->set('status', $status);
Expand All @@ -97,9 +82,8 @@ public function setPublishedStatus(EntityInterface $entity) {
/**
* Sets the article subjects on the article as taxonomy terms.
*/
public function setSubjectTerms(EntityInterface $entity) {
public function setSubjectTerms(EntityInterface $entity, $article) {
$id = $entity->label();
$article = $this->getArticleById($id);
// Use the unpublished JSON if no published exists.
$version = $article->getLatestPublishedVersionJson() ?: $article->getLatestUnpublishedVersionJson();
$json = json_decode($version);
Expand All @@ -122,9 +106,8 @@ public function setSubjectTerms(EntityInterface $entity) {
*
* @param \Drupal\Core\Entity\EntityInterface $entity
*/
private function updateJsonParagraph(EntityInterface $entity) {
private function updateJsonParagraph(EntityInterface $entity, $article) {
$id = $entity->label();
$article = $this->getArticleById($id);
$pid = $entity->get('field_article_json')->getValue()[0]['target_id'];
$paragraph = Paragraph::load($pid);
$published = $article->getLatestPublishedVersionJson();
Expand All @@ -146,9 +129,8 @@ private function updateJsonParagraph(EntityInterface $entity) {
*
* @param \Drupal\Core\Entity\EntityInterface $entity
*/
private function createJsonParagraph(EntityInterface $entity) {
private function createJsonParagraph(EntityInterface $entity, $article) {
$id = $entity->label();
$article = $this->getArticleById($id);
$published = $article->getLatestPublishedVersionJson();
// Store the published JSON if no unpublished exists.
$unpublished = $article->getLatestUnpublishedVersionJson() ?: $published;
Expand Down

0 comments on commit 55e6738

Please sign in to comment.