fixup Cleanud up logging

modules/harvest/harvest.install

@@ -49,7 +49,7 @@ function harvest_get_temp_run_ids($table_name_temp) : array {
     ->fields('hrt', ['id']);
   $query->orderBy('id', 'ASC');
   $result = $query->execute()->fetchCol(0);
-  // Can't do orderBy as the sort is natural, not numeric.
+  // Can't do orderBy as the sort end up natural, not numeric.
   asort($result, SORT_NUMERIC);
 
   return $result ?? [];
@@ -78,12 +78,12 @@ function harvest_read_harvest_run(string $table_name_temp, string $time_id): arr
   return reset($result);
 }
 
-function harvest_write_harvest_run(string $id, $harvest_plan_id, string $data, string $extract_status) {
+function harvest_write_harvest_run(string $id, string $harvest_plan_id, string $data, string $extract_status) {
   /** @var \Drupal\Core\Database\Connection $connection */
   $connection = \Drupal::service('database');
   $result = $connection->insert('harvest_runs')
     ->fields([
-      'timestamp' => $id,
+      'timestamp' => (int) $id,
       'harvest_plan_id' => $harvest_plan_id,
       'data' => $data,
       'extract_status' => $extract_status,
@@ -193,7 +193,6 @@ function harvest_update_8009(&$sandbox) {
   return $messages;
 }
 
-// @todo clean up messages across all three of these update hooks.
 /**
  * Move data from temp table back into harvest_run.
  *
@@ -203,9 +202,6 @@ function harvest_update_8010(&$sandbox) {
   $table_name = 'harvest_runs';
   $table_name_temp = "{$table_name}_temp";
   $messages = '';
-
-  /** @var \Drupal\harvest\entity\HarvestRunRepository $harvest_run_repository */
-  $harvest_run_repository = \Drupal::service('dkan.harvest.storage.harvest_run_repository');
   $schema = \Drupal::database()->schema();
 
   if (!isset($sandbox['total'])) {
@@ -220,7 +216,7 @@ function harvest_update_8010(&$sandbox) {
   foreach ($harvest_runs_batch as $key => $time_id) {
     // Load the old row.
     $row = harvest_read_harvest_run($table_name_temp, $time_id);
-
+    // Write the new harvest run.
     harvest_write_harvest_run($row['id'], $row['harvest_plan_id'], $row['data'], $row['extract_status']);
     // The item has been processed, remove it from the array.
     unset($sandbox['items_to_process'][$key]);
@@ -231,17 +227,14 @@ function harvest_update_8010(&$sandbox) {
   $sandbox['#finished'] = (empty($sandbox['total'])) ? 1 : ($sandbox['current'] / $sandbox['total']);
   $vars = [
     '@completed' => $sandbox['current'],
-    '@element' => $sandbox['element'],
     '@total' => $sandbox['total'],
   ];
 
-  $messages = t('Processed @element. @completed/@total.', $vars) . PHP_EOL;
+  $messages = t('Processed: @completed/@total.', $vars) . PHP_EOL;
   // Log the all finished notice.
   if ($sandbox['#finished'] === 1) {
     // The update of the harvest_runs is complete.
-    \Drupal::logger('script_library')->log(LogLevel::INFO, $completed_message, $vars);
-    $logged_message = new FormattableMarkup($completed_message, $vars);
-    $messages .= t('Data in harvest_runs updated to new schema:') . " {$logged_message}" . PHP_EOL;
+    $messages .= t('Data in harvest_runs updated to new schema:') . PHP_EOL;
     $dropped = $schema->dropTable($table_name_temp);
     if ($dropped) {
       $messages .= t('Temporary table dropped.') . PHP_EOL;

modules/harvest/src/Commands/HarvestCommands.php

@@ -221,11 +221,6 @@ public function runAll($options = ['new' => FALSE]) {
     foreach ($plan_ids as $plan_id) {
       $result = $this->harvestService->runHarvest($plan_id);
       $runs_info[] = $result;
-      // Since run IDs are also one-second-resolution timestamps, we must wait
-      // one second before running the next harvest.
-      // @todo Remove this sleep when we've switched to a better system for
-      //   timestamps.
-      sleep(1);
     }
     $this->renderHarvestRunsInfo($runs_info);
   }

modules/harvest/src/Entity/HarvestRun.php

@@ -70,13 +70,15 @@ public static function baseFieldDefinitions(EntityTypeInterface $entity_type) {
       ])
       ->setReadOnly(TRUE);
 
-    $base_fields['timestamp'] = static::getBaseFieldIdentifier(
-      new TranslatableMarkup('Harvest Run')
-    );
+    $base_fields['timestamp'] = BaseFieldDefinition::create('timestamp')
+      ->setLabel(t('timestamp'))
+      ->setDescription(t('The timestamp of when this harvest was run.'))
+      ->setRequired(TRUE);
 
     $base_fields['uuid'] = BaseFieldDefinition::create('uuid')
       ->setLabel(t('UUID'))
-      ->setDescription(t('The unique identifier for this node'))
+      ->setDescription(t('The unique identifier for this harvest_run'))
+      ->setRequired(TRUE)
       ->setReadOnly(TRUE);
 
     // Harvest plan id. This is the name of the harvest plan as seen in the UI.

modules/harvest/src/Entity/HarvestRunRepository.php

@@ -102,7 +102,7 @@ public function destructForPlanId(string $plan_id) {
    */
   public function storeRun(array $run_data, string $plan_id, string $timestamp): string {
     $field_values = [
-      'timestamp' => $timestamp,
+      'timestamp' => (int) $timestamp,
       'harvest_plan_id' => $plan_id,
     ];
     $field_values['extract_status'] = $run_data['status']['extract'] ?? 'FAILURE';
@@ -138,7 +138,7 @@ public function storeRun(array $run_data, string $plan_id, string $timestamp): s
 
     // JSON encode remaining run data.
     $field_values['data'] = json_encode($run_data);
-// Might want to cast this to a string for BC.?
+
     return $this->writeEntity($field_values, $plan_id, $timestamp);
   }
 
@@ -249,16 +249,16 @@ public function getExtractedUuids(string $plan_id, string $timestamp): array {
    * @return \Drupal\harvest\HarvestRunInterface|\Drupal\Core\Entity\EntityInterface|null
    *   The loaded entity or NULL if none could be loaded.
    *
-   * @deprecated in 2.19 and is removed from 3.x. Use HarvestService::load().
+   * @deprecated in dkan:2.19.11 and is removed from dkan:3.0.0 Use HarvestService::load().
    */
   public function loadEntity(string $plan_id, string $timestamp): ?HarvestRunInterface {
     if ($ids = $this->runStorage->getQuery()
       ->condition('timestamp', $timestamp)
       ->condition('harvest_plan_id', $plan_id)
+      ->sort('id', 'DESC')
       ->range(0, 1)
       ->accessCheck(FALSE)
       ->execute()
-      // @todo sort by id.
     ) {
       return $this->runStorage->load(reset($ids));
     }
@@ -272,21 +272,22 @@ public function loadEntity(string $plan_id, string $timestamp): ?HarvestRunInter
    *   Structured data ready to send to entity_storage->create().
    * @param string $plan_id
    *   Harvest plan identifier.
-   * @param string $timestamp
+   * @param mixed $timestamp
    *   Harvest run timestamp.
    *
    * @return string
    *   Harvest run id.
    */
-  public function writeEntity(array $field_values, string $plan_id, string $timestamp) {
+  public function writeEntity(array $field_values, string $plan_id, mixed $timestamp) {
+    $timestamp = (int) $timestamp;
     /** @var \Drupal\harvest\HarvestRunInterface $entity */
     $entity = $this->loadEntity($plan_id, $timestamp);
     if ($entity) {
       // Modify entity.
-      unset($field_values['id']);
       foreach ($field_values as $key => $value) {
         $entity->set($key, $value);
       }
+      $field_values['id'] = $entity->id();
     }
     else {
       // Create new entity.

modules/harvest/src/HarvestService.php

@@ -208,7 +208,7 @@ public function revertHarvest($id) {
   public function runHarvest($plan_id) {
     $harvester = $this->getHarvester($plan_id);
 
-    $run_id = (string) time();
+    $timestamp = (string) time();
     $result = $harvester->harvest();
 
     if (empty($result['status']['extracted_items_ids'])) {
@@ -218,8 +218,7 @@ public function runHarvest($plan_id) {
       $this->getOrphanIdsFromResult($plan_id, $result['status']['extracted_items_ids']);
     $this->processOrphanIds($result['status']['orphan_ids']);
 
-    $result['identifier'] = $run_id;
-    $this->runRepository->storeRun($result, $plan_id, $run_id);
+    $this->runRepository->storeRun($result, $plan_id, $timestamp);
 
     return $result;
   }
@@ -229,15 +228,15 @@ public function runHarvest($plan_id) {
    *
    * @param string $plan_id
    *   The harvest plan ID.
-   * @param string $run_id
+   * @param string $timestamp
    *   The harvest run ID.
    *
    * @return bool|string
    *   JSON-encoded run information for the given run, or FALSE if no matching
    *   runID is found.
    */
-  public function getHarvestRunInfo(string $plan_id, string $run_id): bool|string {
-    if ($info = $this->runRepository->retrieveRunJson($plan_id, $run_id)) {
+  public function getHarvestRunInfo(string $plan_id, string $timestamp): bool|string {
+    if ($info = $this->runRepository->retrieveRunJson($plan_id, $timestamp)) {
       return $info;
     }
     return FALSE;
@@ -248,14 +247,14 @@ public function getHarvestRunInfo(string $plan_id, string $run_id): bool|string
    *
    * @param string $plan_id
    *   Harvest plan ID.
-   * @param string $run_id
-   *   Harvest run ID.
+   * @param string $timestamp
+   *   Harvest run timestamp.
    *
    * @return array
    *   Array of status info from the run.
    */
-  public function getHarvestRunResult(string $plan_id, string $run_id): array {
-    if ($entity = $this->runRepository->loadEntity($plan_id, $run_id)) {
+  public function getHarvestRunResult(string $plan_id, string $timestamp): array {
+    if ($entity = $this->runRepository->loadEntity($plan_id, $timestamp)) {
       return $entity->toResult();
     }
     else {
@@ -308,8 +307,6 @@ public function getRunIdsForHarvest(string $plan_id): array {
    */
   public function getLastHarvestRunId(string $plan_id): string {
     $run_ids = $this->runRepository->retrieveAllRunIds($plan_id);
-    rsort($run_ids);
-    // @todo SQL query based on numeric id and plan id, sort for timestamp, rather than PHP sort.
     return reset($run_ids);
   }