Fix NPE

[lmartelli]

Fixes https://github.com/webcompere/model-assert/issues/45

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (5873e85) 97.50% compared to head (e976591) 97.50%.

:exclamation: Current head e976591 differs from pull request most recent head 5478303. Consider uploading reports for the commit 5478303 to get more accurate results

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #46 +/- ## ========================================= Coverage 97.50% 97.50% - Complexity 481 482 +1 ========================================= Files 64 64 Lines 840 842 +2 Branches 63 64 +1 ========================================= + Hits 819 821 +2 Misses 12 12 Partials 9 9 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[ashleyfrieze]

Shall we add the test that characterises the bug?

I'm not sure that the null check ought to be done like this either. What if we had a rule that said "must be null" for the key?

Let's start with the tests that fail with the existing code, let's expand them for other edge conditions.

Then maybe we can quit the function early if there's no way the rule can pass.?

[lmartelli]

I've just added the test. Need help for a meaningful name ;-)

[lmartelli]

Shall we add the test that characterises the bug?

I'm not sure that the null check ought to be done like this either. What if we had a rule that said "must be null" for the key?

Yes, you are right. I could not find where to put it elsewhere at first sight.

Because in PredicateWrappedCondition the typeDetector Predicate is a lambda, and I don't know where it is built.

[lmartelli]

It seems more appropriate to do the test in PredicateWrappedCondition, don't you think ?

[ashleyfrieze]

@lmartelli - thanks for trying to fix this. I think you're right that the fix belongs in PredicateWrappedCondition. Please rename the test, and then I'll merge this fix and release it.

[ashleyfrieze]

The various numeric conditions are a bonus. Though JSON doesn't have more than number as a type, Java does, and we can still assert the convertability to other formats of the number, so there's no problem with isInteger etc.

[lmartelli]

The various numeric conditions are a bonus. Though JSON doesn't have more than number as a type, Java does, and we can still assert the convertability to other formats of the number, so there's no problem with isInteger etc.

My point is that it is currently a little cumbersome if you do not care about the Java type, which is a bit odd in a JSON context. Typical use of JSON is web API, where Java type are irrelevant.

Also, isNumber() is too broad : you usually want to distinguish integers and decimal numbers.

How about adding isIntegralNumber() ?

[ashleyfrieze]

@lmartelli - please raise a fresh PR if you wish to try adding more variants of type detection.

From JsonNode:

    public final boolean isPojo() {
        return this.getNodeType() == JsonNodeType.POJO;
    }

    public final boolean isNumber() {
        return this.getNodeType() == JsonNodeType.NUMBER;
    }

    public boolean isIntegralNumber() {
        return false;
    }

    public boolean isFloatingPointNumber() {
        return false;
    }

    public boolean isShort() {
        return false;
    }

    public boolean isInt() {
        return false;
    }

    public boolean isLong() {
        return false;
    }

    public boolean isFloat() {
        return false;
    }

    public boolean isDouble() {
        return false;
    }

    public boolean isBigDecimal() {
        return false;
    }

    public boolean isBigInteger() {
        return false;
    }

    public final boolean isTextual() {
        return this.getNodeType() == JsonNodeType.STRING;
    }

    public final boolean isBoolean() {
        return this.getNodeType() == JsonNodeType.BOOLEAN;
    }

    public final boolean isNull() {
        return this.getNodeType() == JsonNodeType.NULL;
    }

    public final boolean isBinary() {
        return this.getNodeType() == JsonNodeType.BINARY;
    }

So, theoretically, we could provide examples of all of these. It may be better to write tests for any ones you wish to expose inside of WholeTreeUseCaseTest.java rather than the file you've added tests for in this issue.

[ashleyfrieze]

@lmartelli - I've triggered release 1.0.2 - takes a few hours to propagate to your favourite maven repo.

[lmartelli]

@lmartelli - I've triggered release 1.0.2 - takes a few hours to propagate to your favourite maven repo.

Thanks, happy new year 🥳