Testcase for minProperties with properties defined + Fix tests #406

flolivaud · 2017-03-24T10:31:43Z

The minProperties is not working if properties is defined.

It's probably because of https://github.com/justinrainbow/json-schema/blob/master/src/JsonSchema/Constraints/UndefinedConstraint.php#L66

…nrainbow#362) * Add URI translation for retrieval & add local copies of spec schema * Use dist copies of schemas No need to keep duplicate files around in package://tests/fixtures/ if we're distributing them for users anyway. * Move package:// translation after all other rules Allows users to rewrite to package:// targets and still have the URI work.

* centralize errors * isolate 'more' info * throw exception for missing error message * swap args

Fixes issue jsonrainbow#369

…#372) Fixes issue jsonrainbow#371

This reverts commit 73ef463. 'email' is only a valid type attribute in draft-03 (later versions of the spec explicitly limit the value of type to the core primitive types), and this code doesn't validate email addresses anyway. IMO we should be validating it properly or not at all, and noting this went away after draft-03 my opinion is on the not-at-all side of the fence. Note that 'email' is *never* defined as a spec type, in any version - it just slips in under the radar of the draft-03 language which allows users to put arbitrary things in the type field.

…rainbow#376) (jsonrainbow#378) * Add provided schema under a dummy / internal URI (fixes jsonrainbow#376) In order to resolve internal $ref references within a user-provided schema, SchemaStorage needs to know about the schema. As user-supplied schemas do not have an associated URI, use a dummy / internal one instead. * Remove dangling use * Change URI to class constant on SchemaStorage

* add quiet option * use verbose instead of quiet * add quiet option * always output dump-schema * always output dump-schema-url * fix typo and ws

)

* Add test coverage for coercion API * Complete test coverage for SchemaStorage * Add test coverage for ObjectIterator * Add tests for ConstraintError * Add exception test for JsonPointer * MabeEnum\Enum appears to use singletons - add testing const * Don't check this line for coverage mbstring is on all test platforms, so this line will never be reached. * Add test for TypeConstraint::validateTypeNameWording() * Add test for exception on TypeConstraint::validateType() * PHPunit doesn't like an explanation with its @codeCoverageIgnore... * Add various tests for UriRetriever * Add tests for FileGetContents * Add tests for JsonSchema\Uri\Retrievers\Curl * Add missing bad-syntax test file * Restrict ignore to the exception line only * Fix exception scope

* Allow the schema to be an associative array Implements jsonrainbow#388. * Use json_decode(json_encode()) for array -> object cast * Skip exception check on PHP versions < 5.5.0 * Skip test on HHVM, as it's happy to encode resources

…nbow#398)

…w#394) Fixes jsonrainbow#393

…nbow#359) (jsonrainbow#365) * Don't try to fetch files that don't exist Throws an exception when the ref can't be resolved to a useful file URI, rather than waiting for something further down the line to fail after the fact. * Refactor defaults code to use LooseTypeCheck where appropriate * Test for not treating non-containers like arrays * Update comments * Rename variable for clarity * Add CHECK_MODE_ONLY_REQUIRED_DEFAULTS If CHECK_MODE_ONLY_REQUIRED_DEFAULTS is set, then only apply defaults if they are marked as required. * Workaround for $this scope issue on PHP-5.3 * Fix infinite recursion via $ref when applying defaults * Add missing second test for array case * Add test for setting a default value for null * Also fix infinite recursion via $ref for array defaults * Move nested closure into separate method * $parentSchema will always be set when $name is, so don't check it * Handle nulls properly - fixes issue jsonrainbow#377

* Improve performance - don't loop over everything if already valid * Don't coerce already-valid types (fixes jsonrainbow#379) * Add remaining coercion cases & rewrite tests * Add all remaining coercion cases from ajv matrix * Rewrite the coercion tests to tidy things up a bit * Add CHECK_MODE_EARLY_COERCE If set, falls back to the old behavior of coercing to the first compatible type, regardless of whether another already-valid type might be available. * Add multiple-type test that requires coercion * \JSON_PRETTY_PRINT doesn't exist in PHP-5.3, so work around this * Various PR cleanup stuff * Fix whitespace * Turn $early into $extraFlags * Change "string" to "ABC" in string test * Update README.md description of CHECK_MODE_EARLY_COERCE * Move loop after complex tests definition * Move test jsonrainbow#39 to grid jsonrainbow#15

…jsonrainbow#400)

erayd · 2017-03-24T23:59:46Z

Thanks for catching this. What do you think is wrong with that line?

I notice that this PR contains tests, but not a fix. Are you intending to fix this bug, or are you asking us to fix it, and contributing tests?

mathroc · 2017-03-25T00:28:30Z

@flolivaud is a co worker so I can give some more info.

In https://github.com/justinrainbow/json-schema/blob/master/src/JsonSchema/Constraints/UndefinedConstraint.php#L66 I don't think $this->factory->getSchemaStorage()->resolveRefSchema($schema->properties) is right: it’s returning a "properties" object instead of a schema. We have not yet tested it, but dropping the isset($schema->properties) and always passing $schema might be enough

It’s not a blocker for us so I don’t know when we’ll take a look at this, for the moment we're contributing tests. no rush to fix ☺

mathroc · 2017-04-11T07:48:14Z

might be related to /fixed by #411

erayd · 2017-04-11T09:14:01Z

@mathroc I think you're right. I'll check if it's still an issue tomorrow - if it is, then I'll add a fix for this to #411.

erayd · 2017-04-11T19:43:28Z

@mathroc #411 seems to fix it - these tests now pass :-).

@flolivaud Could you please rebase this on 6.0.0-dev?

erayd · 2017-04-11T19:49:03Z

tests/Constraints/MinMaxPropertiesTest.php

@@ -72,7 +72,7 @@ public function getInvalidTests()
        return array(
            array(
                '{
-                  "value": 1


Why is this value being changed, rather than adding the new test independently?

because this test was incorrect, the object was found invalid for the wrong reason (because value was an integer instead of an object)

erayd · 2017-04-11T19:49:11Z

tests/Constraints/MinMaxPropertiesTest.php

                '{
-                  "value": 1


Why is this value being changed, rather than adding the new test independently?

because this test was incorrect, the object was found invalid for the wrong reason (because value was an integer instead of an object)

both tests might be simplified by not having the value wrapping but we chose not to change that to keep the diff simpler

Object validation attempts to use a single variable to store both the object definition, and its properties. This causes validation to be incomplete where "properties" is not set, but "additionalProperties" is. This commit fixes both bugs in issue jsonrainbow#353.

erayd

LGTM. Thanks guys :-).

+ Fix Test

flolivaud · 2017-04-12T07:42:59Z

see #416 for 6.0.0-dev base

erayd

This PR is not OK. It now contains a zillion commits, some of which will break backwards-compatibility.

All that was needed was to change the base of the PR to merge into the 6.0.0-dev branch - that's what I meant when I asked you to rebase it on 6.0.0-dev. Sorry if I phrased this in a way that was unclear. There were no other changes needed here.

The master branch in this repo is frozen, pending merge of 6.0.0-dev into it. New commits do not go there; they will cause merge conflicts. New commits go into 6.0.0-dev, and ones that do not break backwards-compatibility get backported to the 5.x.x branch.

I can see you've opened a duplicate (#416) which looks exactly as this one would have looked after changing the base, so let's just go with that. This one can probably be closed as redundant.

flolivaud · 2017-04-12T08:53:55Z

close duplicate #416

mathroc · 2017-04-12T08:57:12Z

@erayd how do you change the base of a PR ?

erayd · 2017-04-12T09:08:59Z

@mathroc At the top-right of the PR, click the 'Edit' button. Then select the base branch that you wish to merge into from the dropdown menu just below the PR title. Then click 'Save'.

erayd and others added 18 commits March 7, 2017 09:20

centralize errors (jsonrainbow#364)

3dba977

* centralize errors * isolate 'more' info * throw exception for missing error message * swap args

Add use line for InvalidArgumentException (jsonrainbow#370)

a4da6ba

Fixes issue jsonrainbow#369

Add use line for InvalidArgumentException & adjust scope (jsonrainbow…

14a04dc

…#372) Fixes issue jsonrainbow#371

add quiet option (jsonrainbow#382)

d3f7740

* add quiet option * use verbose instead of quiet * add quiet option * always output dump-schema * always output dump-schema-url * fix typo and ws

Add option to disable validation of "format" constraint (jsonrainbow#383

b8d2611

)

Reset errors prior to validation (jsonrainbow#386)

cf8e886

Enable FILTER_FLAG_EMAIL_UNICODE for email format if present (jsonrai…

1567cb6

…nbow#398)

Don't throw exceptions until after checking anyOf / oneOf (jsonrainbo…

c44609a

…w#394) Fixes jsonrainbow#393

add enum wrapper (jsonrainbow#375)

af14372

Add option to also validate the schema (jsonrainbow#357)

e8259a0

Remove stale files from jsonrainbow#357 (obviated by jsonrainbow#362) (…

2553ebd

…jsonrainbow#400)

erayd reviewed Apr 11, 2017

View reviewed changes

This was referenced Apr 11, 2017

[BUGFIX] Split $objectDefinition into $schema and $properties #411

Merged

Backports for 5.2.1 #413

Merged

erayd approved these changes Apr 11, 2017

View reviewed changes

Testcase for minProperties with properties defined

72fc5cb

+ Fix Test

flolivaud force-pushed the master branch from 566938d to 72fc5cb Compare April 12, 2017 07:38

flolivaud mentioned this pull request Apr 12, 2017

Testcase for minProperties with properties defined + Fix Test #416

Merged

erayd requested changes Apr 12, 2017

View reviewed changes

flolivaud closed this Apr 12, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Testcase for minProperties with properties defined + Fix tests #406

Testcase for minProperties with properties defined + Fix tests #406

flolivaud commented Mar 24, 2017

erayd commented Mar 24, 2017

mathroc commented Mar 25, 2017

mathroc commented Apr 11, 2017 •

edited

Loading

erayd commented Apr 11, 2017

erayd commented Apr 11, 2017

erayd Apr 11, 2017

mathroc Apr 11, 2017

erayd Apr 11, 2017

mathroc Apr 11, 2017

mathroc Apr 11, 2017

erayd left a comment

flolivaud commented Apr 12, 2017

erayd left a comment

flolivaud commented Apr 12, 2017

mathroc commented Apr 12, 2017

erayd commented Apr 12, 2017 •

edited

Loading

Testcase for minProperties with properties defined + Fix tests #406

Testcase for minProperties with properties defined + Fix tests #406

Conversation

flolivaud commented Mar 24, 2017

erayd commented Mar 24, 2017

mathroc commented Mar 25, 2017

mathroc commented Apr 11, 2017 • edited Loading

erayd commented Apr 11, 2017

erayd commented Apr 11, 2017

erayd Apr 11, 2017

Choose a reason for hiding this comment

mathroc Apr 11, 2017

Choose a reason for hiding this comment

erayd Apr 11, 2017

Choose a reason for hiding this comment

mathroc Apr 11, 2017

Choose a reason for hiding this comment

mathroc Apr 11, 2017

Choose a reason for hiding this comment

erayd left a comment

Choose a reason for hiding this comment

flolivaud commented Apr 12, 2017

erayd left a comment

Choose a reason for hiding this comment

flolivaud commented Apr 12, 2017

mathroc commented Apr 12, 2017

erayd commented Apr 12, 2017 • edited Loading

mathroc commented Apr 11, 2017 •

edited

Loading

erayd commented Apr 12, 2017 •

edited

Loading