[FEATURE][BC-BREAK] Add possibility to cache schema IDs by hash (#11)

This is needed because when running serializers the cost of hashing the schemas' string representations is way less than going to the registry over HTTP. This adds 3 new methods to the `CacheAdapter` API: `cacheSchemaIdByHash(int $schemaId, string $schemaHash)` `getIdWithHash(string $hash)` `hasSchemaIdForHash(string $schemaHash)` Unfortunately this is a BC breaking change which requires clients that possibly have own implementations of the `CacheAdapterInterface` to implement those 3 methods. Clients that use the default shipped adapters are OK. Implements the new caching features in the `CachedRegistry`
flix-tech · Oct 26, 2017 · ebf27bd · ebf27bd
1 parent 3e842d0
commit ebf27bd
Show file tree

Hide file tree

Showing 8 changed files with 312 additions and 172 deletions.
diff --git a/src/Registry/Cache/AvroObjectCacheAdapter.php b/src/Registry/Cache/AvroObjectCacheAdapter.php
@@ -17,6 +17,11 @@ class AvroObjectCacheAdapter implements CacheAdapter
      */
     private $idToSchema = [];
 
+    /**
+     * @var int[]
+     */
+    private $hashToSchemaId = [];
+
     /**
      * @var AvroSchema[]
      */
@@ -30,6 +35,14 @@ public function cacheSchemaWithId(AvroSchema $schema, int $schemaId)
         $this->idToSchema[$schemaId] = $schema;
     }
 
+    /**
+     * {@inheritdoc}
+     */
+    public function cacheSchemaIdByHash(int $schemaId, string $schemaHash)
+    {
+        $this->hashToSchemaId[$schemaHash] = $schemaId;
+    }
+
     /**
      * {@inheritdoc}
      */
@@ -50,6 +63,15 @@ public function getWithId(int $schemaId)
         return $this->idToSchema[$schemaId];
     }
 
+    public function getIdWithHash(string $hash)
+    {
+        if (!$this->hasSchemaIdForHash($hash)) {
+            return null;
+        }
+
+        return $this->hashToSchemaId[$hash];
+    }
+
     /**
      * {@inheritdoc}
      */
@@ -72,6 +94,14 @@ public function hasSchemaForId(int $schemaId): bool
         return array_key_exists($schemaId, $this->idToSchema);
     }
 
+    /**
+     * {@inheritdoc}
+     */
+    public function hasSchemaIdForHash(string $schemaHash): bool
+    {
+        return array_key_exists($schemaHash, $this->hashToSchemaId);
+    }
+
     /**
      * {@inheritdoc}
      */

diff --git a/src/Registry/Cache/DoctrineCacheAdapter.php b/src/Registry/Cache/DoctrineCacheAdapter.php
@@ -31,6 +31,11 @@ public function cacheSchemaWithId(AvroSchema $schema, int $schemaId)
         $this->doctrineCache->save($schemaId, (string) $schema);
     }
 
+    public function cacheSchemaIdByHash(int $schemaId, string $schemaHash)
+    {
+        $this->doctrineCache->save($schemaHash, $schemaId);
+    }
+
     /**
      * {@inheritdoc}
      */
@@ -56,6 +61,20 @@ public function getWithId(int $schemaId)
         return AvroSchema::parse($rawSchema);
     }
 
+    /**
+     * {@inheritdoc}
+     */
+    public function getIdWithHash(string $hash)
+    {
+        $schemaId = $this->doctrineCache->fetch($hash);
+
+        if (!$schemaId) {
+            return null;
+        }
+
+        return $schemaId;
+    }
+
     /**
      * {@inheritdoc}
      */
@@ -82,6 +101,14 @@ public function hasSchemaForId(int $schemaId): bool
         return $this->doctrineCache->contains($schemaId);
     }
 
+    /**
+     * {@inheritdoc}
+     */
+    public function hasSchemaIdForHash(string $schemaHash): bool
+    {
+        return $this->doctrineCache->contains($schemaHash);
+    }
+
     /**
      * {@inheritdoc}
      */

diff --git a/src/Registry/CacheAdapter.php b/src/Registry/CacheAdapter.php
@@ -32,6 +32,16 @@ public function cacheSchemaWithId(AvroSchema $schema, int $schemaId);
      */
     public function cacheSchemaWithSubjectAndVersion(AvroSchema $schema, string $subject, int $version);
 
+    /**
+     * Caches a schema id by a hash (i.e. the hash of the Avro schema string representation)
+     *
+     * @param int $schemaId
+     * @param string $schemaHash
+     *
+     * @return void
+     */
+    public function cacheSchemaIdByHash(int $schemaId, string $schemaHash);
+
     /**
      * Tries to fetch a cache with the global schema id.
      * Returns either the AvroSchema when found or `null` when not.
@@ -42,6 +52,16 @@ public function cacheSchemaWithSubjectAndVersion(AvroSchema $schema, string $sub
      */
     public function getWithId(int $schemaId);
 
