[FINNA-2032] LIDO: Fix enriching coordinates (#72)

NatLibFi · Oct 3, 2024 · f9a9698 · f9a9698
1 parent 0a54599
commit f9a9698
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 32 deletions.
diff --git a/src/RecordManager/Finna/Record/Lido.php b/src/RecordManager/Finna/Record/Lido.php
@@ -336,20 +336,21 @@ public function getLocations()
                 if (!empty($placeNode->place->gml)) {
                     return [];
                 }
-                if (empty($placeNode->place) && !empty($placeNode->displayPlace)) {
-                    $subjectLocations = [
-                        ...$subjectLocations,
-                        ...$this->splitLocation((string)$placeNode->displayPlace),
-                    ];
-                    continue;
-                }
+                $hierarchicalLocations = false;
                 foreach ($placeNode->place as $place) {
                     if ($result = $this->getHierarchicalLocations($place)) {
+                        $hierarchicalLocations = true;
                         foreach ($result as $location) {
                             $subjectLocations[] = implode(', ', $location);
                         }
                     }
                 }
+                if (!$hierarchicalLocations && !empty($placeNode->displayPlace)) {
+                    $subjectLocations = [
+                        ...$subjectLocations,
+                        ...$this->splitLocation((string)$placeNode->displayPlace),
+                    ];
+                }
             }
         }
 
@@ -448,9 +449,21 @@ public function getRawTopicIds(): array
      * @return array
      */
     public function getRawGeographicTopicIds(): array
+    {
+        return $this->processPlaceIDElements($this->getPlaceIDElements(false, true));
+    }
+
+    /**
+     * Process an array of placeID elements
+     *
+     * @param array $ids PlaceID elements
+     *
+     * @return array
+     */
+    protected function processPlaceIDElements(array $ids): array
     {
         $result = [];
-        foreach ($this->getPlaceIDElements() as $placeID) {
+        foreach ($ids as $placeID) {
             if (!($id = trim((string)$placeID))) {
                 continue;
             }
@@ -468,48 +481,45 @@ public function getRawGeographicTopicIds(): array
     /**
      * Get all the placeID elements
      *
-     * @param bool $allEvents Return all events for event places
+     * @param bool $allEvents                    Return all events for event places
+     * @param bool $excludePlacesWithCoordinates Exclude places that contain coordinates
      *
      * @return array
      */
-    protected function getPlaceIDElements(bool $allEvents = false): array
+    protected function getPlaceIDElements(bool $allEvents = false, bool $excludePlacesWithCoordinates = false): array
     {
         $result = [];
         foreach ($this->getEventNodes($allEvents ? null : $this->getPlaceEvents()) as $eventNode) {
             foreach ($eventNode->eventPlace as $eventPlace) {
-                foreach ($eventPlace->place->placeID ?? [] as $placeID) {
-                    $result[] = $placeID;
+                if (!$excludePlacesWithCoordinates || empty($eventPlace->place->gml)) {
+                    foreach ($eventPlace->place->placeID ?? [] as $placeID) {
+                        $result[] = $placeID;
+                    }
                 }
             }
         }
         foreach ($this->getSubjectNodes() as $subject) {
             foreach ($subject->subjectPlace as $subjectPlace) {
-                foreach ($subjectPlace->place->placeID ?? [] as $placeID) {
-                    $result[] = $placeID;
-                }
-            }
-        }
-        if ($allEvents) {
-            foreach (
-                $this->doc->lido->descriptiveMetadata->objectIdentificationWrap->repositoryWrap->repositorySet
-                ?? [] as $set
-            ) {
-                foreach ($set->repositoryLocation->placeID ?? [] as $placeID) {
-                    $result[] = $placeID;
+                if (!$excludePlacesWithCoordinates || empty($subjectPlace->place->gml)) {
+                    foreach ($subjectPlace->place->placeID ?? [] as $placeID) {
+                        $result[] = $placeID;
+                    }
                 }
             }
         }
         foreach (
             $this->doc->lido->descriptiveMetadata->objectIdentificationWrap->repositoryWrap->repositorySet
             ?? [] as $set
         ) {
-            foreach ($set->repositoryLocation->placeID ?? [] as $placeID) {
-                $attr = $placeID->attributes();
-                if (in_array($attr->type, $this->includedLocationLabels)) {
-                    $result[] = $placeID;
-                }
-                if ($attr->type == 'URI' && $attr->source == 'YSO') {
-                    $result[] = $placeID;
+            if (!$excludePlacesWithCoordinates || empty($set->repositoryLocation->gml)) {
+                foreach ($set->repositoryLocation->placeID ?? [] as $placeID) {
+                    $attr = $placeID->attributes();
+                    if (
+                        $allEvents || in_array($attr->type, $this->includedLocationLabels)
+                        || ($attr->type == 'URI' && $attr->source == 'YSO')
+                    ) {
+                        $result[] = $placeID;
+                    }
                 }
             }
         }
@@ -792,7 +802,7 @@ protected function getTopicIDs($exclude = ['iconclass']): array
      */
     protected function getGeographicTopicIDs()
     {
-        $result = $this->getRawGeographicTopicIds();
+        $result = $this->processPlaceIDElements($this->getPlaceIDElements());
         return $this->addNamespaceToAuthorityIds($result, 'geographic');
     }
 

diff --git a/tests/RecordManagerTest/Finna/Record/LidoTest.php b/tests/RecordManagerTest/Finna/Record/LidoTest.php
@@ -639,5 +639,13 @@ public function testHierarchicalLocations(): void
             ],
         ];
         $this->compareArray($expected, $result, 'Locations');
+
+        $result = ($record instanceof Lido) ? $record->getRawGeographicTopicIds() : [];
+        $expected = [
+            'http://www.yso.fi/onto/yso/p94191',
+            'http://www.yso.fi/onto/yso/p94124',
+            'http://www.yso.fi/onto/yso/p94137',
+        ];
+        $this->compareArray($expected, $result, 'RawGeographicTopicIds');
     }
 }
diff --git a/tests/fixtures/Finna/record/lido_locations.xml b/tests/fixtures/Finna/record/lido_locations.xml
@@ -7,6 +7,7 @@
         <repositoryWrap>
           <repositorySet type="Current location">
             <repositoryLocation>
+              <placeID type="URI" source="YSO">http://www.yso.fi/onto/yso/p94137</placeID>
               <namePlaceSet>
                 <appellationValue label="tarkempi paikka">Should not display</appellationValue>
               </namePlaceSet>
@@ -34,6 +35,7 @@
               <subjectPlace>
                 <displayPlace lang="fi">Pohjantie, Karjaa, Etelä-Uusimaa, Suomi</displayPlace>
                 <place>
+                  <placeID type="URI" source="YSO">http://www.yso.fi/onto/yso/p94191</placeID>
                   <namePlaceSet>
                     <appellationValue label="katuosoite">Pohjantie</appellationValue>
                   </namePlaceSet>
@@ -66,6 +68,9 @@
             <subject>
               <subjectPlace>
                 <displayPlace>Suomi, Hamina; Hamina</displayPlace>
+                <place>
+                  <placeID>http://www.yso.fi/onto/yso/p94124</placeID>
+                </place>
               </subjectPlace>
             </subject>
           </subjectSet>
@@ -97,6 +102,7 @@
           <subjectSet>
             <subjectPlace>
               <place>
+                <placeID type="URI" source="YSO">http://www.yso.fi/onto/yso/p105096</placeID>
                 <namePlaceSet>
                   <appellationValue><!-- Kuvausmaa -->Suomi</appellationValue>
                 </namePlaceSet>