Improve handling of Pipeline deletes (NOJIRA)

app/availableplugins/ApiConnector/src/Model/Table/ApiSourcesTable.php

@@ -307,12 +307,18 @@ public function retrieve(
                                                 'api_source_id' => $source->api_source->id,
                                                 'source_key'    => $source_key
                                               ])
-                                              ->firstOrFail();
+                                              ->first();
 
-    $ret['source_record'] = $apiSourceRecord->source_record;
-    $ret['entity_data'] = $this->resultToEntityData(
-      json_decode(json: $apiSourceRecord->source_record, associative: true)
-    );
+    if(!$apiSourceRecord) {
+      // Record was deleted
+      $ret['source_record'] = null;
+      $ret['entity_data'] = null;
+    } else {
+      $ret['source_record'] = $apiSourceRecord->source_record;
+      $ret['entity_data'] = $this->resultToEntityData(
+        json_decode(json: $apiSourceRecord->source_record, associative: true)
+      );
+    }
 
     return $ret;
   }

app/availableplugins/FileConnector/src/Model/Table/FileSourcesTable.php

@@ -373,6 +373,10 @@ public function retrieve(
     // The first line of a CSV v3 file is our configuration
     fgetcsv($handle);
 
+    // Set null defaults in case we don't find a matching record
+    $ret['source_record'] = null;
+    $ret['entity_data'] = null;
+
     while(($data = fgetcsv($handle)) !== false) {
       if($data[0] == $source_key) {
         // This is our record

app/availableplugins/SqlConnector/src/Model/Table/SqlSourcesTable.php

@@ -662,8 +662,14 @@ public function retrieve(
                               // multi-role support in flat mode
                               ->all();
 
-        $ret['entity_data'] = $this->resultsToEntityData($source->sql_source, $results->toArray());
-        $ret['source_record'] = json_encode($results);
+        if($results->count()==0) {
+          // Record was probably deleted
+          $ret['entity_data'] = null;
+          $ret['source_record'] = null;
+        } else {
+          $ret['entity_data'] = $this->resultsToEntityData($source->sql_source, $results->toArray());
+          $ret['source_record'] = json_encode($results);
+        }
       }
       catch(\Exception $e) {
         throw new \InvalidArgumentException(__d('error', 'notfound', [$source_key]));
@@ -690,6 +696,14 @@ public function retrieve(
         throw new \InvalidArgumentException(__d('error', 'notfound', [$source_key]));
       }
 
+      if($results->count()==0) {
+        // Record was probably deleted, so just return
+        $ret['entity_data'] = null;
+        $ret['source_record'] = null;
+
+        return $ret;
+      }
+
       // From here on out if a table doesn't exist we simply ignore it.
 
       // We pull roles before the MVEAs because we'll manually process each MVEA record,

app/src/Model/Table/ExternalIdentitySourcesTable.php

@@ -231,13 +231,16 @@ public function retrieve(int $id, string $sourceKey): array {
 
     $record = $this->$pModel->retrieve($source, $sourceKey);
 
-    // Inject the source key so every backend doesn't have to do this
-    $record['entity_data']['source_key'] = $sourceKey;
-
-    $record['entity_data']['identifiers'][] = [
-      'identifier' => $sourceKey,
-      'type' => 'sorid'
-    ];
+    if($record['entity_data']) {
+      // Inject the source key so every backend doesn't have to do this,
+      // but only if the backend returned a record.
+      $record['entity_data']['source_key'] = $sourceKey;
+
+      $record['entity_data']['identifiers'][] = [
+        'identifier' => $sourceKey,
+        'type' => 'sorid'
+      ];
+    }
 
     return $record;
   }

app/src/Model/Table/PipelinesTable.php

@@ -541,7 +541,7 @@ public function execute(
       // (4) Sync the External Identity attributes with the Person record
       $person = $this->syncPerson(
         $pipeline,
-        $externalIdentity->id,
+        isset($externalIdentity->id) ? $externalIdentity->id : null,
         $person
       );
 
@@ -614,13 +614,20 @@ protected function manageEISRecord(
     Pipeline                $pipeline, 
     ExternalIdentitySource  $eis,
     string                  $sourceKey,
-    string                  $sourceRecord,
+    ?string                 $sourceRecord,
   ): array {
     $status = 'unknown';
 
     // Are we supposed to use record hashes instead?
     $useHash = isset($eis->hash_source_record) && $eis->hash_source_record;
 
+    // We calculate the hash here to simplify the code below
+    $sourceHash = null;
+
+    if($useHash && !empty($sourceRecord)) {
+      $sourceHash = md5($sourceRecord);
+    }
+
     // Do we already have an EISRecord for this source_key?
     $eisRecord = $this->ExternalIdentitySources->ExtIdentitySourceRecords
                       ->find()
@@ -644,13 +651,17 @@ protected function manageEISRecord(
              // stored as an md5 hash. (This does mean the first time we sync
              // a record after hash_source_record is enabled we'll reprocess it
              // even if nothing changed.)
-             && (($useHash && ($eisRecord->source_record != md5($sourceRecord)))
+             && (($useHash && ($eisRecord->source_record != $sourceHash))
                  || (!$useHash && ($eisRecord->source_record != $sourceRecord))))) {
         // We have an update of some form or another, including, possibly, a delete
 
         $this->llog('trace', "Updating Record for EIS " . $eis->description . " (" . $eis->id . ") source key $sourceKey");
 
-        $eisRecord->source_record = $useHash ? md5($sourceRecord) : $sourceRecord;
+        if(empty($sourceRecord)) {
+          $this->llog('trace', "Record for EIS " . $eis->description . " (" . $eis->id . ") source key $sourceKey has been deleted");
+        }
+
+        $eisRecord->source_record = $useHash ? $sourceHash : $sourceRecord;
         $eisRecord->last_update = date('Y-m-d H:i:s', time());
 
         $status = 'updated';
@@ -667,7 +678,7 @@ protected function manageEISRecord(
                         ->newEntity([
                           'external_identity_source_id' => $eis->id,
                           'source_key'                  => $sourceKey,
-                          'source_record'               => $useHash ? md5($sourceRecord) : $sourceRecord,
+                          'source_record'               => $useHash ? $sourceHash : $sourceRecord,
                           'last_update'                 => date('Y-m-d H:i:s', time())
                         ]);
 
@@ -696,8 +707,13 @@ protected function manageEISRecord(
 
   protected function mapAttributesToCO(
     Pipeline          $pipeline,
-    array             $eisAttributes
+    ?array            $eisAttributes
   ): array {
+    // Check if we'er handling a deleted record
+    if(empty($eisAttributes)) {
+      return [];
+    }
+
     // We explicitly list the valid models, which will effectively filter
     // out any unsupported noise from the backend. (Unsupported attributes
     // will be ignored on save or throw errors.)
@@ -886,7 +902,7 @@ protected function obtainPerson(
     Pipeline                $pipeline,
     ExternalIdentitySource  $eis,
     ExtIdentitySourceRecord $eisRecord,
-    array                   $eisAttributes
+    ?array                  $eisAttributes
   ): array {
     // Shorthand...
     $sourceKey = $eisRecord->source_key;
@@ -902,6 +918,12 @@ protected function obtainPerson(
       ];
     }
 
+    if(empty($eisAttributes)) {
+      // We shouldn't get here since attributes should only be null on a delete,
+      // which should only happen for previously processed records.
+      throw new \RuntimeException('$eisAttributes unexpectedly empty in Pipeline::obtainPerson');
+    }
+
     // There isn't a Person associated with the request, run the configured
     // Match Strategy to see if one exists
 
@@ -1054,8 +1076,8 @@ protected function syncExternalIdentity(
     Person                  $person,
     ExternalIdentitySource  $eis,
     ExtIdentitySourceRecord $eisRecord,
-    array                   $eisAttributes
-  ): ExternalIdentity {
+    ?array                  $eisAttributes
+  ): ?ExternalIdentity {
     if(empty($eisRecord->external_identity_id)) {
       $this->llog('trace', "Creating new External Identity for Person " . $person->id . " from EIS " . $eis->description . " (" . $eis->id . ")");
 
@@ -1120,6 +1142,17 @@ protected function syncExternalIdentity(
       );
 
       return $entity;
+    } elseif(empty($eisAttributes)) {
+      // We're deleting the full External Identity, which we'll do using cascading
+      // deletes (and to trigger ExternalIdentitiesTable callbacks) rather than
+      // do it model by model, below.
+
+      $this->llog('trace', "Deleting removed External Identity " . $eisRecord->external_identity_id . " for Person " . $person->id . " from EIS " . $eis->description . " (" . $eis->id . ")");
+
+      $entity = $this->Cos->People->ExternalIdentities->get($eisRecord->external_identity_id);
+      $this->Cos->People->ExternalIdentities->deleteOrFail($entity);
+
+      return null;
     } else {
       $this->llog('trace', "Updating existing External Identity " . $eisRecord->external_identity_id . " for Person " . $person->id . " from EIS " . $eis->description . " (" . $eis->id . ")");
 
@@ -1441,29 +1474,35 @@ protected function syncExternalIdentity(
 
   protected function syncPerson(
     Pipeline          $pipeline,
-    int               $externalIdentityId,
+    ?int              $externalIdentityId,
     Person            $person
   ): Person {
     // We re-pull the External Identity to account for any changes that might have
-    // been processed by syncExternalIdentity.
-    $externalIdentity = $this->Cos->People->ExternalIdentities->get(
-      $externalIdentityId,
-      ['contain' => [
-        'Addresses',
-        'AdHocAttributes',
-        'EmailAddresses',
-        'Identifiers',
-        'Names',
-        'Pronouns',
-        'TelephoneNumbers',
-        'Urls',
-        'ExternalIdentityRoles' => [
+    // been processed by syncExternalIdentity. Note if the ID is null, the External
+    // Identity was deleted.
+
+    if($externalIdentityId) {
+      $externalIdentity = $this->Cos->People->ExternalIdentities->get(
+        $externalIdentityId,
+        ['contain' => [
+          'Addresses',
           'AdHocAttributes',
-          'Addresses', 
-          'TelephoneNumbers'
-        ]
-      ]]
-    );
+          'EmailAddresses',
+          'Identifiers',
+          'Names',
+          'Pronouns',
+          'TelephoneNumbers',
+          'Urls',
+          'ExternalIdentityRoles' => [
+            'AdHocAttributes',
+            'Addresses', 
+            'TelephoneNumbers'
+          ]
+        ]]
+      );
+    } else {
+      $externalIdentity = null;
+    }
 
     // Because ExternalIdentities belongTo People, we can assume we have at least
     // a Person object here (it would have been created by obtainPerson if there

app/templates/ExternalIdentitySources/retrieve.php

@@ -130,6 +130,7 @@
     </table>
 
     <h3><?= __d('controller', 'ExternalIdentities', 1) ?></h3>
+    <?php if($vv_eis_record['entity_data'] != null): ?>
     <table id="view-external-identity-source-record" class="eis-table">
       <thead>
         <tr>
@@ -139,7 +140,6 @@
         </tr>
       </thead>
       <tbody>
-
         <!-- We order attributes according to their likely importance,
              starting with names. Because $vv_eis_record is an array and
              not an entity, we can't use entity methods to get virtual fields
@@ -209,7 +209,7 @@
         ?>
       </tbody>
     </table>
-
+    <?php endif; // !empty(entity_data) ?>
     <?php 
       if(!empty($vv_eis_record['entity_data']['external_identity_roles'])):
         foreach($vv_eis_record['entity_data']['external_identity_roles'] as $role):