From f7f1e81454b9bae91da3bfde75557cfd240d98ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dezs=C5=91=20BICZ=C3=93?= Date: Wed, 18 Jun 2025 16:49:18 +0200 Subject: [PATCH 1/2] Eliminate repeated update of Drupal users --- src/Job/DeveloperSync.php | 2 +- src/Job/UserCreateUpdate.php | 9 ++++++--- src/UserDeveloperConverter.php | 5 +++++ 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/Job/DeveloperSync.php b/src/Job/DeveloperSync.php index 5ce6eb7ab..ecd353460 100644 --- a/src/Job/DeveloperSync.php +++ b/src/Job/DeveloperSync.php @@ -147,7 +147,7 @@ public function execute(): bool { $last_modified_delta = $developer->getLastModifiedAt()->getTimestamp() - $user->getChangedTime(); // Update Drupal user because the Apigee Edge developer is the most // recent. - if ($last_modified_delta >= 0) { + if ($last_modified_delta > 0) { $update_user_job = new UserUpdate($user->getEmail()); $update_user_job->setTag($this->getTag()); $this->scheduleJob($update_user_job); diff --git a/src/Job/UserCreateUpdate.php b/src/Job/UserCreateUpdate.php index df51748db..c849aeb15 100644 --- a/src/Job/UserCreateUpdate.php +++ b/src/Job/UserCreateUpdate.php @@ -75,6 +75,12 @@ protected function executeRequest() { // the same developer in apigee_edge_user_presave() while creating // Drupal user based on a developer should be avoided. _apigee_edge_set_sync_in_progress(TRUE); + + // Synchronize user's last modified timestamp with developer's timestamp + // to maintain data consistency and prevent infinite sync loops where + // outdated timestamps cause repeated scheduling of sync operations. + // @see \Drupal\apigee_edge\Job\DeveloperSync::execute() + $result->getUser()->setChangedTime($developer->getLastModifiedAt()->getTimestamp()); $result->getUser()->save(); } } @@ -121,9 +127,6 @@ protected function beforeUserSave(DeveloperToUserConversionResult $result) : voi throw $problem; } } - // It's necessary because changed time is automatically updated on the - // UI only. - $result->getUser()->setChangedTime(\Drupal::time()->getCurrentTime()); } /** diff --git a/src/UserDeveloperConverter.php b/src/UserDeveloperConverter.php index 44eac4b66..d5b378b8e 100644 --- a/src/UserDeveloperConverter.php +++ b/src/UserDeveloperConverter.php @@ -182,6 +182,11 @@ public function convertDeveloper(DeveloperInterface $developer) : DeveloperToUse $user = $user_storage->create([ 'pass' => \Drupal::service('password_generator')->generate(), ]); + // Set user creation date to match the original developer's creation + // timestamp to maintain chronological consistency between related + // entities and prevent confusion when comparing created vs modified dates + // in the user interface. + $user->set('created', $developer->getCreatedAt()->getTimestamp()); // Suppress invalid email validation errors. DeveloperEmailUniqueValidator::whitelist($developer->id()); } From 8afcab08c7bca7f6c0d4175f305ab26f23db773b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dezs=C5=91=20BICZ=C3=93?= Date: Wed, 18 Jun 2025 17:48:16 +0200 Subject: [PATCH 2/2] Store last update attempts locally for all developer/users --- apigee_edge.module | 3 ++ apigee_edge.services.yml | 5 +++ src/Job/DeveloperCreateUpdate.php | 5 +++ src/Job/DeveloperSync.php | 70 +++++++++++++++++++++++++------ src/Job/UserCreateUpdate.php | 19 +++++---- src/UserDeveloperConverter.php | 5 --- 6 files changed, 81 insertions(+), 26 deletions(-) diff --git a/apigee_edge.module b/apigee_edge.module index 6a542c398..734d03d72 100644 --- a/apigee_edge.module +++ b/apigee_edge.module @@ -1365,6 +1365,7 @@ function apigee_edge_user_presave(UserInterface $account) { } $developer = $result->getDeveloper(); $developer->save(); + \Drupal::getContainer()->get('apigee_edge.dev_sync.last_update_tracker')->set($developer->getEmail(), \Drupal::time()->getCurrentTime()); } catch (\Exception $exception) { $previous = $exception->getPrevious(); @@ -1386,6 +1387,7 @@ function apigee_edge_user_presave(UserInterface $account) { // "originalEmail" property's value on the entity. $developer->enforceIsNew(TRUE); $developer->save(); + \Drupal::getContainer()->get('apigee_edge.dev_sync.last_update_tracker')->set($developer->getEmail(), \Drupal::time()->getCurrentTime()); } catch (\Exception $exception) { $context = [ @@ -1410,6 +1412,7 @@ function apigee_edge_user_presave(UserInterface $account) { $developer->enforceIsNew(FALSE); try { $developer->save(); + \Drupal::getContainer()->get('apigee_edge.dev_sync.last_update_tracker')->set($result->getDeveloper()->getEmail(), \Drupal::time()->getCurrentTime()); } catch (ApiException $exception) { $logger->error("Unable to update existing @developer developer's data after registered on the portal.", $context); diff --git a/apigee_edge.services.yml b/apigee_edge.services.yml index 82334a68b..599a5868a 100644 --- a/apigee_edge.services.yml +++ b/apigee_edge.services.yml @@ -217,3 +217,8 @@ services: apigee_edge.post_user_delete_action_performer: class: Drupal\apigee_edge\User\RemoveRelatedDeveloperAccountSynchronousPostUserDeleteActionPerformer arguments: ['@entity_type.manager', '@logger.channel.apigee_edge'] + + apigee_edge.dev_sync.last_update_tracker: + class: Drupal\Core\KeyValueStore\KeyValueStoreInterface + factory: [ '@keyvalue', 'get' ] + arguments: [ 'apigee_edge.dev_sync.last_update_tracker' ] diff --git a/src/Job/DeveloperCreateUpdate.php b/src/Job/DeveloperCreateUpdate.php index 191ff6df8..804b16695 100644 --- a/src/Job/DeveloperCreateUpdate.php +++ b/src/Job/DeveloperCreateUpdate.php @@ -70,6 +70,11 @@ protected function executeRequest() { if ($result->getSuccessfullyAppliedChanges() > 0) { $result->getDeveloper()->save(); } + // Record the sync attempt timestamp for the developer's email, + // regardless of whether an entity update occurred, to prevent redundant + // sync operations. This ensures that the developer will only be re-synced + // if a new relevant change is detected after this timestamp. + \Drupal::getContainer()->get('apigee_edge.dev_sync.last_update_tracker')->set($result->getDeveloper()->getEmail(), \Drupal::time()->getCurrentTime()); } catch (\Exception $exception) { $message = '@operation: Skipping %mail developer. @message %function (line %line of %file). 
@backtrace_string
'; diff --git a/src/Job/DeveloperSync.php b/src/Job/DeveloperSync.php index ecd353460..390a6cbe5 100644 --- a/src/Job/DeveloperSync.php +++ b/src/Job/DeveloperSync.php @@ -20,7 +20,10 @@ namespace Drupal\apigee_edge\Job; use Drupal\apigee_edge\Entity\Developer; +use Drupal\apigee_edge\Entity\DeveloperInterface; +use Drupal\Core\KeyValueStore\KeyValueStoreInterface; use Drupal\user\Entity\User; +use Drupal\user\UserInterface; /** * A job that synchronizes Apigee Edge developers and Drupal users. @@ -58,6 +61,11 @@ class DeveloperSync extends EdgeJob { */ protected $filter = NULL; + /** + * KV store tracks last update attempts for each user/developer. + */ + protected KeyValueStoreInterface $lastUpdateTracker; + /** * DeveloperSync constructor. * @@ -67,6 +75,7 @@ class DeveloperSync extends EdgeJob { public function __construct(?string $filter) { parent::__construct(); $this->filter = $filter; + $this->lastUpdateTracker = \Drupal::service('apigee_edge.dev_sync.last_update_tracker'); } /** @@ -130,6 +139,30 @@ protected function executeRequest() { $this->edgeDevelopers = $this->loadDevelopers(); } + /** + * Schedules the update of a user. + * + * @param \Drupal\user\UserInterface $user + * The user. + */ + protected function scheduleUserUpdate(UserInterface $user): void { + $update_user_job = new UserUpdate($user->getEmail()); + $update_user_job->setTag($this->getTag()); + $this->scheduleJob($update_user_job); + } + + /** + * Schedules the update of a developer. + * + * @param \Drupal\apigee_edge\Entity\DeveloperInterface $developer + * The developer. + */ + protected function scheduleDeveloper(DeveloperInterface $developer): void { + $update_developer_job = new DeveloperUpdate($developer->getEmail()); + $update_developer_job->setTag($this->getTag()); + $this->scheduleJob($update_developer_job); + } + /** * {@inheritdoc} */ @@ -144,20 +177,31 @@ public function execute(): bool { /** @var \Drupal\user\UserInterface $user */ $user = $this->drupalUsers[$clean_email]; - $last_modified_delta = $developer->getLastModifiedAt()->getTimestamp() - $user->getChangedTime(); - // Update Drupal user because the Apigee Edge developer is the most - // recent. - if ($last_modified_delta > 0) { - $update_user_job = new UserUpdate($user->getEmail()); - $update_user_job->setTag($this->getTag()); - $this->scheduleJob($update_user_job); + $last_synced = $this->lastUpdateTracker->get($developer->getEmail(), 0); + + if ($last_synced === 0) { + $last_modified_delta = $developer->getLastModifiedAt()->getTimestamp() - $user->getChangedTime(); + if ($last_modified_delta === 0) { + $this->lastUpdateTracker->set($developer->getEmail(), \Drupal::time()->getCurrentTime()); + continue; + } + + // Update Drupal user because the Apigee Edge developer is the most + // recent. + if ($last_modified_delta > 0) { + $this->scheduleUserUpdate($user); + } + // Update Apigee Edge developer because the Drupal user is the most + // recent. + else { + $this->scheduleDeveloper($developer); + } + } + elseif ($last_synced < $developer->getLastModifiedAt()->getTimestamp()) { + $this->scheduleUserUpdate($user); } - // Update Apigee Edge developer because the Drupal user is the most - // recent. - elseif ($last_modified_delta < 0) { - $update_developer_job = new DeveloperUpdate($developer->getEmail()); - $update_developer_job->setTag($this->getTag()); - $this->scheduleJob($update_developer_job); + elseif ($last_synced < $user->getChangedTime()) { + $this->scheduleDeveloper($developer); } } diff --git a/src/Job/UserCreateUpdate.php b/src/Job/UserCreateUpdate.php index c849aeb15..9a5a9c9da 100644 --- a/src/Job/UserCreateUpdate.php +++ b/src/Job/UserCreateUpdate.php @@ -75,14 +75,14 @@ protected function executeRequest() { // the same developer in apigee_edge_user_presave() while creating // Drupal user based on a developer should be avoided. _apigee_edge_set_sync_in_progress(TRUE); - - // Synchronize user's last modified timestamp with developer's timestamp - // to maintain data consistency and prevent infinite sync loops where - // outdated timestamps cause repeated scheduling of sync operations. - // @see \Drupal\apigee_edge\Job\DeveloperSync::execute() - $result->getUser()->setChangedTime($developer->getLastModifiedAt()->getTimestamp()); $result->getUser()->save(); } + + // Record the sync attempt timestamp for the developer's email, + // regardless of whether an entity update occurred, to prevent redundant + // sync operations. This ensures that the developer will only be re-synced + // if a new relevant change is detected after this timestamp. + \Drupal::getContainer()->get('apigee_edge.dev_sync.last_update_tracker')->set($developer->getEmail(), \Drupal::time()->getCurrentTime()); } catch (\Exception $exception) { $message = '@operation: Skipping %mail user. @message %function (line %line of %file). 
@backtrace_string
'; @@ -91,8 +91,8 @@ protected function executeRequest() { '@operation' => get_class($this), ]; $context += Error::decodeException($exception); - // Unset backtrace, exception, severity_level as they are not shown in the log message - // and throws php warning in logs. + // Unset backtrace, exception, severity_level as they are not shown in + // the log message and throws php warning in logs. unset($context['backtrace'], $context['exception'], $context['severity_level']); $this->logger()->error($message, $context); @@ -127,6 +127,9 @@ protected function beforeUserSave(DeveloperToUserConversionResult $result) : voi throw $problem; } } + // It's necessary because changed time is automatically updated on the + // UI only. + $result->getUser()->setChangedTime(\Drupal::time()->getCurrentTime()); } /** diff --git a/src/UserDeveloperConverter.php b/src/UserDeveloperConverter.php index d5b378b8e..44eac4b66 100644 --- a/src/UserDeveloperConverter.php +++ b/src/UserDeveloperConverter.php @@ -182,11 +182,6 @@ public function convertDeveloper(DeveloperInterface $developer) : DeveloperToUse $user = $user_storage->create([ 'pass' => \Drupal::service('password_generator')->generate(), ]); - // Set user creation date to match the original developer's creation - // timestamp to maintain chronological consistency between related - // entities and prevent confusion when comparing created vs modified dates - // in the user interface. - $user->set('created', $developer->getCreatedAt()->getTimestamp()); // Suppress invalid email validation errors. DeveloperEmailUniqueValidator::whitelist($developer->id()); }