pact-foundation / pact-reference

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

Date and Time Matchers Can Generate Invalid Pacts #265

Closed adamrodger closed 1 year ago

adamrodger commented 1 year ago

I have a local change to add the Timestamp, Date and Time matchers for PactNet and the feature is really confusing because of all the different languages in play.

The spec says that the format strings follow the Java date/time format strings, but .Net uses different ones. This creates really unexpected behaviour, e.g. if I do something like this using the idiomatic .Net format strings:

const string format = "HH:mm:ss.fff";
var matcher = Match.Time(DateTime.Now, format);

then the Pact file will quite happily write out a format which is completely invalid:

"$.body[*].time": {
    "format": "HH:mm:ss.fff",
    "match": "time"
}

This then fails at verification time because the format can't be parsed, which means consumers can generate invalid pacts with no errors:

$[1].time -> Expected '"01:37:52.261"' to match a timestamp format of 'HH:mm:ss.fff': Error parsing '01:37:52.261': "Parsing datetime pattern 'HH:mm:ss.fff' failed at text 'HH:mm:ss.fff'"

If I do the opposite though and specify a Java format like HH:mm:ss.SSS then I can't verify that on the .Net side, which creates totally unidiomatic code like:

const string format = "HH:mm:ss.SSS"; // this isn't a valid .Net format string
var matcher = Match.Time(DateTime.Now, format); // how can I now format this properly given the format string isn't valid .Net?

I'd be left with either:

github-actions[bot] commented 1 year ago

👋 Hi! The 'smartbear-supported' label has just been added to this issue, which will create an internal tracking ticket in PactFlow's Jira (PACT-938). We will use this to prioritise and assign a team member to this task. All activity will be public on this ticket. For now, sit tight and we'll update this ticket once we have more information on the next steps.

See our documentation for more information.

rholshausen commented 1 year ago

I've added a function pactffi_validate_datetime that can validate a string value against a format string.

adamrodger commented 1 year ago

Hmm, I'm not really sure if that helps in the .Net case because there's still no proper way to format a .Net DateTime using the different format string

rholshausen commented 1 year ago

Not sure what you are suggesting here. That format was chosen because it is an idiomatic way to represent dates and times in a non-language specific manor. An original suggestion was to just use the Ruby format strings, which use the C format, which means instead of yyyy-MM-dd you would have had to use %Y-%m-%d.

Or are you suggesting the core needs to understand the date/time formats of every language so that a Pact generated with .Net formatted dates can work with a Node, Python, Java or any other backend? How is this meant to work?

adamrodger commented 1 year ago

No I think the problem is that by picking one specific format it's made it really difficult to use these matcher types in any language that uses a different format (which is pretty much all of them I think?)

Either each language needs to translate to and from the Java format, or we need to pass the problem onto the user by creating a stringly-typed API and making the user specify the Java format in non-Java languages.

The former is very difficult for library maintainers (and potentially even impossible if the Java format supports something that the other language doesn't.

The latter makes for a poor experience for the user. For example, in .Net the user would expect to use System.DateTime objects (or similar) and the standard .Net format strings documented on the Microsoft website.

Instead they have to pass a string for the date/time and supply a Java-style format string, which doesn't really make sense from a user's perspective.

If that was the case it's actually easier to just use a regex matcher. You've already got to supply your own formatted string and at least regex patterns are more common across many languages with plenty of websites to create and check them. I think the date/time matchers are actually more confusing than that.

rholshausen commented 1 year ago

easier to just use a regex matcher

Seriously, have you seen what a regex for a date time looks like? As the saying goes, you have a problem and you add a regex to solve it, and now you have two problems. And then how would you support time zones, leap years and seconds? Have you seen the Gregorian calendar for September 1752?

❯ cal 9 1752
   September 1752     
Su Mo Tu We Th Fr Sa  
       1  2 14 15 16  
17 18 19 20 21 22 23  
24 25 26 27 28 29 30  
adamrodger commented 1 year ago

I'd question if this is really closed as completed...?