-
-
Notifications
You must be signed in to change notification settings - Fork 982
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
joda-time returns the new renamed time zone if DateTimeZone.forID is called with the old time zone id #524
Comments
The Pull Request would also fix #427 |
@jodastephen , when can I expect this to be released? |
This is a tricky one. The code has worked this way for nearly 20 years, and normalizing deprecated IDs to their replacements could well be considered to be good behaviour. If Joda-Time stopped normalizing, many applications would start seeing IDs they didn't see before, and that could break other systems that aren't expecting the older IDs. My problem is that I have to weigh up whether the PR would cause more issues than it would fix. I have to take into account the 20 years of working this way, and the project being in maintenance mode. Given all this, I struggle to come to the conclusion that merging this would be wise. |
I don't think applications will start seeing IDs they don't know about because the applications only get the old ID if the use the old name (which is practically the ID as a string) and if they do so, they are most likely able to handle it. Therefore I don't think any application or user will be surprised by the behaviour of DateTimeZone.forID returning the ID matching the name. I would suggest that that's what most people are expecting. The new IDs are much more likely to break an application in a distributed environment because the old IDs are known to all systems but the new IDs are probably not known. Additionally the java classes java.util.TimeZone and java.time.ZoneId work lik that (meaning not normalizing deprecated IDs) and I don't think that anyone is complaining about those classes. I don't see how Joda-Time was working 20 years "one same predictable way". It's the opposite. After every update of the time zone data, Joda-Time normalizes (changing would be more accurate) a different set of IDs. Consequently assuming there are 3 time zone updates a year, you end up with 60 slighlty incompatible Joda-Time versions after 20 years. Of course this is an exaggeration compared to the actual situation because time zone ID changes are rare and system set ups where differen versions of Joda-Time are also rare. But the fact remains that there are a couple of Joda-Time versions whos DateTimeZone.forID methods are behaving differently. And not because of the logic in the method implementation but because of the data behind it. I totally understand your desire to retain the behaviour of normalizing IDs because it is hard to rule it outthat someone out there relies on it. And in general it is a good practice in library developement to change as little as possible to not surprise anyone. But I think that normalizing old Time Zone IDs to new ones was a bad design choice in the first place and the downsides of it are much greater than the benefits. |
Would it be possible to add a method named |
Yes, I think that would be possible, but you have to use |
With serialization, there's an assumption that a proper time zone object was constructed in the first place. Normalization might have already occurred, and so making readResolve be more strict won't break compatibility. Calling I can't imagine how this breaks compatibility, because even for normalization to work, both the new and old time zone ids must be known. If a time zone is serialized with an id unknown by the other side, this never works, regardless of the proposed changes. If for some reason an old id was removed from the rules without a replacement, serialization of the time zone fails regardless of where or if normalization takes place. |
Well... I had a case where 3 servers had to communicate with serialization/deserialization happening between them. First time between server A and B, sencond time between server B and C. If the server B in the middle hast a newer version of JodaTime that normalizes a deprecated TimeZone-ID than the deserialization breaks on server C. Server B changes the the ID upon deserialization to a new ID that the Server C does not know and than the deserialization on server C is impossible. From the outside it looks like a bug, because Server A sends a TimeZone-ID that Server C should understands. But because Server B silently normalizes it to a newer TimeZone-ID the whole communication breaks. The exception upon deserialization would also occure if Server B sends back a response with the changes TimeZone-ID to Server A who doesn't know it. |
The set of time zones that can be supported among the three servers is defined as the intersection of all sets. The current serialization technique favors a union of the sets, which doesn't make any sense. No argument from me that serialization should avoid normalization and send the time zone in the fashion that's least likely to cause problems. Just don't change the FYI: I wrote the original Joda-time time zone code, although I've not contributed to the project in ages. I just stumbled upon this issue out of curiosity. |
…e ids. DateTimeZone.forIDNoMapping() will return an object with an id exactly as provided
…e ids. DateTimeZone.forExactID() will return an object with an id exactly as provided
I have changed the Pull Request. Now only readResolve in deserialization uses the method forExactID that does not normalize time zones. |
I'm still collecting some data, but the recent There will always be a problem when a client is updated to a new tzdb before the server, but it's localized to the few handful of "quick clients". However, if JodaTime is in the middle of the stack, and normalizes I'm wondering if this recent timezone rename has impacted other companies/users as well? |
We are hit by this Kiev/Kyiv too. I expected this to work
But it fails with java.time.zone.ZoneRulesException: Unknown time-zone ID: Europe/Kyiv Second attempt
This is actually worse as I expected that Joda time's |
Also seeing this in release
...result is
|
Proposed change in #676 This retains auto-conversion of the oldest pre-1993 country/fixed IDs but allows all other IDs of the modern style to be retained. I don't believe it creates any new compatibility issues where different versions of Joda-Time are on different servers. It could break compatibility of anyone who expects zone IDs to be normalized, but I'm now willing to take that risk. Please comment if you have or would like to test this before release. |
Awesome --- I patched this and I'm running this against all tests at Google and will report back any issues. EDIT: I'm immediately seeing a few tests fail with: Oh hmm, it seems we have a locally patched copy of |
@kluever Did you get anywhere with the testing? thanks |
Sadly, no --- I haven't made any significant progress. Unfortunately our internal copy of |
For renamed time zones org.joda.time.DateTimeZone.forID(String id) returns a time zone object that uses the new id even if you use the old id as the method parameter.
This brings me into trouble when I let two servers that have differen joda versions talk to each other. For instance a server with joda-time 2.3 sends time zone Asia/Rangoon to a server with joda time 2.10.5. The latter will change the time zone to Asia/Yangon when .DateTimeZone.forID is called in the deserialization. When the servers sends the time zone back to the former server, joda-time 2.3 does not know of the time zone Asia/Yangon that was introduced in 2017
By the way this change of names is not the case for java.util.TimeZone.getTimeZone(String ID) and java.time.ZoneId.of(String zoneId).
Test testRangoonDateTimeZone fails for 2.10.5 but should pass
The text was updated successfully, but these errors were encountered: