unoplatform / uno.extensions

Libraries to ease common developer tasks associated with building multi-platform mobile, desktop and web applications using Uno Platform or WinAppSDK.
https://platform.uno/
Other
72 stars 46 forks source link

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

Open xperiandri opened 2 months ago

xperiandri commented 2 months ago

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):

Visual Studio:

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 commented 2 months ago

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

xperiandri commented 2 months ago

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 commented 1 week ago

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

jeromelaban commented 1 week ago

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