Skip to content

Conversation

jpedryc
Copy link

@jpedryc jpedryc commented Dec 6, 2024

TLDR

Memoization for referencable schemas to limit required instance builds.

Important

Related to discussion #29.


User benefit

Although the majority of declared schema properties are trivial:

public function build(): SchemaContract
{
    return Schema::object('ExampleObject')
        ->properties(
             Schema::string('example-string-prop')->nullable(),
             Schema::integer('example-id')->example(123),
             // ...
        )
}

sometimes more resource demanding actions are being taken, e.g. model-querying to get example values:

Schema::integer('status_id')
    ->enum(Status::pluck('id'))

Note

Another examples are:

  • querying schema descriptions
  • creating custom Collections for ...->enum(...)s
  • accessing some other third-party services to achieve e.g. one of the above

Now, assuming we have a bit more popular Schema that is loaded in multiple other Schemas, e.g. a UserSchema, that is being also referenced with the same objectId (UserSchema::ref('user')):

// Parent A
class ParentASchema extends SchemaFactory implements Reusable
{
    public function build(): SchemaContract
    {
        return Schema::object('ParentA')
            ->properties(
                // ...
                UserSchema::ref('user'),
            );
    }
}

// Parent B
class ParentBSchema extends SchemaFactory implements Reusable
{
    public function build(): SchemaContract
    {
        return Schema::object('ParentB')
            ->properties(
                // ...
                UserSchema::ref('user'),
            );
    }
}

⚠️ The schema-component will be build every time per reference ⚠️

Implementation Overview

diff --git a/src/Concerns/Referencable.php b/src/Concerns/Referencable.php
index fd36525..4dfb8fe 100644
--- a/src/Concerns/Referencable.php
+++ b/src/Concerns/Referencable.php
@@ -14,6 +14,8 @@ use Vyuldashev\LaravelOpenApi\Factories\SecuritySchemeFactory;
 
 trait Referencable
 {
+    static $buildReferencableSchemas = [];
+
     public static function ref(?string $objectId = null): Schema
     {
         $instance = app(static::class);
@@ -38,6 +40,6 @@ trait Referencable
             $baseRef = '#/components/securitySchemes/';
         }
 
-        return Schema::ref($baseRef.$instance->build()->objectId, $objectId);
+        return self::$buildReferencableSchemas[static::class . "_{$objectId}"] ??= Schema::ref($baseRef.$instance->build()->objectId, $objectId);
     }
 }

To make sure we're not re-building schema components of the same class and with the same object ID - we make use of the Memoization Technique.

Test Overview

class MemoizationTest extends TestCase
{
    public function testReferencableInstanceBuildOnce(): void
    {
        self::assertEquals($e1ObjectId = (new ExampleSchema)::ref('e1'), (new ExampleSchema)::ref('e1'));
        self::assertNotEquals($e1ObjectId, (new ExampleSchema)::ref('e2'));
    }
}

We test if objects of the same class and object ID are truly equal. Additionally - we make sure that the object ID matters in the second assertion.

Schema components of the same class and object ID won't be build every
time from scratch which improves performance.
@jpedryc jpedryc force-pushed the add-ref-schemas-memoization branch from 5612b52 to 0fafbfb Compare December 6, 2024 02:13
@jpedryc
Copy link
Author

jpedryc commented Dec 6, 2024

I've run manual Clockwork tests with these changes on one of my internal projects (just as an addendum) - checked package-provided /openapi endpoint:

Current main With memoization added
Resp. time ~1080 ms ~600 ms
Mem. usage ~10 MB ~8 MB
# Models loaded 6599 2242
# DB queries 188 47
DB time 46 ms 14 ms

@TartanLeGrand
Copy link
Collaborator

LGTM!

@TartanLeGrand TartanLeGrand merged commit 7657892 into Nova-Edge:main Dec 9, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants