pact-foundation / pact-reference

Reference implementations for the pact specifications
https://pact.io
MIT License
91 stars 46 forks source link

FFI 'values' matcher doesn't appear to support nesting #216

Closed TimothyJones closed 1 year ago

TimothyJones commented 1 year ago

From slack discussion here

Zebi reported that the following object:

{
  "john-doe1": {
    "brown-fox": {
      "jumps": "over",
      "the": "lazy dog"
    }
  },
  "john-doe2": {
    "brown-fox2": {
      "jumps": "over",
      "the": "lazy dog"
    }
  }
}

isn't matched by this matcher combination in Pact-JS:

Matchers.eachKeyLike(
   "some string", 
   Matchers.eachKeyLike(
      "some-string", 
      Matchers.eachKeyLike(
         "some-string",
         Matchers.string())))

The expected behaviour is for the keys to be ignored at each level. The actual behaviour is that the keys are only ignored at the first level.

From the discussion with @uglyog , the problem is likely to lie in the way the FFI sets up the matcher paths. It may also lie in the Pact-JS matcher definition. Here's the definition in Pact-JS:

export const eachKeyLike = <T extends AnyTemplate>(
  keyTemplate: string,
  template: T
): Matcher<AnyTemplate> => ({
  'pact:matcher:type': 'values',
  value: {
    [keyTemplate]: template,
  },
});

This means that the object that would be passed to the FFI is:

// Note, I hand crafted this
{
  'pact:matcher:type': 'values',
  value: {
    'some-string': {
      'pact:matcher:type': 'values',
      value: {
        'some-string': {
          'pact:matcher:type': 'values',
          value: {
            'some-string': {
              'pact:matcher:type': 'type',
              value: 'some string',
            },
          },
        },
      },
    },
  },
}
rholshausen commented 1 year ago

It's not the FFI that is the problem, the above commit adds a test that works as expected. I'll check the matching logic to see if the problem is there.

rholshausen commented 1 year ago

No, it is the paths that are wrong. It sets up rules as $['some-string'] and then fails with Actual map is missing the following keys: some-string. It should be using *.

rholshausen commented 1 year ago

BTW, the eachKeyLike function is incorrectly named. It should be something like eachValueLike.

zzeebbii commented 1 year ago

@rholshausen is it fixed now with your commit? Can I try it or should I wait for some release?

rholshausen commented 1 year ago

Still some changes are required, I'll get a release out today

rholshausen commented 1 year ago

Pact FFI Library 0.3.12 is released with the fix

mefellows commented 1 year ago

Thanks Ron. I'll push out an update to the core today.

I'll separately consider how to evolve the function name based on our (separate) discussion in Slack.

TimothyJones commented 1 year ago

Haha! I was just reporting this here: https://github.com/pact-foundation/pact-js/issues/952

eburi commented 1 year ago

I'm using the eachKeyLike() and the resulting matchingRules are broken. I think it might be related to this.

willRespondWith: {
    status: 200,
    headers: {
        'Content-Type': 'application/json',
    },
    body: {
        vrskauffallnummer: string('660189354651'),
        angebote: eachKeyLike(
            'CLASSIC',
            {
                angebotStatusTechnisch: string('TECHNISCH_KONSISTENT'),
            }
        ),
        versicherungsproduktspezifischeDaten: {
            objektdaten: eachLike({
                baCode: like({
                    code: integer(),
                }),
            }),
        },
    },
}

The resulting matchingRules are:

  "matchingRules": {
    "body": {
      "$.angebote": {
        "combine": "AND",
        "matchers": [
          {
            "match": "values"
          }
        ]
      },
      "$.angebote.*.*": {
        "combine": "AND",
        "matchers": [
          {
            "match": "type"
          }
        ]
      },
      "$.angeboteNeuInitialisiert": {
        "combine": "AND",
        "matchers": [
          {
            "match": "type"
          }
        ]
      },
      "$.versicherungsproduktspezifischeDaten.objektdaten": {
        "combine": "AND",
        "matchers": [
          {
            "match": "type",
            "min": 1
          }
        ]
      },
      "$.versicherungsproduktspezifischeDaten.objektdaten[*].baCode": {
        "combine": "AND",
        "matchers": [
          {
            "match": "type"
          }
        ]
      },
      "$.versicherungsproduktspezifischeDaten.objektdaten[*].baCode.code": {
        "combine": "AND",
        "matchers": [
          {
            "match": "integer"
          }
        ]
      },
      "$.vrskauffallnummer": {
        "combine": "AND",
        "matchers": [
          {
            "match": "type"
          }
        ]
      }
    },
    "header": {}
  },
  "status": 200
}

IMHO the path for the matchingRule from the everKeyLike() should be: $.angebote.*.angebotStatusTechnisch similar to what is generated for eachLike() (e.g. $.versicherungsproduktspezifischeDaten.objektdaten[*].baCode)

Versions:

"metadata": {
  "pact-js": { "version": "11.0.2"},
  "pactRust": { 
    "ffi": "0.4.0",
    "models": "1.0.4"
  },
  "pactSpecification": { "version": "3.0.0" }
},
eburi commented 1 year ago

Note: When extending the template given to everyKeyLike(), the matcher are simply taken together, where I would expect the matchers to show up for separate paths to know what matcher needs to applied to what field.

Extending the above example with:

angebote: eachKeyLike(
    'CLASSIC',
    {
        angebotStatusTechnisch: string('TECHNISCH_KONSISTENT'),
        statusCode: number(100)
    }
),

will result in the corresponding matchingRule:

"$.angebote.*.*": {
  "combine": "AND",
  "matchers": [
    {
      "match": "type"
    },
    {
      "match": "number"
    }
  ]
},

Where I would expect:

"$.angebote.*.angebotStatusTechnisch": {
  "combine": "AND",
  "matchers": [
    {
      "match": "type"
    },
  ]
},
"$.angebote.*.statusCode": {
  "combine": "AND",
  "matchers": [
    {
      "match": "number"
    }
  ]
},
rholshausen commented 1 year ago

@eburi eachKeyLike is meant to set matchers on the keys which are strings. I think the actual error is being able to specify

eachKeyLike(
    'CLASSIC',
    {
        angebotStatusTechnisch: string('TECHNISCH_KONSISTENT'),
        statusCode: number(100)
    }
),

where it should only support something like eachKeyLike(regex('\\w+', 'CLASSIC'))