UriImageSource caching not implemented for Windows & check TODO comments around caching in general

[GeorgeLeithead]

Description

Looking at the UriImageSource control, in the GetStreamAsync method it states the following which gives the impression that caching is not implemented. I believe that in Xamarin.Forms caching was done directly in ImageSource but in MAUI I believe it's done in UriImageSourceService for the platforms?

Should this be cleaned up as appropriate?

        async Task<Stream> GetStreamAsync(Uri uri, CancellationToken cancellationToken = default(CancellationToken))
        {
            cancellationToken.ThrowIfCancellationRequested();

            Stream stream = null;

            if (CachingEnabled)
            {
                // TODO: CACHING https://github.com/dotnet/runtime/issues/52332

                // var key = GetKey();
                // var cached = TryGetFromCache(key, out stream)
                if (stream is null)
                    stream = await DownloadStreamAsync(uri, cancellationToken).ConfigureAwait(false);
                // if (!cached)
                //    Cache(key, stream)
            }
            else
            {
                stream = await DownloadStreamAsync(uri, cancellationToken).ConfigureAwait(false);
            }

            return stream;
        }

Steps to Reproduce

View the source of UriImageSource.cs

Version with bug

6.0.408 (current)

Last version that worked well

Unknown/Other

Affected platforms

I was not able test on other platforms

Affected platform versions

NA

Did you find any workaround?

No response

Relevant log output

No response

[jfversluis]

Comment in the code is out of date, we just need to delete it still, thanks! :)

[GeorgeLeithead]

Gong in deeper to the service handlers:

• *ImageSourceService handlers for windows and Tizen don't appear to implement any caching.
• *ImageSourceService handlers for windows, and iOS all have a TODO stating, "use a real caching library with the URI".

[jfversluis]

If you think things aren't working I'd love to see reproductions!

[jfversluis]

Just from looking at the code it seems like we indeed still need to implement caching for Windows (and Tizen although that isn't really our responsibility). I am curious what the "use a real caching library with the URI" comments mean.

[GeorgeLeithead]

Just from looking at the code it seems like we indeed still need to implement caching for Windows (and Tizen although that isn't really our responsibility). I am curious what the "use a real caching library with the URI" comments mean.

@jfversluis There is a lot of code. Things change and some things need review.

Looking even more at the code, it appears like none of the platform implementations meet what I would (IMO) expect from caching. Take for example the iOS implementation, which looks the most complete. I cannot find anywhere where the cache validity timespan is used or even referenced.

The TODO were there possibly (IMO) to indicate that a cross-platform Cache implementation is still required. Like done for XF Xamarin.Forms.Platform.Android/ImageCache.cs

[ghost]

Hi @GeorgeLeithead. We have added the "s/needs-repro" label to this issue, which indicates that we require steps and sample code to reproduce the issue before we can take further action. Please try to create a minimal sample project/solution or code samples which reproduce the issue, ideally as a GitHub repo that we can clone. See more details about creating repros here: https://github.com/dotnet/maui/blob/main/.github/repro.md

This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

[ghost]

This issue has been automatically marked as stale because it has been marked as requiring author feedback to reproduce the issue but has not had any activity for 4 days. It will be closed if no further activity occurs within 3 days of this comment. If it is closed, feel free to comment when you are able to provide the additional information and we will re-investigate.

[GeorgeLeithead]

I don't think there is a need for a 'repo', as the issue is in THIS repo!

I have looked at the code and it does look like UriImageSource caching is now available for:

• Android (using PlatformInterop.LoadImageFromUri)
• iOS (still has a // TODO: use a real caching library with the URI)

However, it doesn't look like anything is available for the following, and there is no use at all of the CachingEnabled property on these platforms:

• Tizen
• Windows

See: src/Core/src/ImageSources/UriImageSourceService

[ninachen03]

See the code in https://github.com/dotnet/maui/blob/fecc58134be698c9c8349207a9b1c76ff8ef7554/src/Core/src/ImageSources/UriImageSourceService/UriImageSourceService.Windows.cs#L22 https://github.com/dotnet/maui/blob/fecc58134be698c9c8349207a9b1c76ff8ef7554/src/Core/src/ImageSources/UriImageSourceService/UriImageSourceService.iOS.cs#L48