+    /**
+     * Tries to fetch a cached schema id with a given hash.
+     * Either returns the schema id as int or `null` when none is found.
+     *
+     * @param string $hash
+     *
+     * @return int|null
+     */
+    public function getIdWithHash(string $hash);
+
     /**
      * Tries to fetch a cache with a given subject and version.
      * Returns either the AvroSchema when found or `null` when not.
@@ -62,6 +82,15 @@ public function getWithSubjectAndVersion(string $subject, int $version);
      */
     public function hasSchemaForId(int $schemaId): bool;
 
+    /**
+     * Checks if a schema id exists for the given hash
+     *
+     * @param string $schemaHash
+     *
+     * @return bool
+     */
+    public function hasSchemaIdForHash(string $schemaHash): bool;
+
     /**
      * Checks if the cache engine has a cached schema for a given subject and version.
      *

diff --git a/src/Registry/CachedRegistry.php b/src/Registry/CachedRegistry.php
@@ -23,10 +23,23 @@ class CachedRegistry implements Registry
      */
     private $cacheAdapter;
 
-    public function __construct(Registry $registry, CacheAdapter $cacheAdapter)
+    /**
+     * @var callable
+     */
+    private $hashAlgoFunc;
+
+    public function __construct(Registry $registry, CacheAdapter $cacheAdapter, callable $hashAlgoFunc = null)
     {
         $this->registry = $registry;
         $this->cacheAdapter = $cacheAdapter;
+
+        if (!$hashAlgoFunc) {
+            $hashAlgoFunc = function (AvroSchema $schema) {
+                return md5((string) $schema);
+            };
+        }
+
+        $this->hashAlgoFunc = $hashAlgoFunc;
     }
 
     /**
@@ -36,6 +49,7 @@ public function register(string $subject, AvroSchema $schema, callable $requestC
     {
         $closure = function (int $schemaId) use ($schema) {
             $this->cacheAdapter->cacheSchemaWithId($schema, $schemaId);
+            $this->cacheAdapter->cacheSchemaIdByHash($schemaId, $this->getSchemaHash($schema));
 
             return $schemaId;
         };
@@ -74,8 +88,15 @@ function (PromiseInterface $promise) use ($closure) {
      */
     public function schemaId(string $subject, AvroSchema $schema, callable $requestCallback = null)
     {
-        $closure = function (int $schemaId) use ($schema) {
+        $schemaHash = $this->getSchemaHash($schema);
+
+        if ($this->cacheAdapter->hasSchemaIdForHash($schemaHash)) {
+            return $this->cacheAdapter->getIdWithHash($schemaHash);
+        }
+
+        $closure = function (int $schemaId) use ($schema, $schemaHash) {
             $this->cacheAdapter->cacheSchemaWithId($schema, $schemaId);
+            $this->cacheAdapter->cacheSchemaIdByHash($schemaId, $schemaHash);
 
             return $schemaId;
         };
@@ -100,6 +121,7 @@ public function schemaForId(int $schemaId, callable $requestCallback = null)
 
         $closure = function (AvroSchema $schema) use ($schemaId) {
             $this->cacheAdapter->cacheSchemaWithId($schema, $schemaId);
+            $this->cacheAdapter->cacheSchemaIdByHash($schemaId, $this->getSchemaHash($schema));
 
             return $schema;
         };
@@ -159,4 +181,9 @@ private function applyValueHandlers($value, callable $promiseHandler, callable $
 
         return $valueHandler($value);
     }
+
+    private function getSchemaHash(AvroSchema $schema): string
+    {
+        return call_user_func($this->hashAlgoFunc, $schema);
+    }
 }
diff --git a/test/Registry/Cache/AbstractCacheAdapterTestCase.php b/test/Registry/Cache/AbstractCacheAdapterTestCase.php
@@ -0,0 +1,82 @@
+<?php
+
+declare(strict_types=1);
+
+namespace FlixTech\SchemaRegistryApi\Test\Registry\Cache;
+
+use AvroSchema;
+use FlixTech\SchemaRegistryApi\Registry\CacheAdapter;
+use PHPUnit\Framework\TestCase;
+
+abstract class AbstractCacheAdapterTestCase extends TestCase
+{
+    abstract protected function getAdapter(): CacheAdapter;
+
+    /**
+     * @var CacheAdapter
+     */
+    protected $cacheAdapter;
+
+    protected function setUp()
+    {
+        $this->cacheAdapter = $this->getAdapter();
+    }
+
+    /**
+     * @test
+     */
+    public function it_should_store_and_fetch_schemas_with_ids()
+    {
+        $schemaId = 3;
+        $invalidSchemaId = 2;
+        $schema = AvroSchema::parse('{"type": "string"}');
+
+        $this->cacheAdapter->cacheSchemaWithId($schema, $schemaId);
+
+        $this->assertFalse($this->cacheAdapter->hasSchemaForId($invalidSchemaId));
+        $this->assertTrue($this->cacheAdapter->hasSchemaForId($schemaId));
+
+        $this->assertNull($this->cacheAdapter->getWithId($invalidSchemaId));
+        $this->assertEquals($schema, $this->cacheAdapter->getWithId($schemaId));
+    }
+
+    /**
+     * @test
+     */
+    public function it_should_store_and_fetch_schemas_with_subject_and_version()
+    {
+        $subject = 'test';
+        $version = 2;
+        $invalidSubject = 'none';
+        $schema = AvroSchema::parse('{"type": "string"}');
+
+        $this->cacheAdapter->cacheSchemaWithSubjectAndVersion($schema, $subject, $version);
+
+        $this->assertFalse($this->cacheAdapter->hasSchemaForSubjectAndVersion($invalidSubject, $version));
+        $this->assertTrue($this->cacheAdapter->hasSchemaForSubjectAndVersion($subject, $version));
+
+        $this->assertNull($this->cacheAdapter->getWithSubjectAndVersion($invalidSubject, $version));
+        $this->assertEquals($schema, $this->cacheAdapter->getWithSubjectAndVersion($subject, $version));
+    }
+
+    /**
+     * @test
+     */
+    public function it_should_store_and_fetch_schema_ids_with_schema_hashes()
+    {
+        $schemaId = 3;
+        $hash = 'hash';
+        $anotherHash = 'another';
+
+        $this->assertFalse($this->cacheAdapter->hasSchemaIdForHash($hash));
+        $this->assertFalse($this->cacheAdapter->hasSchemaIdForHash($anotherHash));
+
+        $this->cacheAdapter->cacheSchemaIdByHash($schemaId, $hash);
+
+        $this->assertTrue($this->cacheAdapter->hasSchemaIdForHash($hash));
+        $this->assertFalse($this->cacheAdapter->hasSchemaIdForHash($anotherHash));
+
+        $this->assertNull($this->cacheAdapter->getIdWithHash($anotherHash));
+        $this->assertSame($schemaId, $this->cacheAdapter->getIdWithHash($hash));
+    }
+}
diff --git a/test/Registry/Cache/AvroObjectCacheAdapterTest.php b/test/Registry/Cache/AvroObjectCacheAdapterTest.php
@@ -4,56 +4,13 @@
 
 namespace FlixTech\SchemaRegistryApi\Test\Registry\Cache;
 
-use AvroSchema;
 use FlixTech\SchemaRegistryApi\Registry\Cache\AvroObjectCacheAdapter;
-use PHPUnit\Framework\TestCase;
+use FlixTech\SchemaRegistryApi\Registry\CacheAdapter;
 
-class AvroObjectCacheAdapterTest extends TestCase
+class AvroObjectCacheAdapterTest extends AbstractCacheAdapterTestCase
 {
-    /**
-     * @var \FlixTech\SchemaRegistryApi\Registry\Cache\AvroObjectCacheAdapter
-     */
-    private $cacheAdapter;
-
-    protected function setUp()
-    {
-        $this->cacheAdapter = new AvroObjectCacheAdapter();
-    }
-
-    /**
-     * @test
-     */
-    public function it_should_store_and_fetch_schemas_with_ids()
+    protected function getAdapter(): CacheAdapter
     {
-        $schemaId = 3;
-        $invalidSchemaId = 2;
-        $schema = AvroSchema::parse('{"type": "string"}');
-
-        $this->cacheAdapter->cacheSchemaWithId($schema, $schemaId);
-
-        $this->assertFalse($this->cacheAdapter->hasSchemaForId($invalidSchemaId));
-        $this->assertTrue($this->cacheAdapter->hasSchemaForId($schemaId));
-
-        $this->assertNull($this->cacheAdapter->getWithId($invalidSchemaId));
-        $this->assertEquals($schema, $this->cacheAdapter->getWithId($schemaId));
-    }
-
-    /**
-     * @test
-     */
-    public function it_should_store_and_fetch_schemas_with_subject_and_version()
-    {
-        $subject = 'test';
-        $version = 2;
-        $invalidSubject = 'none';
-        $schema = AvroSchema::parse('{"type": "string"}');
-
-        $this->cacheAdapter->cacheSchemaWithSubjectAndVersion($schema, $subject, $version);
-
-        $this->assertFalse($this->cacheAdapter->hasSchemaForSubjectAndVersion($invalidSubject, $version));
-        $this->assertTrue($this->cacheAdapter->hasSchemaForSubjectAndVersion($subject, $version));
-
-        $this->assertNull($this->cacheAdapter->getWithSubjectAndVersion($invalidSubject, $version));
-        $this->assertEquals($schema, $this->cacheAdapter->getWithSubjectAndVersion($subject, $version));
+        return new AvroObjectCacheAdapter();
     }
 }