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

Archive caching results in empty ZIP files #29

Open
Archomeda opened this issue Jan 23, 2020 · 4 comments
Open

Archive caching results in empty ZIP files #29

Archomeda opened this issue Jan 23, 2020 · 4 comments
Labels
Area: Web cache Related to the web API caching Priority: Medium Medium priority Type: Bug Something isn't working

Comments

@Archomeda
Copy link
Owner

It seems that for regular API requests the archive caching isn't working.
It has been reported that the resulting ZIP was empty.

While I personally cannot recommend using a file based caching for JSON API requests, some investigation is warranted to see what is going on because the interface does suggest that it's possible.

This probably needs some fixing when the problem is found.

Ref: https://discordapp.com/channels/384735285197537290/589031897963692034/669911617722515466

@Archomeda Archomeda added help wanted Extra attention is needed good first issue Good for newcomers Priority: Medium Medium priority Area: Web cache Related to the web API caching labels Jan 23, 2020
@Archomeda
Copy link
Owner Author

@werdes Do you still have problems with this? Otherwise I'll close it sometime next week.

@Archomeda Archomeda added Status: Discussion needed Extra discussion is needed and removed good first issue Good for newcomers help wanted Extra attention is needed labels Mar 21, 2020
@werdes
Copy link
Contributor

werdes commented Mar 21, 2020

Haven‘t tried so far, but feel free to close. If i come along this again i‘ll let you know and link this issue.

@Archomeda
Copy link
Owner Author

Sounds good! 😄

@Archomeda Archomeda added Resolution: Not reproducible The issue could not be reproduced and removed Priority: Medium Medium priority Status: Discussion needed Extra discussion is needed labels Mar 21, 2020
@Archomeda
Copy link
Owner Author

This issue seems to have popped up again, and it doesn't only apply for regular API requests, but also render requests.

Reproducible steps:

using Gw2Sharp;
using Gw2Sharp.WebApi.Caching;

Console.WriteLine("Hello, World!");

var connection = new Connection
{
    CacheMethod = new ArchiveCacheMethod("cache.zip"),
    RenderCacheMethod = new ArchiveCacheMethod("render-cache.zip")
};

using var client = new Gw2Client(connection);

var items = await client.WebApi.V2.Items.PageAsync(0).ConfigureAwait(false);
var itemUrl = items[0].Icon.Url;
var icon = await client.WebApi.Render.DownloadToByteArrayAsync(itemUrl).ConfigureAwait(false);

Console.WriteLine(items.Count);
Console.WriteLine(icon.Length);

I traced it to an unlucky construction of IDisposable chains.
Every ICacheMethod is IDisposable, which means that you should dispose it after you're done with it. Sadly Gw2Sharp doesn't seem to dispose this object automatically.
The core of the issue is that because of this, the ZipArchive that's used under the hood isn't disposed, meaning that changes are not written to disk.

The current workaround is to keep track of the ArchiveCacheMethod instance outside the Connection object, and dispose of it manually when the Gw2Client and Connection objects are no longer needed.

@Archomeda Archomeda reopened this Oct 9, 2022
@Archomeda Archomeda added Type: Bug Something isn't working Priority: Medium Medium priority and removed Resolution: Not reproducible The issue could not be reproduced labels Oct 9, 2022
@Archomeda Archomeda changed the title Investigate archive caching for regular API requests Archive caching results in empty ZIP files Oct 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Web cache Related to the web API caching Priority: Medium Medium priority Type: Bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants