Adds NonStrictArrayMatcher which will matches softly value and pattern

[javierseixas]

After studying a bit issue #39 , I found out that the only thing required to be able to match softly a JSON, would be having a new ArrayMatcher, being able to keep going through the array instead of returning an error when there is a value not contained in the pattern.
So here it is my proposal of creating a new NonStrictArrayMatcher, a subtype of the ArrayMatcher, where only changes the iterateMatch() method.

So, to get what I requested on issue #39 it would be only needed to create a JsonMatcher containing a NonStrictArrayMatcher.

[benbor]

+1

[norberttech]

Sorry @javierseixas I'm 👎 for this one. I dont like that fact that value match the pattern when it contains more elements than pattern itself. I think that @json@ type pattern with proper expanders would be much more clean and we will avoid doing magic.
I'm going to work in few days on a @JSON@ type pattern draft and then we could decide which way should we take.

[javierseixas]

@norzechowicz thanks for taking into account the pull request. Why do you think it adds magic?

Regarding the solution itself, I liked your proposal as well, but what I wasn't convince because of the legibility in a Behat feature file. Your sample looked great, however I was wondering how it would be with a more complex json value.

{
    "id": "@string@",
    "title": "My Title",
    "short_description": "@string@",
    "campaign_start": "@string@",
    "campaign_end": "@string@",
    "status": "@integer@",
    "_links": "@wildcard@",
    "_embedded": {
        "products": [
            {
                "name": "My product 1",
                "price": "@double@",
                "description": "@string@",
                "quantity": "@integer@"
            },
            {
                "name": "My product 2",
                "price": "@double@",
                "description": "@string@",
                "quantity": "@integer@"
            }
        ],
        "account": {
            "id": "@string@",
            "name": "foo",
            "society_name": "@string@",
            "_links": "@wildcard@"
        }
    }
}

With the solution in this pull request, would be like this:

{
    "title": "My Title",
    "status": 2,
    "_embedded": {
        "products": [
            {
                "name": "foo"
            },
            {
                "name": "bar"
            }
        ],
        "account": {
            "name": "foo"
        }
    }
}

[javierseixas]

@norzechowicz hello again!

After thinking a bit more, I want to propose a solution to make clearer this non strict work. I was inspired by the strictness solution of JSON Expression.

My proposal is to have a pattern, as I first proposed in the issue #39 but in the following way:

{
    "title": "My Title",
    "status": 2,
    "_embedded": {
        "account": {
            "name": "foo"
        }
    }
}@ignore_extra_keys@

It would keep the NonStrictArrayMatcher, but doing some modifications in the JsonMatcher we can have this solution.

What do you think?

[norberttech]

Yea looks a lot better but now we need to figure out how to pass this "ignore_extra_keys" into JsonMatcher in order to use NonStrictArrayMatcher instead of ArrayMatcher.

[javierseixas]

See my second commit in this pull request 😄

[norberttech]

Interesting but @something@ is a type pattern definition when @ignore_extra_keys@ is more like for example "multiline" modifier in regular expressions. Maybe we should think about some more generic solution for that? Maybe we could add modifiers for type patterns that would be nothing more that options removed from pattern during parsing and passed to pattern?

[javierseixas]

Ok, make sense. So would me something like instead of @ignore_extra_keys@ use .ignore_extra_keys!?

[norberttech]

Most important I think its to create a pattern for those modifiers. Maybe |modifier1,modifier2|?
So your exapmle would be like:

{
    "title": "My Title",
    "status": 2,
    "_embedded": {
        "account": {
            "name": "foo"
        }
    }
}|ignore_extra_keys|

[norberttech]

We can also consider moving modifiers at the beggining of the pattern. So we can assume that everything that is between | at the beggining of each patter is a set of modifiers so before executing "caMatch" in each matcher we need to remove this part of pattern.
So your example could look like that:

|ignore_extra_keys|
{
    "title": "My Title",
    "status": 2,
    "_embedded": {
        "account": {
            "name": "foo"
        }
    }
}

Or in simpler cases |ignore_extra_keys,case_insensitive|{title: "test", id: @integer@}

[javierseixas]

I really like the idea! I'm not sure if I understood well the concept modifier though. What does it modify? For me, it does not modify the value, neither the pattern (it is just removed from the pattern). What it modifies is the behaviour of the matcher. Are you agree? So, I'm not sure about calling them modifiers. I'd vote for "strictnessers" (A made up word :P). For me it would make sense for the two samples ignore_extra_keysand case_insensitive.

I agree to put it them at the beginning of the json and between |.

I'll work on this for a couple of days.

[norberttech]

Indeed those values would modifie the matcher behavior. I came up with modifiers because there is very similar thing in regular expression, you can add it at the end of expression and it will modifie the pattern for example by ignoring case sensitive.
But I'm open for suggestions.
And @javierseixas thanks for your work, I really appreciate your help.

[javierseixas]

Hi! I've been busy the last days. Now I'm working on these improvements. @norzechowicz you're welcome. It's a pleasure to work on this :)

I 've started working on that, and as far as I go more doubts about how to do it come to my mind.
In my proposal, the ignore_extra_keys is done in the JsonMatcher level, just injecting a second ChainMatcherwithin a NonStrictArrayMatcherinside.
In the other hand, I've seen that for doing a case_insensitive matching, there are two options. The first one could be to modify (in some way) the ScalarMatcher making it optionally case insensitive. The second option could be to change $valueand $pattern forcing them all to be lower case.
In a first instance I was going to chose the second one, because the first one would require to inject a CaseInsensitveScalarMatcher. I didn't like too much the fact of modifying the $value and $pattern, forcing them to match though.

And when I was doing that, and I was thinking in injecting to the JsonMatcher a kind of ChainModifier, I realised that both modifiers where doing things in different levels. ignore_extra_keys was done by injection and to the constructor, and case_insensitive by modifying value and pattern. So I thought that if I want to use some IgnoreExtraKeysModifier and CaseInsensitiveModifier (and I think we should), I should first standarize how a Modifier works. So summarizing:

1. A modifier could do its job by changing the value and the pattern and then they would match in the normal way the php-matcher does. For example: For the case_insensitive would transform all values in the arrays (value and pattern) to lowercase, or for the ignore_extra_keys by deleting from the value the keys that the pattern doesn't have.
2. The other is that the modifier could change the Matcher already injected in the JsonMatcher, for example, changing the ScalarMatcher for a CaseInsensitiveScalarMatcher or the ArrayMatcher for a IgnoreExtraKeysArrayMatcher.

And here I am. Personally, I think the option 2 is cleaner, separating classes with its own behaviour, but the fact of changing the injected object on the fly depending on the modifiers passed to the pattern is quite magic. Besides, I don't really like the fact of modifying value and pattern forcing them to match.

So please, let me know what you think about it or if you have any question about the explanations. I'm very open to suggestions as well :)

Cheers!

[norberttech]

Changing value/pattern is not a good solution. Method 2 sounds better but... ;) I dont think that we need to replace "loaded" matchers. IMO modifier is more like a option for specific matcher. So lets take "case_insnensitive" modifier for example.

$pattern = '|case_insensitive|{id: 12345, name:"Norbert Orzechowicz"}'

In such situation json matcher should do 3 things.

1. Remove modifiers from $pattern inside of canMatch method in order to get {id: 12345, name:"Norbert Orzechowicz"}' pattern and this is what should be done using Coduo\PHPMatcher\Parser

    /**
     * {@inheritDoc}
     */
    public function canMatch($pattern)
    {
        if (!is_string($pattern)) {
            return false;
        }

        //remove modifiers in transformPattern method
        return $this->isValidJson($this->transformPattern($pattern)); 
    }

2. Parse modifiers string, create modifiers (objects) collection (as there might be more than one modifier) and pass modifiers to each matcher that can handle them.

In order to do so we need following methods in Coduo\PHPMatcher\Parser

• public function parseTypePattern($pattern) (current parse method)
• public function trimModifiers($pattern) - returns pattern without modifiers.
• public function parseModifiers($pattern) - returns collection od modifiers (if there are not modifiers in pattern then return empty collection)

We also need to add 2 methods into Coduo\PHPMatcher\Matcher\ValueMatcher interface.

• public function allowsModifier(Modifier $modifier); - feel free to think about better name :P Basicaly this method will return boolean that will tell us if matcher behavior can be modified by specific modifier.
• public function modify(Modifier $modifier); - if allowsModifier return true then we can safely modify matcher behavior by passing $modifier into it.

    public function match($value, $pattern)
    {
        if (!is_string($value) || !$this->isValidJson($value)) {
            return false;
        }

        $this->modifyMatchers($this->parser->parseModifiers($pattern));

        $pattern = $this->transformPattern($pattern); // remove modifiers here using trimModifiers
        $match = $this->matcher->match(json_decode($value, true), json_decode($pattern, true));
        if (!$match) {
            $this->error = $this->matcher->getError();
            return false;
        }
        return true;
    }

modifyMatchers will simply iterate through all matchers (if there are more than 1 modifier) and check if any matcher can handle any modifier.

3. Match value with modified matchers.

Thanks to this approach there will be no need to create IgnoreExtraKeysArrayMatcher or CaseInsensitveScalarMatcher because ScalarMatcher/ArrayMatcher behavior could be simply modified by CaseInsensitive/IgnoreExtraKeys modifiers.

There is only one important thing that we need to keep in minds during implementation. Modifiers should be passed through each matcher than can make use of them.

For example current JsonMatcher in order to work require ArrayMatcher that require ScalarMatcher.
So in this situation CaseInsensitive modifiers should be passed from JsonMatcher to ArrayMatcher and finally to ScalarMatcher. All JsonMatcher, ArrayMatcher and ScalarMatcher should return true in method allowsModifier for CaseInsensitive matcher, first two should only pass modifier deeper and last one should use it.

Of course feedback is more than welcome @javierseixas ;)

[javierseixas]

Hi! Just say that I don't forget this pull request. I've been very busy the last weeks. Hoping to get some free time next month to advance on this :)

[norberttech]

Hey @javierseixas thanks for information. PR is open, take as long as you need to finish it and let me know if you will need some help.

[norberttech]

@javierseixas I'm closing this PR for now, if u will find some time to work on it ping me and I will reopen it or create new one on your own.

[javierseixas]

@norzechowicz no problem. I'm sorry I haven't had the time to work on this. Thanks!

[koemeet]

👍 Like this idea a lot! any update on this?

[vitalyiegorov]

👍Any news on this? @javierseixas

[javierseixas]

Hi @vitalyiegorov ! No news actually. I haven't been programming with php for a while, so I stop needing that feature. I can say though other languages like java have it.

[vitalyiegorov]

Maybe we can merge this PR and enable this feature? вс, 3 июня 2018 г. в 17:15, Javier Seixas <[email protected]>:

…

Hi @vitalyiegorov <https://github.com/vitalyiegorov> ! No news actually. I haven't been programming with php for a while, so I stop needing that feature. I can say though other languages like java have it. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#40 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAjzPtp2wITK2W0tvU0rT_RrpS9d6YIEks5t4--HgaJpZM4DFS0C> .

[javierseixas]

My PR is from 2015. Not sure if is still compatible with the current version of the project.
Besides, I didn't apply the last suggestions from the owners of the library, see comment above.
In any case, it depends on them. Or maybe someone else can carry on with that PR.