Fix requirement of nested objects

[gabrielgiussi]

The problem

Nested objects mark as required in a swagger Schema Object are translated to optional keys. See the test require-nested-objects for an real use case obtained from the k8s api with chaos-mesh installed.

A possible solution

The solution implemented in this PR is adding a denormalised attribute to prevent being denormalised again which seems to be what is causing the issue. Perhaps someone with a better understanding of the codebase can provide a more principled solution. The main contribution is the test showing which scenario is not working properly.

[oliyh]

Hi,

Thank you for the contribution, it's much appreciated. I will try to find time soon to go through it.

I am in favour of improved test output but unfortunately this library doesn't seem to be compatible with babashka so the babashka build is failing, see CircleCI. Can this library just be used on the JVM?

Thanks

[gabrielgiussi]

I'm not sure, I've tried to follow these instructions but the script is failing with

Error building classpath. Manifest type not detected when finding deps for babashka/babashka.core in coordinate #:local{:root "/Users/gabriel.giussi/repos/babashka/babashka.core"}

So I'll just remove matcher-combinators.

[gabrielgiussi]

It seems we also have to add matcher-combinators in the bb file since it doesn't know about the dev alias defined in project.clj. BTW babashka itself uses matcher-combinators https://github.com/babashka/babashka/blame/e84eb29fa16de78625309d31a853435218bd35fb/deps.edn#L185

[oliyh]

Hi,

Thanks for this, I've had a look and found that the pre-emptive denormalisation of object graphs introduced in #111 has resulted in this behaviour. It is unneccessary, because the object graph is walked anyway, but it did have the benefit of disambiguating the required field which differs between leaves and objects.

I've done some work on a different approach which passes all tests except for the one introduced for #111, but I think it's nearly there.

https://github.com/oliyh/martian/compare/master...fix-required-nested-objects

[gabrielgiussi]

Great, thanks for taking a look to this. Feel free to close this PR if you are going to be working on that branch.

[oliyh]

I'll leave it open as a reminder to me if you don't mind, and I'll update you here when I've got something into the main branch.

Cheers

[oliyh]

Hi,

Apologies for the delayed response. I have decided that the test failed because we have in fact improved the behaviour - it had an optional key which was itself an object, which had two required parameters - so the object could not be a maybe. Merged in #197

Thank you for your work! I will publish a snapshot to clojars and a release soon.

Cheers

[oliyh]

Released in 0.1.27-SNAPSHOT