closes #293: allow empty options object in all commands

[ivansenic]

What this PR does:

Allows mongoose to send empty options object even when a command does not support any options. Confirmed it was failing before.

Which issue(s) this PR fixes:
Fixes #293

Checklist

• Check all commands in integration tests

[ivansenic]

@tatu-at-datastax any better idea?

[tatu-at-datastax]

We could (and probably should) use

 @JsonIgnoreProperties({ "options" })
 public interface Command {}

in a base class/interface. It can be applied either with annotations or using "configOverrides" (let me dig up reference), or using mix-in annotations.
This type of ignoral is only used in case there is no explicit property (unlike @JsonIgnore that forcibly prevents use) so it should not prevent use of non-empty "ignores" cases.

It'd probably be better to do that so we don't accidentally hide properties with typos etc.

Also: we could enable "fail all unknown" just on Command types with

 @JsonIgnoreProperties(ignoreUnknown = true)

and not globally.

[tatu-at-datastax]

Ok so "Config overrides" work like so:

        ObjectMapper mapper = JsonMapper.builder()
            .withConfigOverride(Point.class,
                cfg -> cfg.setIgnorals(JsonIgnoreProperties.Value.forIgnoredProperties("y")))
            .build();

and let one apply equivalent of @JsonIgnoreProperties on any type. But one big limitation is that it only works on exact target class, but not base class.

So I think what we can use is:

@JsonIgnoreProperties({ "options" })
public interface Command {}

[ivansenic]

Done in 50a276f

[ivansenic]

The proposal does not work, it completely omits options even in the commands that should have them.. See https://github.com/stargate/jsonapi/actions/runs/4562316051/jobs/8049363613

Reverting to previous solution..

[tatu-at-datastax]

Alas. Sorry about suggesting that; you are right.

[ivansenic]

@tatu-at-datastax @maheshrajamani Can we try to keep the integration tests clean and readable please, by following the below guidelines.. I know I can be perfectionist, but it's way better when we have everything nice and readable:

• format jsons, yes it requires manual interaction cause the stupid formatter we use does not format multi line strings
• remove Order annotation when they are not needed, they are making confusion if they are placed and mean nothing
• name tests byId() rather than deleteManyById(), we already have top level and nested classes describing the op, it looks way better:
  

Thanks..

[tatu-at-datastax]

@ivansenic On clean up, naming; good ideas. Only one counterpoint: for bigger tests (which is most ITs over time), context of single test makes it less easy to see the surrounding context. So sometimes little bit of redundancy on actual method name is useful when editing tests, scrolling back and forth. I realize it won't be necessary on left-side result tree, but as long as it's nothing super long may be useful.

Thank you for doing the cleanup; I have started dropping @Order ones when modifying existing tests, and will do the same for names.
Indentation is trickier because I honestly am not sure what is the expectation; and we may even have different preferences wrt everything aligned, minimal indentation and so on. But I'll do my best.

[ivansenic]

@ivansenic On clean up, naming; good ideas. Only one counterpoint: for bigger tests (which is most ITs over time), context of single test makes it less easy to see the surrounding context. So sometimes little bit of redundancy on actual method name is useful when editing tests, scrolling back and forth. I realize it won't be necessary on left-side result tree, but as long as it's nothing super long may be useful.

Thank you for doing the cleanup; I have started dropping @Order ones when modifying existing tests, and will do the same for names. Indentation is trickier because I honestly am not sure what is the expectation; and we may even have different preferences wrt everything aligned, minimal indentation and so on. But I'll do my best.

Thanks for understanding.. Names are fine, we can not get wrong with that..

Regarding json, I expect formatted JSON, with 2 spaces indentation, smth like:

https://github.com/stargate/jsonapi/blob/50a276f4e92f6aeda143f4cb75942b7187f1a00f/src/test/java/io/stargate/sgv2/jsonapi/api/v1/CountIntegrationTest.java#L26-L57

[tatu-at-datastax]

@ivansenic On clean up, naming; good ideas. Only one counterpoint: for bigger tests (which is most ITs over time), context of single test makes it less easy to see the surrounding context. So sometimes little bit of redundancy on actual method name is useful when editing tests, scrolling back and forth. I realize it won't be necessary on left-side result tree, but as long as it's nothing super long may be useful.
Thank you for doing the cleanup; I have started dropping @Order ones when modifying existing tests, and will do the same for names. Indentation is trickier because I honestly am not sure what is the expectation; and we may even have different preferences wrt everything aligned, minimal indentation and so on. But I'll do my best.

Thanks for understanding.. Names are fine, we can not get wrong with that..

Regarding json, I expect formatted JSON, with 2 spaces indentation, smth like:

Ah ok, yes, that's fine, makes sense.

[tatu-at-datastax]

@ivansenic I do have alternate solution to ignoral; instead of @JsonIgnoreProperties (which unfortunately does hard ignoral), I think this:

#317

works. Basically default "setter" in Command interface will drop "options" unless sub-class defines it (by having options Record field). We could even add basic validation that "options" is empty if we want to.

[vkarpov15]

LGTM 👍