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

[Windows] Make sure that FileSystem.Current.AppDataDirectory folder exists in unpackaged #23265

Merged

Conversation

MartyIX
Copy link
Contributor

@MartyIX MartyIX commented Jun 26, 2024

Description of Change

I'm not too sure if the PR is correct or not as I haven't figured out how to run Windows tests locally.

Anyway, #22231 mentions C:\Users\davidortinau\AppData\Roaming\User Name\com.simplyprofound.sentencestudio\Data\ and that Data folder comes from this place.

Issues Fixed

Fixes #22231

@MartyIX MartyIX requested a review from a team as a code owner June 26, 2024 07:44
Copy link
Contributor

Hey there @MartyIX! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Jun 26, 2024
@@ -8,7 +8,7 @@ namespace Microsoft.Maui.Essentials.DeviceTests
[Category("FileSystem")]
public class FileSystem_Tests
{
const string bundleFileContents = "This file was in the app bundle.";
const string BundleFileContents = "This file was in the app bundle.";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just code style changes in this file.


if (!File.Exists(_platformAppDataDirectory))
{
Directory.CreateDirectory(_platformAppDataDirectory);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not great that this method can throw an exception in the constructor.

=> AppInfoUtils.IsPackagedApp
? ApplicationData.Current.LocalFolder.Path
: Path.Combine(Environment.GetFolderPath(Environment.SpecialFolder.ApplicationData), AppSpecificPath, "Data");
string PlatformAppDataDirectory => _platformAppDataDirectory;
Copy link
Member

Choose a reason for hiding this comment

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

Maybe instead of moving the logic the constructor, it might be better to do all the checks and creating in this property? So this property will throw vs the ctor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But then you would have some perf penalty on every access. Would that be fine or not?

Ah you probably meant using Lazy. That might be a viable way indeed.

@MartyIX
Copy link
Contributor Author

MartyIX commented Jun 26, 2024

Microsoft.Maui.Essentials.Sample.exe reports in the File System section:

image

and the folders are there.

@MartyIX
Copy link
Contributor Author

MartyIX commented Jun 27, 2024

@mattleibow Does this look good to you?

@mattleibow
Copy link
Member

/azp run

@mattleibow
Copy link
Member

Very nice, thanks.

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen PureWeen added the area-essentials Essentials: Device, Display, Connectivity, Secure Storage, Sensors, App Info label Jun 28, 2024
@mattleibow mattleibow merged commit fa46d10 into dotnet:main Jul 1, 2024
42 of 47 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Jul 31, 2024
@samhouts samhouts added fixed-in-8.0.70 fixed-in-net9.0-nightly This may be available in a nightly release! labels Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-essentials Essentials: Device, Display, Connectivity, Secure Storage, Sensors, App Info community ✨ Community Contribution fixed-in-8.0.70 fixed-in-net9.0-nightly This may be available in a nightly release!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FileSystem.Current.AppDataDirectory returns an invalid path when built for Windows as unpackaged
4 participants