Fix for xs:date with tz format #213
Conversation
|
Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test." |
| if(dateText.endsWith('Z') || dateText.length === 16){ | ||
| dateText = text.substr(0, 10); | ||
| } | ||
| value = new Date(dateText); |
There was a problem hiding this comment.
In my test:
> new Date('2019-03-26Z')
2019-03-26T00:00:00.000Z
> new Date('2019-03-26')
2019-03-26T00:00:00.000Z
> new Date('2019-03-26Z06:00')
2019-03-26T06:00:00.000Z
> new Date('2019-03-26-06:00')
Invalid Date
Which date format do you try to support? Is it a valid xsd:date or xsd:dateTime?
There was a problem hiding this comment.
Valid date formats with TZ for xsd:date are
2002-09-24Z
2002-09-24-06:00
2002-09-24+06:00
Taken from: https://www.w3schools.com/xml/schema_dtypes_date.asp
xsd:dateTime has no problem because it includes time and parses as is (ISO string).
There was a problem hiding this comment.
I don't think we can just drop -06:00 or +06:00 to support the following xml dates:
- 2002-09-24-06:00
- 2002-09-24+06:00
Please also add some comments and tests to cover the cases.
There was a problem hiding this comment.
The thing is that the Date object cannot store an arbitrary timezone, it just takes the time as is and puts the local machine timezone. For representation purposes, the TZ cannot be stored unless we create a "custom" Date object, but I think that could break functionality.
My best suggestion is to drop the TZ information, because in this case there is no time to calculate an offset.
I need some directions to where write tests to this, I tried with a dummy request-response-sample but it doesn't execute my code.
I finally written a test just to execute the parseValue function under this condition, see the updated PR.
There was a problem hiding this comment.
As far as I understand, Date objects use a Unix Time Stamp, an integer value that is the number of milliseconds since 1 January 1970 UTC. So it's agonistic of time zone.
There was a problem hiding this comment.
Yeah, that's basically it, there's no way to store the TZ info, that's why I set it to discard it when there's only date without time.
Do I have to do anything else for this to be merged or just wait for @b-admike 's review?, We need this to be able to work with a soap service that returns dates with tz info.
|
Added some tests to cover the cases, also added tests for xsd:dateTime. |
I'm a bit confused. IMO, the date should represent the exact same value regardless of the time zone. You can reformat the same date into various time zones. In your PR, the time zone information is discarded and the value is created from a local time zone. The resulting date is different from the one from XML. |
are you are talking about Locale?, I'm talking about time offset, this doesn't have anything to do with the date format or Locale btw, the offset is used just to... well, offset the time with a specific timezone. In this case it doesn't apply because it is only a date without time information. |
The offset will impact the date. |
It shouldn't, the xs:date doesn't have a time information, from what time will be offset?, we can't assume that is 00:00:00 for example and offset it to the day before with -06:00 offset for example, because is only date information, no date+time (not xs:dateTime). |
|
Taking two examples from https://www.w3schools.com/xml/schema_dtypes_date.asp:
If we don't care about the offset, why does XML allow them? What's the use case? |
It's allowed by the w3c XSD spec, although why it is allowed or the use case, I personally don't know, in our case we specifically just need the date and the soap service is not under our control. I was thinking maybe is for representation purposes?, I looked at the XSD specification but didn't find anything meaningful or "enlightening". |
|
I think I understand the offset now - https://en.wikipedia.org/wiki/UTC_offset. Basically, 2002-09-24+06:00 is 2002/09/24 in a TZ that's 6 hours ahead of UTC. So it cannot tell what exactly the date in UTC as the time part is missing. |
Yeah, that's the issue, we cannot leave it as is because it parses to a Invalid Date, we need the date, as long this comes from a xs:date, time is irrelevant and therefore, the offset also is irrelevant. Unless we represent the date only + offset with another object type, but that would be breaking. |
|
@arthmoeros PR landed and released in strong-soap@1.18.0. Thanks! 🚀 |
Description
Hi,
We found that xs:date elements with Timezone are parsed as null, I did this fix for these cases and tested it locally and works.
However, I tried adding a request-response-sample for unit testing but it doesn't run my code, Can you point me in the right direction to add test coverage for this?
Related issues
No issues posted
Checklist
guide