search

pboettch / json-schema-validator

JSON schema validator for JSON for Modern C++
Other
466 stars 134 forks source link

Default generation #89

Closed Finkman closed 4 years ago

Finkman commented 4 years ago

This is my contribution that targets #25. It brings almost no impact on the public interface which means that it would not break previous usage.

It adds a return value to the validation call which contains a patch (RFC 6902) which is supported by the nlohmann::json. See: JSON Pointer and JSON Patch

I would appreciate comments, changes and suggestions a lot.

(Close #25)

Finkman commented 4 years ago

We would need some unit-tests ideally before merging.

Yes, of course. But I need a little time to investigate into cmake-tests.

Finkman commented 4 years ago

Are more UnitTests are needed? What kind?

pboettch commented 4 years ago

I approved the changes and then did a ctest: 

99% tests passed, 1 tests failed out of 70

Total Test time (real) =  12.35 sec

The following tests FAILED:
      5 - issue-25-default-values (Failed)
Errors while running CTest
pboettch commented 4 years ago

The is the JSON-patch I'm getting:

[[],{"op":"add","path":"/address/street","value":"Abbey Road"}]
Finkman commented 4 years ago

hmm.. that's odd. I get

...
[ctest] 67/69 Test  #5: issue-25-default-values ...............................   Passed    0.01 sec
...

for commit 6709194

pboettch commented 4 years ago

What version of nlohmann::json? and what gcc/clang?

pboettch commented 4 years ago

It works for me when I change the constructor of jsonpatch to ` j(R"([])"_json)` instead of using the braces-initializer.

It also works, if I do not set an initializer at all.

Finkman commented 4 years ago

What version of nlohmann::json? and what gcc/clang?

I'm using json 3.7.3 and clang 8.0.1

Finkman commented 4 years ago

Now, it should work-out fine. I am using the default initializer.

pboettch commented 4 years ago

Integrated. Thanks a lot.