mc-image-helper icon indicating copy to clipboard operation
mc-image-helper copied to clipboard

feat(get): file name is computed from the v2/resources/(resource_id) URL value

Open atyrode opened this issue 2 years ago • 4 comments

Feature request

Hello!

I suggest that the downloaded file name for a given resource is not ID.jar but instead computed from the ['file'] -> ['url'] key of the SPIGET API at the v2/resources/ + ID endpoint.

By extracting the filename between resource/ and /download in the value of the URL key, and sanitizing it (replace every non [a-zA-Z0-9] character by _), this would transform ID.jar as 'example_name_ID.jar'.

For resource ID=81 81.jar becomes libs_disguise_free_81.jar


This change would have two benefits:

  1. Being able to identify what the file is representing right away in the filesystem
  2. The file remains maintainable programatically because the resource ID will always be the last part of the file name

I tried to see if I could do this msyelf but Java is not my expertise at all. However, if you can pinpoint where this change needs to be made, I'll gladly do a PR once I work this out; if you agree with the idea.

Thanks! Keep up the great work.

atyrode avatar Oct 08 '23 01:10 atyrode

Actually, spiget support isn't yet converted over to mc-image-helper

https://github.com/itzg/docker-minecraft-server/blob/master/scripts/start-spiget

That's why the file naming algorithm is overly simplistic so far.

When it is converted over the proper file naming can be used like the other retrieval logic here.

itzg avatar Oct 08 '23 02:10 itzg

I see, my bad for the overlook. I was watching the debug log and assumed the call to mc get meant it was doing the parsing, now I think I understand that it's being used to get the json path and start-spiget does the rest.

Would it make sense to submit a PR that does the suggested enhancement on the dockerfile side or would this be futile since a proper implementation is planned here?

atyrode avatar Oct 08 '23 02:10 atyrode

Good question. Yeah, I'd prefer to put the effort into porting it fully to mc-image-helper.

I'll try to take a quick go at that today.

itzg avatar Oct 08 '23 13:10 itzg

...ah, I remember now: I don't like Spiget's API. It is annoying that

https://spiget.org/documentation/#!/resources/get_resources_resource_download

redirects and downloads from Spiget's CDN whereas

https://spiget.org/documentation/#!/resources/get_resources_resource_versions_version_download

acts completely different and just redirects to spigotmc, which in turn responds with a 403 forbidden since they don't want automated downloads supported.

I would prefer a cleaner/robust API usage of

  1. check lastest resource metadata
  2. compare version in metadata with the one we already downloaded
  3. if not the same, download that specific version

Instead it's simply a blind download from the latest of that resource ID. To make matter worse:

  1. the directed URL does not contain the filename identifier, such as https://cdn.spiget.org/file/spiget-resources/28103.jar
  2. the response headers of the CDN redirect do not include a Content-Disposition which is where CDNs normally convey the resolved filename

Your suggestion of deriving from the file->url of the resource metadata does seem to be about the only solution, but that feels like it might be brittle.

itzg avatar Oct 08 '23 17:10 itzg