Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve: Better way to resolve allOf #208

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
2984dc1
Tooling: dynamic container name to remove confusion created by multip…
SOHELAHMED7 Sep 11, 2024
b111f49
Tooling: dynamic container name to remove confusion created by multip…
SOHELAHMED7 Sep 13, 2024
27e2618
Remove Composer dev dependencies apis-guru/openapi-directory, nexmo/a…
SOHELAHMED7 Sep 14, 2024
b4c19c7
Temporary remove old PHPUnit config
SOHELAHMED7 Sep 14, 2024
bffeea7
Add failing test
SOHELAHMED7 Sep 14, 2024
99dd8be
Implement
SOHELAHMED7 Sep 20, 2024
35a9f90
Refactor + fix bug WIP
SOHELAHMED7 Sep 20, 2024
f5a6472
Fix bug related to improper array merging
SOHELAHMED7 Sep 22, 2024
a9b2dbd
Refactor and add more tests
SOHELAHMED7 Sep 23, 2024
b5c87a4
Refactor
SOHELAHMED7 Sep 23, 2024
fc75f0b
Add docs and tests
SOHELAHMED7 Sep 23, 2024
9805713
Add docs and fix bugs
SOHELAHMED7 Sep 23, 2024
a3a6421
Implement for JSON
SOHELAHMED7 Sep 23, 2024
b28249f
Revert "Remove Composer dev dependencies apis-guru/openapi-directory,…
SOHELAHMED7 Sep 23, 2024
74a7e03
Revert "Temporary remove old PHPUnit config"
SOHELAHMED7 Sep 23, 2024
d83af16
Mange TODOs
SOHELAHMED7 Sep 23, 2024
15faaba
Delete TODOs file
SOHELAHMED7 Sep 23, 2024
b68d0d1
Add a test for `description` for https://github.com/php-openapi/yii2-…
SOHELAHMED7 Sep 23, 2024
8698470
Add more tests
SOHELAHMED7 Sep 26, 2024
81ee4ed
Move `arrayMergeRecursiveDistinct()` to separate helper class
SOHELAHMED7 Nov 15, 2024
0bc15f5
Add test case explaining recursion in resolving `allOf`
SOHELAHMED7 Nov 16, 2024
8513118
Update src/SpecBaseObject.php. PR feedback
SOHELAHMED7 Nov 18, 2024
a95c9d1
Update src/SpecBaseObject.php. PR feedback
SOHELAHMED7 Nov 18, 2024
00e4669
Update src/SpecBaseObject.php. PR feedback
SOHELAHMED7 Nov 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,4 @@
/.phpunit.result.cache

php-cs-fixer.phar
xdebug_remote.log
19 changes: 18 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ DOCKER_PHP=docker-compose run --rm php
DOCKER_NODE=docker-compose run --rm -w /app node
endif

CONTAINER_NAME=$$(echo $$(pwd) | tr / _ | cut -c 2-) # lets have unique container name when dealing with lot of forks
UID=$(shell id -u)

all:
@echo "the following commands are available:"
@echo ""
Expand All @@ -32,6 +35,12 @@ all:
check-style: php-cs-fixer.phar
PHP_CS_FIXER_IGNORE_ENV=1 ./php-cs-fixer.phar fix src/ --diff --dry-run

cli:
COMPOSE_PROJECT_NAME=$(CONTAINER_NAME) docker-compose exec --user=$(UID) php bash # lets have unique container name when dealing with lot of forks

cli_root:
COMPOSE_PROJECT_NAME=$(CONTAINER_NAME) docker-compose exec --user="root" php bash

fix-style: php-cs-fixer.phar
$(DOCKER_PHP) vendor/bin/indent --tabs composer.json
$(DOCKER_PHP) vendor/bin/indent --spaces .php_cs.dist
Expand Down Expand Up @@ -81,5 +90,13 @@ coverage: .php-openapi-covA .php-openapi-covB
.php-openapi-covB:
grep -rhPo '^class \w+' src/spec/ | awk '{print $$2}' |grep -v '^Type$$' | sort > $@

.PHONY: all check-style fix-style install test lint coverage
build-docker:
COMPOSE_PROJECT_NAME=$(CONTAINER_NAME) docker-compose build

start-docker:
COMPOSE_PROJECT_NAME=$(CONTAINER_NAME) docker-compose up -d

stop-docker:
COMPOSE_PROJECT_NAME=$(CONTAINER_NAME) docker-compose down --remove-orphans

.PHONY: all check-style fix-style install test lint coverage build-docker start-docker stop-docker
69 changes: 69 additions & 0 deletions doc/array merge recursive distinct.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
### array merge recursive distinct

While resolving `allOf`s (pull request https://github.com/cebe/php-openapi/pull/208), if a duplicate property is found

```yaml
components:
schemas:
User:
type: object
required:
- id
- name # <--------------------------------------------------------------
properties:
id:
type: integer
name: # <--------------------------------------------------------------
type: string
maxLength: 10 # <--------------------------------------------------------------
Pet:
type: object
required:
- id2
- name # <--------------------------------------------------------------
properties:
id2:
type: integer
name: # <--------------------------------------------------------------
type: string
maxLength: 12 # <--------------------------------------------------------------
Post:
type: object
properties:
id:
type: integer
content:
type: string
user:
allOf:
- $ref: '#/components/schemas/User'
- $ref: '#/components/schemas/Pet'
- x-faker: true
```

then property from the last component schema will be considered:

```yaml
Post:
type: object
properties:
id:
type: integer
content:
type: string
user:
type: object
required:
- id
- name # <--------------------------------------------------------------
- id2
properties:
id:
type: integer
name: # <--------------------------------------------------------------
type: string
maxLength: 12 # <--------------------------------------------------------------
id2:
type: integer
x-faker: true
```
58 changes: 58 additions & 0 deletions src/Helper.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
<?php

/**
* @copyright Copyright (c) 2018 Carsten Brandt <mail@cebe.cc> and contributors
* @license https://github.com/cebe/php-openapi/blob/master/LICENSE
*/

namespace cebe\openapi;

/**
* Helper class containing widely used custom functions used in library
*/
class Helper
{
/**
* Thanks https://www.php.net/manual/en/function.array-merge-recursive.php#96201
*
* Merges any number of arrays / parameters recursively, replacing
* entries with string keys with values from latter arrays.
* If the entry or the next value to be assigned is an array, then it
* automagically treats both arguments as an array.
* Numeric entries are appended, not replaced, but only if they are
* unique
*
* Function call example: `$result = array_merge_recursive_distinct(a1, a2, ... aN);`
* More documentation is present at [array merge recursive distinct.md](../../../doc/array merge recursive distinct.md) file
* @return array
*/
public static function arrayMergeRecursiveDistinct()
{
$arrays = func_get_args();
$base = array_shift($arrays);
if (!is_array($base)) {
$base = empty($base) ? [] : [$base];
}
foreach ($arrays as $append) {
if (!is_array($append)) {
$append = [$append];
}
foreach ($append as $key => $value) {
if (!array_key_exists($key, $base) and !is_numeric($key)) {
$base[$key] = $append[$key];
continue;
}
if (is_array($value) || is_array($base[$key])) {
$base[$key] = static::arrayMergeRecursiveDistinct($base[$key], $append[$key]);
} elseif (is_numeric($key)) {
if (!in_array($value, $base)) {
$base[] = $value;
}
} else {
$base[$key] = $value;
}
}
}
return $base;
}
}
15 changes: 13 additions & 2 deletions src/Reader.php
Original file line number Diff line number Diff line change
Expand Up @@ -74,14 +74,18 @@ public static function readFromYaml(string $yaml, string $baseType = OpenApi::cl
* Since version 1.5.0 this can be a string indicating the reference resolving mode:
* - `inline` only resolve references to external files.
* - `all` resolve all references except recursive references.
* @param bool $resolveAllOfs whether to automatically resolve all `allOf`s automatically. It will
* only work if [[$resolveReferences]] is `true` or [[ReferenceContext::RESOLVE_MODE_ALL]]. The
* concept of resolving all `allOf`s is explained at https://github.com/cebe/php-openapi/pull/208
* in detail with example.
* @return SpecObjectInterface|OpenApi the OpenApi object instance.
* The type of the returned object depends on the `$baseType` argument.
* @throws TypeErrorException in case invalid spec data is supplied.
* @throws UnresolvableReferenceException in case references could not be resolved.
* @throws IOException when the file is not readable.
* @throws InvalidJsonPointerSyntaxException in case an invalid JSON pointer string is passed to the spec references.
*/
public static function readFromJsonFile(string $fileName, string $baseType = OpenApi::class, $resolveReferences = true): SpecObjectInterface
public static function readFromJsonFile(string $fileName, string $baseType = OpenApi::class, $resolveReferences = true, bool $resolveAllOfs = false): SpecObjectInterface
{
$fileContent = file_get_contents($fileName);
if ($fileContent === false) {
Expand All @@ -101,6 +105,9 @@ public static function readFromJsonFile(string $fileName, string $baseType = Ope
}
$spec->resolveReferences();
}
if ($resolveAllOfs && ($resolveReferences === true || $resolveReferences === ReferenceContext::RESOLVE_MODE_ALL)) {
$spec->resolveAllOf();
}
return $spec;
}

Expand All @@ -121,13 +128,14 @@ public static function readFromJsonFile(string $fileName, string $baseType = Ope
* Since version 1.5.0 this can be a string indicating the reference resolving mode:
* - `inline` only resolve references to external files.
* - `all` resolve all references except recursive references.
* @param bool $resolveAllOfs whether to automatically resolve all `allOf`s automatically. It will only work if [[$resolveReferences]] is `true` or [[ReferenceContext::RESOLVE_MODE_ALL]]. The concept of resolving all `allOf`s is explained at https://github.com/cebe/php-openapi/pull/208 in detail with example.
* @return SpecObjectInterface|OpenApi the OpenApi object instance.
* The type of the returned object depends on the `$baseType` argument.
* @throws TypeErrorException in case invalid spec data is supplied.
* @throws UnresolvableReferenceException in case references could not be resolved.
* @throws IOException when the file is not readable.
*/
public static function readFromYamlFile(string $fileName, string $baseType = OpenApi::class, $resolveReferences = true): SpecObjectInterface
public static function readFromYamlFile(string $fileName, string $baseType = OpenApi::class, $resolveReferences = true, bool $resolveAllOfs = false): SpecObjectInterface
{
$fileContent = file_get_contents($fileName);
if ($fileContent === false) {
Expand All @@ -147,6 +155,9 @@ public static function readFromYamlFile(string $fileName, string $baseType = Ope
}
$spec->resolveReferences();
}
if ($resolveAllOfs && ($resolveReferences === true || $resolveReferences === ReferenceContext::RESOLVE_MODE_ALL)) {
$spec->resolveAllOf();
}
return $spec;
}
}
84 changes: 84 additions & 0 deletions src/SpecBaseObject.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use cebe\openapi\json\JsonPointer;
use cebe\openapi\json\JsonReference;
use cebe\openapi\spec\Reference;
use cebe\openapi\spec\Schema;
use cebe\openapi\spec\Type;

/**
Expand All @@ -31,6 +32,7 @@ abstract class SpecBaseObject implements SpecObjectInterface, DocumentContextInt
private $_recursingReferences = false;
private $_recursingReferenceContext = false;
private $_recursingDocumentContext = false;
private $_recursingAllOf = false;

private $_baseDocument;
private $_jsonPointer;
Expand Down Expand Up @@ -525,4 +527,86 @@ public function getExtensions(): array
}
return $extensions;
}

public function getProperties(): array
{
return $this->_properties;
}

public function mergeProperties($properties)
{
$this->_properties = Helper::arrayMergeRecursiveDistinct($this->_properties, $properties);
}

public function resolveAllOf()
{
// avoid recursion to get stuck in a loop
if ($this->_recursingAllOf) {
return;
}
$this->_recursingAllOf = true;

foreach ($this->_properties as $property => $value) {
$this->handleMergingOfAllAllOfs($property, $value);
$this->removeAllOfKey($property, $value);
}
$this->_recursingAllOf = false;
}

private function mergeAllAllOfsInToSingleObject(): self
{
$allOfs = $this->allOf;
/** @var static $first */
$first = $this->allOf[0];
unset($allOfs[0]);
foreach ($allOfs as $allOf) {
/** @var Schema $allOf */
$first->mergeProperties($allOf->getProperties());
}
return $first;
}

private function handleMergingOfAllAllOfs(string $property, $value): void
{
if ($property === 'allOf' && !empty($value)) {
$this->_properties[$property] = $this->mergeAllAllOfsInToSingleObject();
} elseif ($value instanceof SpecObjectInterface && method_exists($value, 'resolveAllOf')) {
$value->resolveAllOf();
} elseif (is_array($value)) {
foreach ($value as $k => $item) {
if ($k === 'allOf' && !empty($item)) {
$this->_properties[$property][$k] = $this->mergeAllAllOfsInToSingleObject();
} elseif ($item instanceof SpecObjectInterface && method_exists($item, 'resolveAllOf')) {
$item->resolveAllOf();
}
}
}
}

private function removeAllOfKey(string $property, $value): void
{
if ($property === 'properties' && !empty($value)) {
foreach ($value as $k => $v) {
if (!empty($v->allOf)) {
$temp = $v->allOf;
$this->_properties[$property][$k] = $temp;
}
}
} elseif ($value instanceof SpecObjectInterface && method_exists($value, 'resolveAllOf')) {
$value->resolveAllOf();
} elseif (is_array($value)) {
foreach ($value as $arrayValueKey => $item) {
if ($arrayValueKey === 'properties' && !empty($item)) {
foreach ($item as $itemKey => $itemValue) {
if (!empty($itemValue->allOf)) {
$tempIn = $itemValue->allOf;
$this->_properties[$property][$arrayValueKey][$itemKey] = $tempIn;
}
}
} elseif ($item instanceof SpecObjectInterface && method_exists($item, 'resolveAllOf')) {
$item->resolveAllOf();
}
}
}
}
}
1 change: 0 additions & 1 deletion src/spec/OpenApi.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

namespace cebe\openapi\spec;

use cebe\openapi\exceptions\TypeErrorException;
use cebe\openapi\SpecBaseObject;

/**
Expand Down
Loading