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

IKeyValueStorage.GetAsync does not support value types and has a wrong XML comment #2420

Open
xperiandri opened this issue Jul 11, 2024 · 4 comments · May be fixed by #2559
Open

IKeyValueStorage.GetAsync does not support value types and has a wrong XML comment #2420

xperiandri opened this issue Jul 11, 2024 · 4 comments · May be fixed by #2559
Labels
kind/bug Something isn't working triage/untriaged Indicates an issue requires triaging or verification.

Comments

@xperiandri
Copy link

xperiandri commented Jul 11, 2024

You state

	/// <summary>
	/// Gets a value saved under that name. If that value does not exist, throws a <seealso cref="KeyNotFoundException"/>.
	/// </summary>
	ValueTask<TValue?> GetAsync<TValue>(string key, CancellationToken ct);

Current behavior

It does not throw any exceptions but returns a default value

Expected behavior

Either documentation is adjusted or the code returns what is declared

How to reproduce it (as minimally and precisely as possible)

  1. Write a DateTime value
  2. Read a DateTime value

Environment

Nuget Package (s): Uno.Extensions.Storage

Package Version(s): 4.1.23, 4.3.0-dev.5

Affected platform(s):

  • iOS
  • macOS (AppKit)
  • Mac Catalyst
  • Android
  • WebAssembly
  • Windows
  • Skia (WPF)
  • Skia (GTK on Linux/macOS/Windows)
  • Skia (Linux Framebuffer)

Visual Studio:

  • 2022 (version: 17.10.4)

Anything else we need to know?

This piece of code is wrong

public async ValueTask<TValue?> GetAsync<TValue>(string key, CancellationToken ct)
{
	using (await _lock.LockAsync(ct))
	{
		if (!settings.DisableInMemoryCache)
		{
			var val = await InMemoryStorage.GetAsync<TValue>(key, ct);
			if (val is not null)
			{
				return val;
			}
		}
		var internalVal = await InternalGetAsync<TValue>(key, ct);
		if (!settings.DisableInMemoryCache &&
			internalVal is not null)
		{
#pragma warning disable CS8714 // Incorrect Nullability warning
			await InMemoryStorage.SetAsync(key, internalVal, ct);
#pragma warning restore CS8714 // The type cannot be used as type parameter in the generic type or method. Nullability of type argument doesn't match 'notnull' constraint.
		}
		return internalVal;
	}
}

instead of is not null it must be != default

@xperiandri xperiandri added kind/bug Something isn't working triage/untriaged Indicates an issue requires triaging or verification. labels Jul 11, 2024
@xperiandri
Copy link
Author

@nickrandolph do you want a PR with a fix?
cc @jeromelaban

@xperiandri
Copy link
Author

Although DateTime is supported by AppData I get null if I move the pointer and go to InternalGetAsync
https://learn.microsoft.com/en-us/windows/apps/design/app-settings/store-and-retrieve-app-data#settings

@xperiandri
Copy link
Author

@sasakrsmanovic can this be addressed?
Is it a 1 line change?

@jeromelaban
Copy link
Member

@xperiandri if you do know what needs changing could you make a PR? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working triage/untriaged Indicates an issue requires triaging or verification.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants