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

Make Application.Properties obsolete and fix implementation #2158

Merged
merged 10 commits into from
Aug 25, 2021

Conversation

Redth
Copy link
Member

@Redth Redth commented Aug 18, 2021

First of all, mark Application.Properties and Application.SavePropertiesAsync as obsolete and suggest Essentials.Preferences instead.

Next, the current implementation is broken/throwing on some platforms, and on at least MacCatalyst saving to a file is causing a native permission dialog to popup when the app exits the first time (since the properties get saved on exit).

So, switch to using more normal System.IO API's to do the work:

  • Windows: read/write to Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData)
  • Android: read/write to Essentials.FileSystem.CacheDirectory
  • iOS/MacCatalyst: use NSUserDerfaults to read/write the serialized data

Note: I avoided using Essentials.Preferences as the implementation for the existing api for a couple of reasons:

  1. The API has no concept of getting ALL the dictionary values, from the native platform, and unfortunately on iOS, for reasons, it's particularly hard to do that in a way that the resulting data type for a stored 'object' is retrieved in a predictable way.
  2. The behavior could indeed be different from how xml serialization is used currently if it got moved to some other mechanism.
  3. We could have just serialized and used Preferences.Set(..) on every platform, except.... Android has issues with larger data being stored with SharedPreferences (which is what Preferences API uses), so serializing many MB of data could cause it to break on some android devices.

On Windows, use LocalApplication folder with a file, on iOS/MacCatalyst store the serialized blob into NSUserDefaults, on Android use the cache directory from essentials to store the file.
@Redth Redth requested a review from mattleibow August 18, 2021 23:02
@Redth Redth added this to the 6.0.100-rc.1 milestone Aug 18, 2021
@Redth
Copy link
Member Author

Redth commented Aug 18, 2021

Fixes #1920

@hartez
Copy link
Contributor

hartez commented Aug 19, 2021

This sort of resolves #22. Though the original spec calls for simply removing the feature altogether (and telling folks to use Essentials).

@@ -94,6 +94,7 @@ public Page MainPage
}
}

[Obsolete("Properties API is obsolete, use Essentials.Preferences instead.")]
public IDictionary<string, object> Properties
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EditorBrowsable.Never?

@mattleibow
Copy link
Member

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

Copy link
Member

@rmarinho rmarinho left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to fix usage on COntrolGallery

"D:\agent\1\s\src\Compatibility\ControlGallery\test\WinUI.UITests\WinUI.UITests.csproj" (default target) (15:6) ->
  D:\agent\1\s\src\Compatibility\ControlGallery\src\Issues.Shared\Issue2104.cs(47,6): error CS0618: 'Application.Properties' is obsolete: 'Properties API is obsolete, use Essentials.Preferences instead.' [D:\agent\1\s\src\Compatibility\ControlGallery\test\WinUI.UITests\WinUI.UITests.csproj]
  D:\agent\1\s\src\Compatibility\ControlGallery\src\Issues.Shared\Issue2104.cs(52,7): error CS0618: 'Application.Properties' is obsolete: 'Properties API is obsolete, use Essentials.Preferences instead.' [D:\agent\1\s\src\Compatibility\ControlGallery\test\WinUI.UITests\WinUI.UITests.csproj]
  D:\agent\1\s\src\Compatibility\ControlGallery\src\Issues.Shared\Issue2104.cs(58,7): error CS0618: 'Application.Properties' is obsolete: 'Properties API is obsolete, use Essentials.Preferences instead.' [D:\agent\1\s\src\Compatibility\ControlGallery\test\WinUI.UITests\WinUI.UITests.csproj]
  D:\agent\1\s\src\Compatibility\ControlGallery\src\Issues.Shared\Issue2104.cs(75,6): error CS0618: 'Application.Properties' is obsolete: 'Properties API is obsolete, use Essentials.Preferences instead.' [D:\agent\1\s\src\Compatibility\ControlGallery\test\WinUI.UITests\WinUI.UITests.csproj]
  D:\agent\1\s\src\Compatibility\ControlGallery\src\Issues.Shared\Issue3541.cs(65,4): error CS0618: 'Application.Properties' is obsolete: 'Properties API is obsolete, use Essentials.Preferences instead.' [D:\agent\1\s\src\Compatibility\ControlGallery\test\WinUI.UITests\WinUI.UITests.csproj]
  D:\agent\1\s\src\Compatibility\ControlGallery\src\Issues.Shared\Issue3541.cs(66,10): error CS0618: 'Application.SavePropertiesAsync()' is obsolete: 'Properties API is obsolete, use Essentials.Preferences instead.' [D:\agent\1\s\src\Compatibility\ControlGallery\test\WinUI.UITests\WinUI.UITests.csproj]
  D:\agent\1\s\src\Compatibility\ControlGallery\src\Issues.Shared\Issue3541.cs(80,8): error CS0618: 'Application.Properties' is obsolete: 'Properties API is obsolete, use Essentials.Preferences instead.' [D:\agent\1\s\src\Compatibility\ControlGallery\test\WinUI.UITests\WinUI.UITests.csproj]
  D:\agent\1\s\src\Compatibility\ControlGallery\src\Issues.Shared\Issue3541.cs(81,12): error CS0618: 'Application.Properties' is obsolete: 'Properties API is obsolete, use Essentials.Preferences instead.' [D:\agent\1\s\src\Compatibility\ControlGallery\test\WinUI.UITests\WinUI.UITests.csproj]
  D:\agent\1\s\src\Compatibility\ControlGallery\src\Issues.Shared\Bugzilla28709.cs(51,21): error CS0618: 'Application.Properties' is obsolete: 'Properties API is obsolete, use Essentials.Preferences instead.' [D:\agent\1\s\src\Compatibility\ControlGallery\test\WinUI.UITests\WinUI.UITests.csproj]
  D:\agent\1\s\src\Compatibility\ControlGallery\src\Issues.Shared\Bugzilla28709.cs(60,33): error CS0618: 'Application.Properties' is obsolete: 'Properties API is obsolete, use Essentials.Preferences instead.' [D:\agent\1\s\src\Compatibility\ControlGallery\test\WinUI.UITests\WinUI.UITests.csproj]
  D:\agent\1\s\src\Compatibility\ControlGallery\src\Issues.Shared\Bugzilla28709.cs(64,27): error CS0618: 'Application.Properties' is obsolete: 'Properties API is obsolete, use Essentials.Preferences instead.' [D:\agent\1\s\src\Compatibility\ControlGallery\test\WinUI.UITests\WinUI.UITests.csproj]
  D:\agent\1\s\src\Compatibility\ControlGallery\src\Issues.Shared\Bugzilla28709.cs(68,32): error CS0618: 'Application.Properties' is obsolete: 'Properties API is obsolete, use Essentials.Preferences instead.' [D:\agent\1\s\src\Compatibility\ControlGallery\test\WinUI.UITests\WinUI.UITests.csproj]
  D:\agent\1\s\src\Compatibility\ControlGallery\src\Issues.Shared\Bugzilla28709.cs(70,5): error CS0618: 'Application.Properties' is obsolete: 'Properties API is obsolete, use Essentials.Preferences instead.' [D:\agent\1\s\src\Compatibility\ControlGallery\test\WinUI.UITests\WinUI.UITests.csproj]
  D:\agent\1\s\src\Compatibility\ControlGallery\src\Issues.Shared\Bugzilla28709.cs(71,5): error CS0618: 'Application.Properties' is obsolete: 'Properties API is obsolete, use Essentials.Preferences instead.' [D:\agent\1\s\src\Compatibility\ControlGallery\test\WinUI.UITests\WinUI.UITests.csproj]
  D:\agent\1\s\src\Compatibility\ControlGallery\src\Issues.Shared\Bugzilla28709.cs(72,5): error CS0618: 'Application.Properties' is obsolete: 'Properties API is obsolete, use Essentials.Preferences instead.' [D:\agent\1\s\src\Compatibility\ControlGallery\test\WinUI.UITests\WinUI.UITests.csproj]
  D:\agent\1\s\src\Compatibility\ControlGallery\src\Issues.Shared\Bugzilla28709.cs(82,10): error CS0618: 'Application.SavePropertiesAsync()' is obsolete: 'Properties API is obsolete, use Essentials.Preferences instead.' [D:\agent\1\s\src\Compatibility\ControlGallery\test\WinUI.UITests\WinUI.UITests.csproj]
  D:\agent\1\s\src\Compatibility\ControlGallery\src\Issues.Shared\Bugzilla28709.cs(88,10): error CS0618: 'Application.SavePropertiesAsync()' is obsolete: 'Properties API is obsolete, use Essentials.Preferences instead.' [D:\agent\1\s\src\Compatibility\ControlGallery\test\WinUI.UITests\WinUI.UITests.csproj]

@@ -11,37 +12,35 @@ namespace Microsoft.Maui.Controls.Compatibility.Platform.Android
{
internal class Deserializer : IDeserializer
{
const string PropertyStoreFile = "PropertyStore.forms";
const string PropertyStoreFile = "PropertyStore.maui";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since no compatibility with Xamarin.Forms is expected, would it be better to use System.Json for serialization and deserialization? It is at least more space efficient if not faster too.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that is because it is a separate dependency we have to bring in. Also, this feature is very obsolete and bringing in a dependency for a feature we basically want to delete is not worth it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was under the impression that System.Json is part of net6.0 and hence net6.0-android. Never mind then 😅. I agree, it's not worth it.

@mattleibow mattleibow merged commit 21049ab into main Aug 25, 2021
@mattleibow mattleibow deleted the application-properties-obsolete branch August 25, 2021 00:36
@github-actions github-actions bot locked and limited conversation to collaborators Dec 22, 2023
@samhouts samhouts added the fixed-in-6.0.100-rc.1.7 Look for this fix in 6.0.100-rc.1.7! label Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants