Skip to content
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

Preferences DateTime Storage Issues #25930

Open
david-maw opened this issue Nov 18, 2024 · 11 comments · May be fixed by #26033
Open

Preferences DateTime Storage Issues #25930

david-maw opened this issue Nov 18, 2024 · 11 comments · May be fixed by #26033
Labels
area-essentials Essentials: Device, Display, Connectivity, Secure Storage, Sensors, App Info i/regression This issue described a confirmed regression on a currently supported version platform/windows 🪟 potential-regression This issue described a possible regression on a currently supported version., verification pending s/needs-attention Issue has more information and needs another look s/triaged Issue has been reviewed s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working
Milestone

Comments

@david-maw
Copy link

Description

On .NET 8 a Preferences.Set of a DateTime type would store a string but in .NET 9 it seems to store a long. This has two issues:

  1. When Preferences.Get of a DateTime is presented with a value stored by .NET 8 it throws a format error.
  2. Preferences.Get of a string against a value stored by Preferences.Set of a DateTime on Android faults with: Java.Lang.ClassCastException
    Message=java.lang.Long cannot be cast to java.lang.String

The .NET 9 versions of Preferences.Get and Preferences.Set are compatible with each other as are the .NET 8 versions, it's writing a value with one and reading it with the other that's a problem. The new scheme seems better, but it needs to migrate cleanly.

Steps to Reproduce

Create a default MAU App and insert using CommunityToolkit.Maui; and this code:

        const string name = "Test";
        Preferences.Set(name, new DateTime(2024, 11, 15, 8, 9, 10));
        string s = Preferences.Get(name, "");

Run this on Windows (it will fail on Android) and examine the string, on .NET 9 it will contain "638672549500000000" whereas on .NET 8 it will be "11/15/2024 08:09:10".

Link to public reproduction project repository

No response

Version with bug

9.0.0 GA

Is this a regression from previous behavior?

Yes, this used to work in .NET MAUI

Last version that worked well

8.0.100 SR10

Affected platforms

Android, Windows, I was not able test on other platforms

Affected platform versions

No response

Did you find any workaround?

Explicitly converting a string works on Windows but because long-to-string faults on Android it's trickier there.

Relevant log output

@david-maw david-maw added the t/bug Something isn't working label Nov 18, 2024
@PureWeen PureWeen added area-essentials Essentials: Device, Display, Connectivity, Secure Storage, Sensors, App Info s/needs-repro Attach a solution or code which reproduces the issue labels Nov 18, 2024
@david-maw
Copy link
Author

I thought I had provided steps, do you need something else?

@dotnet-policy-service dotnet-policy-service bot added s/needs-attention Issue has more information and needs another look and removed s/needs-repro Attach a solution or code which reproduces the issue labels Nov 19, 2024
@mattleibow
Copy link
Member

mattleibow commented Nov 19, 2024

I see that we store a long for DateTime and a string for DateTimeOffset. Not sure if you perhaps used that type.

Edit DateTimeOffset is net9, but I see the code in net8 always was a DateTime to long: https://github.com/dotnet/maui/blob/release/8.0.1xx-sr10/src/Essentials/src/Preferences/Preferences.android.cs#L84-L86

Maybe a repro sample will help.

@mattleibow mattleibow added potential-regression This issue described a possible regression on a currently supported version., verification pending s/needs-repro Attach a solution or code which reproduces the issue and removed s/needs-attention Issue has more information and needs another look labels Nov 19, 2024
@david-maw
Copy link
Author

david-maw commented Nov 19, 2024

Well, I don't use that type intentionally @mattleibow , but who knows? You'll note that the repro steps I provided call Preferences.Set with a DateTime and what they read on .NET 8 is a string with no time offset information. In contrast, .NET 9 behaves as you suggest storing, and expecting, a long value, I'll embed the steps in a project and send a reference.

@dotnet-policy-service dotnet-policy-service bot added s/needs-attention Issue has more information and needs another look and removed s/needs-repro Attach a solution or code which reproduces the issue labels Nov 19, 2024
@david-maw
Copy link
Author

Here's a project https://github.com/david-maw/Community-Preferences.git. It's basically my sample code from above wrapped in a UI.

Build and run it on Windows (it defaults to .NET 8) and click the button and I hope you'll see what i did:
Image

Repeat the process under .NET 9 (there's a branch for convenience) and instead of the textual date you'll see 638672549500000000. So .NET 9 appears to behave as you describe, storing a DateTime in a long. I'm not sure what .NET 8 is doing, since the textual date and time do not seem to include a time offset, which is what I'd have expected if it were storing a DateTimeOffset.

@mattleibow
Copy link
Member

mattleibow commented Nov 20, 2024

Thanks for the sample I'll check it out.

@kevinxufei kevinxufei added s/verified Verified / Reproducible Issue ready for Engineering Triage s/triaged Issue has been reviewed labels Nov 20, 2024
@kevinxufei
Copy link

kevinxufei commented Nov 20, 2024

I can repro this issue at Windows platform on the latest 17.13 Preview 1(9.0.0),but this used to work in 8.0.100 & 8.0.93.
Image

@kevinxufei kevinxufei added the i/regression This issue described a confirmed regression on a currently supported version label Nov 20, 2024
@mattleibow
Copy link
Member

Wow, ok. I will check it out. Very strange.

@david-maw
Copy link
Author

It is strange, I couldn't find a Preference.Set call that took a DateTimeOffset parameter so it's probably not that. I added some workaround code just in case anyone else gets bitten by this. With the workaround in place, it's no longer a significant issue for me.

@mattleibow
Copy link
Member

I tested net8 here and I get an exception saying it is a long - invalid cast:

Image

@mattleibow
Copy link
Member

I think I see what is happening here... This has to do with the different implementations of the preferences API on Windows depending on packaged or unpackaged.

  • The .NET 8 template was packaged
  • The .NET 9 template was unpackaged

The implementation differs:

  • packaged uses the ApplicationDataContainer APIs and stores as long and cannot be retrieved as string
  • unpackaged uses a string dictionary that is serialized to a file

The real change comes in at this PR: https://github.com/dotnet/maui/pull/22815/files#diff-6ed591e38dc7754551705bf5bb96f992115c384d19166e8dcda76267e177fb0cR196-R197

In .NET 8 everything was a "ToString" system for unpackaged. In .NET 9 there is now a ToBinary.

I think this may have been too aggressive a change (even though it is correct) and should have had a fallback. I will do a PR for that now.

@david-maw
Copy link
Author

I'm not going to pretend I understand all of that explanation, but I certainly understand the outcome, so thanks.

mattleibow added a commit that referenced this issue Nov 21, 2024
@mattleibow mattleibow linked a pull request Nov 21, 2024 that will close this issue
@PureWeen PureWeen added this to the .NET 9 SR2 milestone Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-essentials Essentials: Device, Display, Connectivity, Secure Storage, Sensors, App Info i/regression This issue described a confirmed regression on a currently supported version platform/windows 🪟 potential-regression This issue described a possible regression on a currently supported version., verification pending s/needs-attention Issue has more information and needs another look s/triaged Issue has been reviewed s/verified Verified / Reproducible Issue ready for Engineering Triage t/bug Something isn't working
Projects
Status: Todo
Development

Successfully merging a pull request may close this issue.

5 participants