Merge pull request #281 from norkunas/skip-geocoding

Skip geocoding if address has not changed

Author: norkunas

Doctrine/ORM/GeocoderListener.php

@@ -17,6 +17,7 @@
 use Doctrine\Common\EventSubscriber;
 use Doctrine\ORM\Event\OnFlushEventArgs;
 use Doctrine\ORM\Events;
+use Doctrine\ORM\UnitOfWork;
 use Geocoder\Provider\Provider;
 use Geocoder\Query\GeocodeQuery;
 
@@ -61,7 +62,10 @@ public function onFlush(OnFlushEventArgs $args)
                 continue;
             }
 
-            $this->geocodeEntity($entity);
+            /** @var ClassMetadata $metadata */
+            $metadata = $this->driver->loadMetadataFromObject($entity);
+
+            $this->geocodeEntity($metadata, $entity);
 
             $uow->recomputeSingleEntityChangeSet(
                 $em->getClassMetadata(get_class($entity)),
@@ -74,7 +78,14 @@ public function onFlush(OnFlushEventArgs $args)
                 continue;
             }
 
-            $this->geocodeEntity($entity);
+            /** @var ClassMetadata $metadata */
+            $metadata = $this->driver->loadMetadataFromObject($entity);
+
+            if (!$this->shouldGeocode($metadata, $uow, $entity)) {
+                continue;
+            }
+
+            $this->geocodeEntity($metadata, $entity);
 
             $uow->recomputeSingleEntityChangeSet(
                 $em->getClassMetadata(get_class($entity)),
@@ -83,11 +94,11 @@ public function onFlush(OnFlushEventArgs $args)
         }
     }
 
-    private function geocodeEntity($entity)
+    /**
+     * @param object $entity
+     */
+    private function geocodeEntity(ClassMetadata $metadata, $entity)
     {
-        /** @var ClassMetadata $metadata */
-        $metadata = $this->driver->loadMetadataFromObject($entity);
-
         if (null !== $metadata->addressGetter) {
             $address = $metadata->addressGetter->invoke($entity);
         } else {
@@ -106,4 +117,18 @@ private function geocodeEntity($entity)
             $metadata->longitudeProperty->setValue($entity, $result->getCoordinates()->getLongitude());
         }
     }
+
+    /**
+     * @param object $entity
+     */
+    private function shouldGeocode(ClassMetadata $metadata, UnitOfWork $unitOfWork, $entity): bool
+    {
+        if (null !== $metadata->addressGetter) {
+            return true;
+        }
+
+        $changeSet = $unitOfWork->getEntityChangeSet($entity);
+
+        return isset($changeSet[$metadata->addressProperty->getName()]);
+    }
 }

Tests/Doctrine/ORM/GeocoderListenerTest.php

@@ -18,11 +18,14 @@
 use Doctrine\Common\Annotations\SimpleAnnotationReader;
 use Doctrine\DBAL\DriverManager;
 use Doctrine\ORM\EntityManager;
+use Doctrine\ORM\Events;
 use Doctrine\ORM\Tools\SchemaTool;
 use Doctrine\Tests\DoctrineTestCase;
 use Doctrine\Tests\OrmTestCase;
 use Geocoder\Provider\Nominatim\Nominatim;
 use Http\Client\Curl\Client;
+use Psr\Http\Message\RequestInterface;
+use Psr\Http\Message\ResponseInterface;
 use Symfony\Bridge\PhpUnit\SetUpTearDownTrait;
 
 /**
@@ -148,6 +151,39 @@ public function testPersistForEmptyProperty()
         $this->assertNull($dummy->latitude);
         $this->assertNull($dummy->longitude);
     }
+
+    public function testDoesNotGeocodeIfAddressNotChanged()
+    {
+        $this->em->getEventManager()->removeEventListener(Events::onFlush, $this->listener);
+
+        $reader = new SimpleAnnotationReader();
+        $reader->addNamespace('Bazinga\GeocoderBundle\Mapping\Annotations');
+        $reader->addNamespace('Doctrine\ORM\Mapping');
+
+        $driver = new AnnotationDriver($reader);
+
+        $client = new TrackedCurlClient();
+        $geocoder = Nominatim::withOpenStreetMapServer($client, 'BazingaGeocoderBundle/Test');
+        $listener = new GeocoderListener($geocoder, $driver);
+
+        $this->em->getEventManager()->addEventSubscriber($listener);
+
+        $dummy = new DummyWithProperty();
+        $dummy->address = 'Frankfurt, Germany';
+
+        $this->em->persist($dummy);
+        $this->em->flush();
+
+        $dummy->latitude = 0;
+        $dummy->longitude = 0;
+
+        $this->em->flush();
+
+        $this->assertSame('Frankfurt, Germany', $dummy->address);
+        $this->assertSame(0, $dummy->latitude);
+        $this->assertSame(0, $dummy->longitude);
+        $this->assertCount(1, $client->getResponses());
+    }
 }
 
 /**
@@ -337,3 +373,18 @@ class DummyWithEmptyProperty
      */
     public $address;
 }
+
+class TrackedCurlClient extends Client
+{
+    private $responses = [];
+
+    public function sendRequest(RequestInterface $request): ResponseInterface
+    {
+        return $this->responses[] = parent::sendRequest($request);
+    }
+
+    public function getResponses(): array
+    {
+        return $this->responses;
+    }
+}