From d0042c6463bd34c927d254c4b4ef33dd8fcbc0c4 Mon Sep 17 00:00:00 2001
From: Paul Mitchum <paul@mile23.com>
Date: Fri, 29 Mar 2024 09:34:59 -0700
Subject: [PATCH] Fix hash entity update fails (#4152)

---
 modules/harvest/src/Entity/HarvestHash.php    |  8 +++-
 .../HarvestHashesEntityDatabaseTable.php      | 38 ++++++++++++-------
 2 files changed, 30 insertions(+), 16 deletions(-)

diff --git a/modules/harvest/src/Entity/HarvestHash.php b/modules/harvest/src/Entity/HarvestHash.php
index 05bcbc723a..41e95cabb4 100644
--- a/modules/harvest/src/Entity/HarvestHash.php
+++ b/modules/harvest/src/Entity/HarvestHash.php
@@ -32,7 +32,7 @@
  *   },
  *   base_table = "harvest_hashes",
  *   entity_keys = {
- *     "id" = "data_uuid",
+ *     "id" = "id",
  *   },
  *   internal = TRUE,
  * )
@@ -45,10 +45,14 @@ class HarvestHash extends ContentEntityBase implements HarvestHashInterface {
    * {@inheritdoc}
    */
   public static function baseFieldDefinitions($entity_type) {
+    // Parent will give us an 'id' field because we annotated it as the id key.
+    $fields = parent::baseFieldDefinitions($entity_type);
+
     // Data node UUID. This is a UUID to a Data node, probably of type
     // 'dataset'.
+    // Note that this is a UUID by convention, and could be any string.
     // @todo Add uuid constraint.
-    $fields['data_uuid'] = BaseFieldDefinition::create('uuid')
+    $fields['data_uuid'] = BaseFieldDefinition::create('string')
       ->setLabel(t('Data node UUID'))
       ->setDescription(t('The Data node UUID.'))
       ->setReadOnly(FALSE)
diff --git a/modules/harvest/src/Storage/HarvestHashesEntityDatabaseTable.php b/modules/harvest/src/Storage/HarvestHashesEntityDatabaseTable.php
index 8fd1b231c8..dbde79dbef 100644
--- a/modules/harvest/src/Storage/HarvestHashesEntityDatabaseTable.php
+++ b/modules/harvest/src/Storage/HarvestHashesEntityDatabaseTable.php
@@ -113,7 +113,7 @@ public function store($data, string $id = NULL) : string {
       ]);
     }
     $entity->save();
-    return $entity->get($this->primaryKey())->getString();
+    return $entity->get('data_uuid')->getString();
   }
 
   /**
@@ -150,7 +150,9 @@ public function remove(string $id) {
       ->execute()
     ) {
       $entity_id = reset($ids);
-      $this->entityStorage->delete([$this->loadEntity($entity_id)]);
+      $this->entityStorage->delete([
+        $this->entityStorage->load($entity_id),
+      ]);
     }
     return $id;
   }
@@ -159,16 +161,23 @@ public function remove(string $id) {
    * {@inheritDoc}
    */
   public function retrieveAll(): array {
-    // Some calling code is very particular about the output being an array,
-    // both as a return value here and after json_encode(). Since the entity
-    // query returns a keyed array, json_encode() will think it's an object. We
-    // don't want that, so we use array_values() to yield an indexed array.
-    return array_values(
+    $data_uuids = [];
+    $entities = $this->entityStorage->loadMultiple(
       $this->entityStorage->getQuery()
         ->condition('harvest_plan_id', $this->planId)
         ->accessCheck(FALSE)
         ->execute()
     );
+    /** @var \Drupal\harvest\HarvestHashInterface  $entity*/
+    foreach ($entities as $entity) {
+      $uuid = $entity->get('data_uuid')->getString();
+      $data_uuids[$uuid] = $uuid;
+    }
+    // Some calling code is very particular about the output being an array,
+    // both as a return value here and after json_encode(). Since we're using a
+    // keyed array, json_encode() will think it's an object. We don't want that,
+    // so we use array_values() to yield an indexed array.
+    return array_values($data_uuids);
   }
 
   /**
@@ -205,9 +214,10 @@ public function count(): int {
    * {@inheritDoc}
    */
   public function primaryKey() {
-    // Use the primary key defined in the entity definition.
-    $definition = $this->entityTypeManager->getDefinition(static::ENTITY_TYPE);
-    return ($definition->getKeys())['id'];
+    // The primary key for entity API is 'id'. But the primary key for the
+    // database table interface is 'data_uuid'. This is mostly arbitrary for our
+    // purposes because we're not actually subclassing AbstractDatabaseTable.
+    return 'data_uuid';
   }
 
   /**
@@ -241,18 +251,18 @@ public function getSchema(): array {
   /**
    * Helper method to load an entity given an ID.
    *
-   * @param string $id
+   * @param string $data_uuid
    *   Entity ID.
    *
    * @return \Drupal\harvest\HarvestHashInterface|null
    *   The loaded entity or NULL if none could be loaded.
    */
-  protected function loadEntity(string $id): ?HarvestHashInterface {
-    if (!$id) {
+  protected function loadEntity(string $data_uuid): ?HarvestHashInterface {
+    if (!$data_uuid) {
       return NULL;
     }
     if ($ids = $this->entityStorage->getQuery()
-      ->condition($this->primaryKey(), $id)
+      ->condition('data_uuid', $data_uuid)
       ->condition('harvest_plan_id', $this->planId)
       ->range(0, 1)
       ->accessCheck(FALSE)