pact-foundation / pact-specification

Describes the pact format and verification specifications
MIT License
295 stars 28 forks source link

Matcher for dynamic Object keys #47

Closed Mende closed 2 years ago

Mende commented 6 years ago

Pact Matcher for objects with dynamic keys

My use case is pretty specific but i feel like it might be common. I have an systemMessage object that has a very consistent schema that i defined like this

export const systemMessage = {
  id: like(123456),
  message: like('some text here'),
  priority: term({
    matcher: 'LOW|MEDIUM|HIGH|CRITICAL',
    generate: 'HIGH'
  }),
  destination: term({
    matcher: 'EVERYWHERE|SOMEWHERE|ELSEWHERE',
    generate: 'EVERYWHERE'
  })
};

I return a very high volume of these objects and to simplify my render process I use a hash where each message is keyed by it's ID.

const systemMessages = {
    123456:  MessageWithID123456
    ....
}

Since the objects contained in the hash are of a specific type I would like a function akin to eachLike for objects that simply validates that the values in the object conform to the pattern above and ignores the keys.

{
    systemMessages: objectLike({
      id: like(123456),
      message: like('some text here'),
      priority: term({
        matcher: 'LOW|MEDIUM|HIGH|CRITICAL',
        generate: 'HIGH'
      }),
      destination: term({
        matcher: 'EVERYWHERE|CMS',
        generate: 'EVERYWHERE'
      })
    })
}

I'm including a link to the react-immutable-proptypes project. They have a function called ImmutablePropTypes.mapOf() which does what I described. https://github.com/HurricaneJames/react-immutable-proptypes#api

mefellows commented 6 years ago

Thanks for posting this detailed request @mende.

I actually asked a few people at my place of work when this came up on the forums, and unanimously this was seen as unusual to be exposed as an API. So I'm skeptical of it being a common case.

This will take some serious thought as it is pretty core to any matching logic, although you could see how it might be implemented from a users perspective.

uglyog commented 6 years ago

Here is an example of a test that shows this behaviour in Pact-JVM: https://github.com/DiUS/pact-jvm/blob/master/pact-jvm-consumer-junit/src/test/java/au/com/dius/pact/consumer/WildcardKeysTest.java

mefellows commented 6 years ago

I stand corrected

uglyog commented 6 years ago

I propose we add a ignoreKeys flag to the matcher defined on the map. The current implementation in Pact-JVM is a work around and is causing other issues. I'll add this to the V4 spec.

uglyog commented 6 years ago

After some thought, I've decided a matchValues matcher defined on the map would be better. See https://github.com/pact-foundation/pact-specification/blob/version-4/README.md#ignoring-the-keys-in-a-map

joelgithub commented 6 years ago

Will this solve #401?

uglyog commented 6 years ago

@joelgithub hopefully it will

quincy commented 6 years ago

I have a slightly different, but related, use case. https://stackoverflow.com/questions/50532051/pact-how-do-i-match-an-object-whose-keys-match-a-regular-expression

I have a domain object which we call a Schedule which is really nothing more than an ordered map. The keys are dates. You set an entry in the map to denote that the value for the Schedule is that new value on the given date, and all subsequent dates until the next highest valued key.

For example:

[
  {
      "accountId" : 1,
      "permissions" : [
         {
            "schedule" : {
               "01/01/2018" : false,
               "01/01/1900" : true
            },
            "permissionId" : 3
         }
      ]
   }
]

I would like to ensure that the proposed new functionality covers the case of mapping matched keys to primitive values as well as objects and arrays. I don't believe a change to the proposal is necessary for this, because as it is currently worded it doesn't preclude my use case.

Thanks!

andersthorbeck commented 6 years ago

I also have a use case similar to https://github.com/pact-foundation/pact-specification/issues/47#issuecomment-392801836, where we are storing dates as keys.

The JSON we are trying to match looks something like this

{
  "id": {
    "type": "doc",
    "date": "2018-07-31",
    "seq": 123
  },
  "versions": {
    "2018-09-01": [
      {
        "status": "draft",
        "shortTitle": "My draft title"
      },
      {
        "status": "published",
        "shortTitle": "My published title"
      }
    ],
    "2018-08-02": [
      {
        "status": "published",
        "shortTitle": "My published title"
      }
    ]
  }
}

The versions JSON object has dynamic keys, where each key is a date. Ideally, we would like our pact to be able to verify both the structure of the values mapped to by these keys (array containing objects containing "status" and "shortTitle" keys, both of type string) and to be able to verify that each key matches an ISO8601 date format (ref. the date type).

I actually asked a few people at my place of work when this came up on the forums, and unanimously this was seen as unusual to be exposed as an API. So I'm skeptical of it being a common case.

This does not seem to me as a particularly outlandish use case. If we generalize this, what we really want is to be able embed a generic map (one of the most commonly used data structures) in the JSON response of one of our API endpoints, and to be able to use pact to verify the structure of both the values and the keys of that map.

mefellows commented 6 years ago

Just because a map is a ubiquitous concept doesn't mean it should be dumped into an API design.

APIs should be designed, not simply a reflection of internal data structures.

Nevertheless I'm happily convinced there are enough cases in the wild to support dynamic keys and this feature.

quincy commented 6 years ago

@mefellows, thank you for your feedback. I agree that APIs should be designed. However, many of us are working on legacy projects which were created prior to Pact. We don't necessarily have the luxury of redesigning the API at this time but we'd still like to get contracts in place.

Let's say I was redesigning my API though. It is the job of this particular provider service to transmit domain objects to its consumers. These domain objects are schedules of values keyed by date. I can imagine transforming the data from something like the following:

[
  {
      "accountId" : 1,
      "permissions" : [
         {
            "schedule" : {
               "01/01/2018" : false,
               "01/01/1900" : true
            },
            "permissionId" : 3
         }
      ]
   }
]

to something like this instead:

[
  {
      "accountId" : 1,
      "permissions" : [
         {
            "schedule" : [
              { "01/01/2018" : false },
              { "01/01/1900" : true }
            ],
            "permissionId" : 3
         }
      ]
   }
]

That would make the API work with Pact as it is today. But I fail to see how this is fundamentally a better designed API. Perhaps you had something else in mind and I simply can't see it at the moment. Can you offer guidance on how something like this could be improved? Do you have some literature you could refer me to?

I am not trying to be argumentative, but I would like to understand your comment and make improvements in the work I do in the future.

mefellows commented 6 years ago

Totally agree with you on the retrofitting use case, which is another strong reason to support this.

Whether I agree with it or not doesn't matter, the fact that it is common in the real world is a good enough reason to support it.

FWIW I would have each item within schedule mapped to something more like the following:

[
  {
      "accountId" : 1,
      "permissions" : [
         {
            "schedule" : [
                { 
                    "date": "01/01/2018",
                    "enabled": true 
                },
                { 
                    "date": "01/01/1900",
                    "enabled": false 
                },
            ],
            "permissionId" : 3
         }
      ]
   }
]

Having each schedule as a domain object keyed by well-defined names allows greater flexibility in the model should you choose to expand, and is more easily modelled by today's tooling (albeit OAS will be able to sort-of support this down the track: https://github.com/OAI/OpenAPI-Specification/issues/1505)

quincy commented 6 years ago

Thanks for sharing your opinion @mefellows. I appreciate the insight.

tcanavan commented 5 years ago

Will this be implemented in 4.0.0?

uglyog commented 5 years ago

Not necessarily. The new matcher is added behaviour, and doesn't change the structure of the pact file.

uglyog commented 3 years ago

Released values matcher with Pact-JVM and Pact-Rust