Merge pull request #122 from wol-soft/refImprovements

Ref improvements

Author: wol-soft

src/Model/SchemaDefinition/SchemaDefinitionDictionary.php

@@ -16,9 +16,12 @@
  */
 class SchemaDefinitionDictionary extends ArrayObject
 {
-    /** @var Schema[] */
+    /** @var Schema[] keyed by canonical file path */
     private array $parsedExternalFileSchemas = [];
 
+    /** @var string[] maps raw ref string → canonical file path */
+    private array $rawRefToCanonical = [];
+
     /**
      * SchemaDefinitionDictionary constructor.
      */
@@ -111,8 +114,9 @@ public function getDefinition(string $key, SchemaProcessor $schemaProcessor, arr
                 $externalKey = '';
             }
 
-            if (array_key_exists($jsonSchemaFile, $this->parsedExternalFileSchemas)) {
-                return $this->parsedExternalFileSchemas[$jsonSchemaFile]->getSchemaDictionary()->getDefinition(
+            if (isset($this->rawRefToCanonical[$jsonSchemaFile])) {
+                $canonicalKey = $this->rawRefToCanonical[$jsonSchemaFile];
+                return $this->parsedExternalFileSchemas[$canonicalKey]->getSchemaDictionary()->getDefinition(
                     "#$externalKey",
                     $schemaProcessor,
                     $path,
@@ -136,23 +140,73 @@ private function parseExternalFile(
         SchemaProcessor $schemaProcessor,
         array &$path,
     ): ?SchemaDefinition {
+        // Resolve the ref to obtain the canonical file path or URL via the provider.
+        // Using $jsonSchema->getFile() as the key (not the raw $jsonSchemaFile argument) ensures
+        // that relative refs from different directories pointing to the same file share one entry,
+        // and that network URLs resolved via $id are handled correctly.
         $jsonSchema = $schemaProcessor->getSchemaProvider()->getRef(
             $this->schema->getFile(),
             $this->schema->getJson()['$id'] ?? null,
             $jsonSchemaFile,
         );
 
-        // set up a dummy schema to fetch the definitions from the external file
-        $schema = new Schema(
-            '',
-            $schemaProcessor->getCurrentClassPath(),
-            'ExternalSchema',
-            $jsonSchema,
-            new self($jsonSchema),
-        );
+        $canonicalKey = $jsonSchema->getFile();
+
+        if ($existingSchema = $schemaProcessor->getProcessedFileSchema($canonicalKey)) {
+            // The file was already processed (either as a top-level initial class, or by an
+            // earlier $ref from another schema). Reuse the existing schema — no duplicate class.
+            $this->parsedExternalFileSchemas[$canonicalKey] = $existingSchema;
+            $this->rawRefToCanonical[$jsonSchemaFile] = $canonicalKey;
+
+            return $existingSchema->getSchemaDictionary()->getDefinition($externalKey, $schemaProcessor, $path);
+        }
+
+        // First encounter of this external file. If the file lives within the provider's base
+        // directory it will eventually be iterated by the provider and must receive the canonical
+        // class name derived from its filename. Process it eagerly now so the correct class is
+        // used even when the referencing schema is discovered first (issue #116).
+        //
+        // If the file is outside the provider's base directory the provider will never iterate it,
+        // so we use an ExternalSchema placeholder instead — this avoids rendering and requiring
+        // the class on every generateModels() call when test infrastructure re-runs the generator
+        // with a fresh SchemaProcessor that has no memory of previous runs.
+        $baseDir         = str_replace('\\', '/', $schemaProcessor->getSchemaProvider()->getBaseDirectory());
+        $canonicalNorm   = str_replace('\\', '/', $canonicalKey);
+        $isInsideBaseDir = str_starts_with($canonicalNorm, $baseDir . '/') || $canonicalNorm === $baseDir;
+
+        if ($isInsideBaseDir) {
+            // The file lives within the provider's base directory and will eventually be iterated.
+            // Process it eagerly now so the canonical class name (derived from the filename) is
+            // used regardless of which schema the provider discovers first (issue #116).
+            // processTopLevelSchema also registers the schema in processedFileSchemas, so no
+            // separate registerProcessedFileSchema call is needed here.
+            //
+            // processTopLevelSchema returns null when the file has no top-level type:object or
+            // composition (e.g. a definitions-only file). Fall through to ExternalSchema in that
+            // case so fragment refs inside it still resolve.
+            $schema = $schemaProcessor->processTopLevelSchema($jsonSchema);
+        } else {
+            $schema = null;
+        }
+
+        if ($schema === null) {
+            // The file is outside the provider's base directory, or it defines no top-level
+            // object/composition. Use an ExternalSchema placeholder so fragment refs still
+            // resolve, without rendering or requiring a class file on disk.
+            $schema = new Schema(
+                '',
+                $schemaProcessor->getCurrentClassPath(),
+                'ExternalSchema',
+                $jsonSchema,
+                new self($jsonSchema),
+            );
+
+            $schema->getSchemaDictionary()->setUpDefinitionDictionary($schemaProcessor, $schema);
+            $schemaProcessor->registerProcessedFileSchema($canonicalKey, $schema);
+        }
 
-        $schema->getSchemaDictionary()->setUpDefinitionDictionary($schemaProcessor, $schema);
-        $this->parsedExternalFileSchemas[$jsonSchemaFile] = $schema;
+        $this->parsedExternalFileSchemas[$canonicalKey] = $schema;
+        $this->rawRefToCanonical[$jsonSchemaFile] = $canonicalKey;
 
         return $schema->getSchemaDictionary()->getDefinition($externalKey, $schemaProcessor, $path);
     }

src/PropertyProcessor/Property/ReferenceProcessor.php

@@ -32,17 +32,33 @@ public function process(string $propertyName, JsonSchema $propertySchema): Prope
             $definition = $dictionary->getDefinition($reference, $this->schemaProcessor, $path);
 
             if ($definition) {
+                $definitionSchema = $definition->getSchema();
+
                 if (
-                    $this->schema->getClassPath() !== $definition->getSchema()->getClassPath() ||
-                    $this->schema->getClassName() !== $definition->getSchema()->getClassName() ||
+                    $this->schema->getClassPath() !== $definitionSchema->getClassPath() ||
+                    $this->schema->getClassName() !== $definitionSchema->getClassName() ||
                     (
                         $this->schema->getClassName() === 'ExternalSchema' &&
-                        $definition->getSchema()->getClassName() === 'ExternalSchema'
+                        $definitionSchema->getClassName() === 'ExternalSchema'
                     )
                 ) {
                     $this->schema->addNamespaceTransferDecorator(
-                        new SchemaNamespaceTransferDecorator($definition->getSchema()),
+                        new SchemaNamespaceTransferDecorator($definitionSchema),
                     );
+
+                    // When the definition resolves to a canonical (non-ExternalSchema) class that
+                    // lives in a different namespace from the current schema, register its FQCN
+                    // directly as a used class. The ExternalSchema intermediary that previously
+                    // performed this registration (transitively via its own usedClasses list) is
+                    // no longer created when the file was already processed; this explicit call
+                    // ensures the referencing schema's import list remains complete.
+                    if ($definitionSchema->getClassName() !== 'ExternalSchema') {
+                        $this->schema->addUsedClass(join('\\', array_filter([
+                            $this->schemaProcessor->getGeneratorConfiguration()->getNamespacePrefix(),
+                            $definitionSchema->getClassPath(),
+                            $definitionSchema->getClassName(),
+                        ])));
+                    }
                 }
 
                 return $definition->resolveReference($propertyName, $path, $this->propertyMetaDataCollection);

src/SchemaProcessor/SchemaProcessor.php

@@ -36,6 +36,24 @@ class SchemaProcessor
     protected array $processedSchema = [];
     /** @var PropertyInterface[] Collect processed schemas to avoid duplicated classes */
     protected array $processedMergedProperties = [];
+    /**
+     * Global index of schemas keyed by the canonical file path or URL returned by
+     * SchemaProviderInterface::getRef(). Used to deduplicate external $ref resolutions across
+     * all schema processings, making class generation order-independent.
+     *
+     * When a $ref triggers processTopLevelSchema() for a file that the provider has not yet
+     * reached, the canonical Schema is registered here before property processing begins. If
+     * the provider later iterates the same file, generateModel() detects the match via the
+     * combined file-path + content-signature check and returns the already-registered Schema
+     * without creating a duplicate render job.
+     *
+     * Note: for providers such as OpenAPIv3Provider that yield multiple distinct schemas from
+     * a single source file, each schema has a unique content signature; the signature check
+     * prevents false-positive deduplication across schemas that merely share the same file.
+     *
+     * @var Schema[]
+     */
+    protected array $processedFileSchemas = [];
     /** @var string[] */
     protected array $generatedFiles = [];
 
@@ -120,6 +138,21 @@ protected function generateModel(
             return $this->processedSchema[$schemaSignature];
         }
 
+        // For initial-class calls: if this exact file+content was already processed eagerly via
+        // processTopLevelSchema() (triggered by a $ref resolution), reuse that schema to avoid a
+        // duplicate render job. Both checks are required:
+        // - The file-path check detects that this file was already processed via a $ref.
+        // - The signature check ensures we do not short-circuit when a different schema shares
+        //   the same source file (e.g. OpenAPI v3 where all component schemas are yielded from
+        //   the same spec file — each has a unique signature).
+        if (
+            $initialClass
+            && isset($this->processedSchema[$schemaSignature])
+            && $this->getProcessedFileSchema($jsonSchema->getFile()) !== null
+        ) {
+            return $this->processedSchema[$schemaSignature];
+        }
+
         $schema = new Schema(
             $this->getTargetFileName($classPath, $className),
             $classPath,
@@ -130,7 +163,13 @@ protected function generateModel(
             $this->generatorConfiguration,
         );
 
+        // Register by content signature (secondary dedup for content-identical inline schemas).
         $this->processedSchema[$schemaSignature] = $schema;
+        // Register by canonical file path/URL (primary dedup for external $ref resolutions).
+        // Registering here — before property processing — ensures that any $ref back to this
+        // file encountered while processing the referencing schema finds this canonical schema
+        // immediately, regardless of which schema was discovered first by the provider.
+        $this->registerProcessedFileSchema($jsonSchema->getFile(), $schema);
         $json = $jsonSchema->getJson();
         $json['type'] = 'base';
 
@@ -310,10 +349,20 @@ function () use ($property, $schema, $mergedPropertySchema): void {
      */
     protected function setCurrentClassPath(string $jsonSchemaFile): void
     {
-        $path = str_replace($this->schemaProvider->getBaseDirectory(), '', dirname($jsonSchemaFile));
+        $fileDir  = str_replace('\\', '/', dirname($jsonSchemaFile));
+        $baseDir  = str_replace('\\', '/', $this->schemaProvider->getBaseDirectory());
+        $relative = str_replace($baseDir, '', $fileDir);
+
+        // If the file is outside the provider's base directory, str_replace leaves the absolute
+        // path untouched. In that case fall back to using just the last directory component so
+        // the generated class path stays sensible rather than encoding an absolute path.
+        if ($relative === $fileDir) {
+            $relative = basename($fileDir);
+        }
+
         $pieces = array_map(
             static fn(string $directory): string => ucfirst((string) preg_replace('/\W/', '', $directory)),
-            explode(DIRECTORY_SEPARATOR, $path),
+            explode('/', $relative),
         );
 
         $this->currentClassPath = join('\\', array_filter($pieces));
@@ -344,6 +393,62 @@ public function getSchemaProvider(): SchemaProviderInterface
         return $this->schemaProvider;
     }
 
+    public function getProcessedFileSchema(string $fileKey): ?Schema
+    {
+        return $this->processedFileSchemas[$this->normaliseFileKey($fileKey)] ?? null;
+    }
+
+    public function registerProcessedFileSchema(string $fileKey, Schema $schema): void
+    {
+        $this->processedFileSchemas[$this->normaliseFileKey($fileKey)] = $schema;
+    }
+
+    /**
+     * Normalise a file path or URL to a consistent key for processedFileSchemas.
+     * On Windows, RecursiveDirectoryIterator may produce backslash-separated paths while
+     * RefResolverTrait produces forward-slash paths for the same file. Normalising to forward
+     * slashes ensures the two representations map to the same key.
+     */
+    private function normaliseFileKey(string $fileKey): string
+    {
+        return str_replace('\\', '/', $fileKey);
+    }
+
+    /**
+     * Process an external schema file with its canonical class name and path, exactly as
+     * process() would, but without overwriting the current class path / class name context
+     * (which belongs to the schema that triggered the $ref resolution).
+     *
+     * Returns the resulting Schema, or null if the file does not define an object/composition.
+     *
+     * @throws SchemaException
+     */
+    public function processTopLevelSchema(JsonSchema $jsonSchema): ?Schema
+    {
+        $savedClassPath  = $this->currentClassPath;
+        $savedClassName  = $this->currentClassName;
+
+        $this->setCurrentClassPath($jsonSchema->getFile());
+        $this->currentClassName = $this->generatorConfiguration->getClassNameGenerator()->getClassName(
+            str_ireplace('.json', '', basename($jsonSchema->getFile())),
+            $jsonSchema,
+            false,
+        );
+
+        $schema = $this->processSchema(
+            $jsonSchema,
+            $this->currentClassPath,
+            $this->currentClassName,
+            new SchemaDefinitionDictionary($jsonSchema),
+            true,
+        );
+
+        $this->currentClassPath = $savedClassPath;
+        $this->currentClassName = $savedClassName;
+
+        return $schema;
+    }
+
     private function getTargetFileName(string $classPath, string $className): string
     {
         return join(

src/SchemaProvider/OpenAPIv3Provider.php

@@ -26,6 +26,7 @@ class OpenAPIv3Provider implements SchemaProviderInterface
      */
     public function __construct(private string $sourceFile)
     {
+        $this->sourceFile = realpath($this->sourceFile) ?: $this->sourceFile;
         $jsonSchema = file_get_contents($this->sourceFile);
 
         if (!$jsonSchema || !($this->openAPIv3Spec = json_decode($jsonSchema, true))) {

src/SchemaProvider/RecursiveDirectoryProvider.php

@@ -33,7 +33,7 @@ public function __construct(string $sourceDirectory)
             throw new FileSystemException("Source directory '$sourceDirectory' doesn't exist");
         }
 
-        $this->sourceDirectory = rtrim($sourceDirectory, "\\/");
+        $this->sourceDirectory = realpath(rtrim($sourceDirectory, "\\/")) ?: rtrim($sourceDirectory, "\\/");
     }
 
     /**

src/SchemaProvider/RefResolverTrait.php

@@ -73,7 +73,7 @@ private function getLocalRefPath(string $currentFile, string $ref): ?string
         if (!str_starts_with($jsonSchemaFile, '/')) {
             $candidate = $currentDir . '/' . $jsonSchemaFile;
 
-            return file_exists($candidate) ? $candidate : null;
+            return file_exists($candidate) ? realpath($candidate) : null;
         }
 
         // absolute paths: traverse up to find the context root directory
@@ -83,7 +83,7 @@ private function getLocalRefPath(string $currentFile, string $ref): ?string
         while (true) {
             $candidate = $dir . '/' . $relative;
             if (file_exists($candidate)) {
-                return $candidate;
+                return realpath($candidate);
             }
 
             $parent = dirname($dir);

tests/ComposedValue/ComposedRequiredPromotionTest.php

@@ -255,6 +255,24 @@ public function testAnyOfRequiredOnlyBranchesDoesNotCrash(): void
         $this->assertEmpty($rc->getProperties(ReflectionMethod::IS_PUBLIC));
     }
 
+    /**
+     * anyOf: property is defined in root schema properties and required in every branch
+     * (branches carry only 'required', no 'properties'). The root-level property must be
+     * promoted to non-nullable because every anyOf branch guarantees its presence.
+     */
+    #[DataProvider('implicitNullDataProvider')]
+    public function testAnyOfRootPropertyRequiredInAllBranchesIsPromoted(bool $implicitNull): void
+    {
+        $className = $this->generateClassFromFile(
+            'AnyOfRootPropertyRequiredInAllBranches.json',
+            null,
+            false,
+            $implicitNull,
+        );
+
+        $this->assertNonNullableStringProperty($className, 'name');
+    }
+
     /**
      * anyOf: branches define an untyped property (no 'type' keyword) in required —
      * promoteProperty returns early when getType() is null. No crash, and the
@@ -439,6 +457,23 @@ public function testIfThenElseOneBranchRequiredIsNotPromoted(bool $implicitNull)
         $this->assertNullableStringProperty($className, 'name');
     }
 
+    /**
+     * if/else only (no then): required in else only → not promoted
+     * (collectFromConditional count < 2 path — only one condition branch exists).
+     */
+    #[DataProvider('implicitNullDataProvider')]
+    public function testIfElseOnlyNotPromoted(bool $implicitNull): void
+    {
+        $className = $this->generateClassFromFile(
+            'IfElseOnlyRequired.json',
+            null,
+            false,
+            $implicitNull,
+        );
+
+        $this->assertNullableStringProperty($className, 'name');
+    }
+
     /**
      * if/then/else: property already required at root level → short-circuit.
      */