Make InvalidRef a critical exception to avoid suppression (#122)

swaggest · Jun 17, 2021 · d0126bd · d0126bd
1 parent ec90855
commit d0126bd
Show file tree

Hide file tree

Showing 11 changed files with 227 additions and 77 deletions.
diff --git a/.github/workflows/cloc.yml b/.github/workflows/cloc.yml
@@ -0,0 +1,35 @@
+name: cloc
+on:
+  pull_request:
+jobs:
+  cloc:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v2
+        with:
+          path: pr
+      - name: Checkout base code
+        uses: actions/checkout@v2
+        with:
+          ref: ${{ github.event.pull_request.base.sha }}
+          path: base
+      - name: Count Lines Of Code
+        id: loc
+        run: |
+          curl -OL https://github.com/vearutop/sccdiff/releases/download/v1.0.1/linux_amd64.tar.gz && tar xf linux_amd64.tar.gz
+          OUTPUT=$(cd pr && ../sccdiff -basedir ../base)
+          OUTPUT="${OUTPUT//'%'/'%25'}"
+          OUTPUT="${OUTPUT//$'\n'/'%0A'}"
+          OUTPUT="${OUTPUT//$'\r'/'%0D'}"
+          echo "::set-output name=diff::$OUTPUT"
+
+      - name: Comment Code Lines
+        uses: marocchino/sticky-pull-request-comment@v2
+        with:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          header: LOC
+          message: |
+            ### Lines Of Code
+
+            ${{ steps.loc.outputs.diff }}
diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
@@ -0,0 +1,37 @@
+name: lint
+on:
+  push:
+    branches:
+      - master
+      - main
+  pull_request:
+jobs:
+  run:
+    runs-on: ${{ matrix.operating-system }}
+    strategy:
+      matrix:
+        operating-system: [ 'ubuntu-latest' ]
+        php-versions: [ '7.4' ]
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v2
+
+      - name: Cache vendor
+        uses: actions/cache@v2
+        with:
+          path: |
+            vendor
+          key: vendor-${{ hashFiles('composer.lock') }}
+
+      - name: Setup PHP
+        uses: shivammathur/setup-php@v2
+        with:
+          php-version: ${{ matrix.php-versions }}
+          ini-values: post_max_size=256M, max_execution_time=180
+          tools: composer
+
+      - name: Populate vendor
+        run: '[ -e vendor ] || composer install'
+
+      - name: Lint
+        run: make lint
diff --git a/.github/workflows/test-unit-cov.yml b/.github/workflows/test-unit-cov.yml
@@ -0,0 +1,38 @@
+name: test-unit-cov
+on:
+  push:
+    branches:
+      - master
+      - main
+  pull_request:
+jobs:
+  run:
+    runs-on: ${{ matrix.operating-system }}
+    strategy:
+      matrix:
+        operating-system: [ 'ubuntu-latest' ]
+        php-versions: [ '7.4' ]
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v2
+
+      - name: Cache vendor
+        uses: actions/cache@v2
+        with:
+          path: |
+            vendor
+          key: vendor-${{ hashFiles('composer.lock') }}
+
+      - name: Setup PHP
+        uses: shivammathur/setup-php@v2
+        with:
+          php-version: ${{ matrix.php-versions }}
+          ini-values: post_max_size=256M, max_execution_time=180
+          coverage: xdebug
+          tools: composer
+
+      - name: Populate vendor
+        run: '[ -e vendor ] || composer install'
+
+      - name: Run Tests With Coverage
+        run: make deps test-coverage && bash <(curl -s https://codecov.io/bash)
diff --git a/.github/workflows/test-unit.yml b/.github/workflows/test-unit.yml
@@ -0,0 +1,37 @@
+name: test-unit
+on:
+  push:
+    branches:
+      - master
+      - main
+  pull_request:
+jobs:
+  run:
+    runs-on: ${{ matrix.operating-system }}
+    strategy:
+      matrix:
+        operating-system: [ 'ubuntu-latest' ]
+        php-versions: [ '5.6', '7.0', '7.1', '7.2', '7.3', '8.0' ]
+    steps:
+      - name: Checkout
+        uses: actions/checkout@v2
+
+      - name: Cache vendor
+        uses: actions/cache@v2
+        with:
+          path: |
+            vendor
+          key: vendor-${{ hashFiles('composer.lock') }}
+
+      - name: Setup PHP
+        uses: shivammathur/setup-php@v2
+        with:
+          php-version: ${{ matrix.php-versions }}
+          ini-values: post_max_size=256M, max_execution_time=180
+          tools: composer
+
+      - name: Populate vendor
+        run: '[ -e vendor ] || composer install'
+
+      - name: Run Tests
+        run: make deps test
diff --git a/.scrutinizer.yml b/.scrutinizer.yml
diff --git a/.travis.yml b/.travis.yml
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,11 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [0.12.35] - 2021-06-18
+
+### Fixed
+- Suppressed failure during reference resolution in complex schemas.
+
 ## [0.12.34] - 2021-06-17
 
 ### Fixed
@@ -83,6 +88,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Changed
 - Export `null` value instead of skipping it for properties having `null` type.
 
+[0.12.35]: https://github.com/swaggest/php-json-schema/compare/v0.12.34...v0.12.35
 [0.12.34]: https://github.com/swaggest/php-json-schema/compare/v0.12.33...v0.12.34
 [0.12.33]: https://github.com/swaggest/php-json-schema/compare/v0.12.32...v0.12.33
 [0.12.32]: https://github.com/swaggest/php-json-schema/compare/v0.12.31...v0.12.32

diff --git a/Makefile b/Makefile
@@ -5,7 +5,7 @@ deps:
 	@git submodule init && git submodule update
 
 lint:
-	@test -f ${HOME}/.cache/composer/phpstan-${PHPSTAN_VERSION}.phar || (mkdir -p ${HOME}/.cache/composer/ && wget https://github.com/phpstan/phpstan/releases/download/${PHPSTAN_VERSION}/phpstan.phar -O ${HOME}/.cache/composer/phpstan-${PHPSTAN_VERSION}.phar)
+	@test -f ${HOME}/.cache/composer/phpstan-${PHPSTAN_VERSION}.phar || (mkdir -p ${HOME}/.cache/composer/ && wget -q https://github.com/phpstan/phpstan/releases/download/${PHPSTAN_VERSION}/phpstan.phar -O ${HOME}/.cache/composer/phpstan-${PHPSTAN_VERSION}.phar)
 	@php $$HOME/.cache/composer/phpstan-${PHPSTAN_VERSION}.phar analyze -l 7 -c phpstan.neon ./src
 
 docker-lint:

diff --git a/src/InvalidRef.php b/src/InvalidRef.php
@@ -2,7 +2,7 @@
 
 namespace Swaggest\JsonSchema;
 
-class InvalidRef extends InvalidValue
+class InvalidRef extends Exception
 {
 
 }
diff --git a/src/Schema.php b/src/Schema.php
@@ -736,7 +736,7 @@ private function processObject($data, Context $options, $path, $result = null)
                             $ref->setImported($refResult);
                             return $refResult;
                         } catch (InvalidValue $exception) {
-                            if ($this->objectItemClass === 'Swaggest\JsonSchema\Schema' || $exception instanceof InvalidRef) {
+                            if ($this->objectItemClass === 'Swaggest\JsonSchema\Schema') {
                                 throw $exception;
                             }
 

diff --git a/tests/src/PHPUnit/Suite/SwaggerTest.php b/tests/src/PHPUnit/Suite/SwaggerTest.php
@@ -4,7 +4,7 @@
 namespace Swaggest\JsonSchema\Tests\PHPUnit\Suite;
 
 
-use Swaggest\JsonSchema\InvalidValue;
+use Swaggest\JsonSchema\InvalidRef;
 use Swaggest\JsonSchema\Schema;
 
 class SwaggerTest extends \PHPUnit_Framework_TestCase
@@ -27,18 +27,77 @@ public function testInvalid()
         $failed = false;
         try {
             $instance = $schema->in(json_decode($petstore));
-        } catch (InvalidValue $exception) {
+        } catch (InvalidRef $exception) {
             $failed = true;
-            $this->assertEquals('No valid results for oneOf {
- 0: No valid results for oneOf {
-  0: No valid results for anyOf {
-    0: Could not resolve #/definitions/Foo@: Foo at #->properties:paths->$ref[#/definitions/paths]->patternProperties[^/]:/pets->$ref[#/definitions/pathItem]->properties:get->$ref[#/definitions/operation]->properties:responses->$ref[#/definitions/responses]->patternProperties[^([0-9]{3})$|^(default)$]:200->$ref[#/definitions/responseValue]->oneOf[0]->$ref[#/definitions/response]->properties:schema->oneOf[0]->$ref[#/definitions/schema]->properties:items->anyOf[0]->$ref[#/definitions/schema]
-    1: Array expected, {"$ref":"#\/definitions\/Foo"} received at #->properties:paths->$ref[#/definitions/paths]->patternProperties[^/]:/pets->$ref[#/definitions/pathItem]->properties:get->$ref[#/definitions/operation]->properties:responses->$ref[#/definitions/responses]->patternProperties[^([0-9]{3})$|^(default)$]:200->$ref[#/definitions/responseValue]->oneOf[0]->$ref[#/definitions/response]->properties:schema->oneOf[0]->$ref[#/definitions/schema]->properties:items->anyOf[1]
-  } at #->properties:paths->$ref[#/definitions/paths]->patternProperties[^/]:/pets->$ref[#/definitions/pathItem]->properties:get->$ref[#/definitions/operation]->properties:responses->$ref[#/definitions/responses]->patternProperties[^([0-9]{3})$|^(default)$]:200->$ref[#/definitions/responseValue]->oneOf[0]->$ref[#/definitions/response]->properties:schema->oneOf[0]->$ref[#/definitions/schema]->properties:items
-  1: Enum failed, enum: ["file"], data: "array" at #->properties:paths->$ref[#/definitions/paths]->patternProperties[^/]:/pets->$ref[#/definitions/pathItem]->properties:get->$ref[#/definitions/operation]->properties:responses->$ref[#/definitions/responses]->patternProperties[^([0-9]{3})$|^(default)$]:200->$ref[#/definitions/responseValue]->oneOf[0]->$ref[#/definitions/response]->properties:schema->oneOf[1]->$ref[#/definitions/fileSchema]->properties:type
- } at #->properties:paths->$ref[#/definitions/paths]->patternProperties[^/]:/pets->$ref[#/definitions/pathItem]->properties:get->$ref[#/definitions/operation]->properties:responses->$ref[#/definitions/responses]->patternProperties[^([0-9]{3})$|^(default)$]:200->$ref[#/definitions/responseValue]->oneOf[0]->$ref[#/definitions/response]->properties:schema
- 1: Required property missing: $ref, data: {"description":"pet response","schema":{"type":"array","items":{"$ref":"#/definitions/Foo"}}} at #->properties:paths->$ref[#/definitions/paths]->patternProperties[^/]:/pets->$ref[#/definitions/pathItem]->properties:get->$ref[#/definitions/operation]->properties:responses->$ref[#/definitions/responses]->patternProperties[^([0-9]{3})$|^(default)$]:200->$ref[#/definitions/responseValue]->oneOf[1]->$ref[#/definitions/jsonReference]
-} at #->properties:paths->$ref[#/definitions/paths]->patternProperties[^/]:/pets->$ref[#/definitions/pathItem]->properties:get->$ref[#/definitions/operation]->properties:responses->$ref[#/definitions/responses]->patternProperties[^([0-9]{3})$|^(default)$]:200->$ref[#/definitions/responseValue]',
+            $this->assertEquals('Could not resolve #/definitions/Foo@: Foo',
+                $exception->getMessage());
+        }
+
+        $this->assertTrue($failed);
+    }
+
+    public function testInvalid2()
+    {
+        $schema = Schema::import(json_decode(file_get_contents(__DIR__ . '/../../../resources/swagger-schema.json')));
+
+        $json = <<<'JSON'
+{
+    "swagger": "2.0",
+    "info": {
+        "title": "test",
+        "version": "1.0.0"
+    },
+    "paths": {
+        "/test": {
+            "get": {
+                "summary": "test",
+                "responses": {
+                    "200": {
+                        "description": "successful response",
+                        "schema": {
+                            "$ref": "#/definitions/response"
+                        }
+                    }
+                }
+            }
+        }
+    },
+    "definitions": {
+        "response": {
+            "properties": {
+                "foo": {
+                    "type": "array",
+                    "items": {
+                        "$ref": "#/definitions/good"
+                    }
+                },
+                "bar": {
+                    "type": "array",
+                    "items": {
+                        "$ref": "#/definitions/missing1"
+                    }
+                }
+            }
+        },
+        "good": {
+            "properties": {
+                "foo": {
+                    "$ref": "#/definitions/missing2"
+                }
+            }
+        }
+    }
+}
+
+JSON;
+
+
+        $failed = false;
+        try {
+            $instance = $schema->in(json_decode($json));
+        } catch (InvalidRef $exception) {
+            $failed = true;
+            $this->assertEquals('Could not resolve #/definitions/missing2@: missing2',
                 $exception->getMessage());
         }