Hardcode known wrapper checksums to avoid network requests

[Marcono1234]

Fixes #161

If a checksum is not found in the hardcoded list, the action falls back to fetching the checksums from the Gradle API, as before.

For now there is no logic for automatically updating the list of known checksums; that has to be done manually. But maybe it could be automated in some way in the future.

Here is my somewhat hacky (but hopefully bug-free) Java code which I used to generate the list of checksums:

Checksums list creator

import java.io.IOException;
import java.lang.reflect.Type;
import java.time.Instant;
import java.time.format.DateTimeFormatter;
import java.util.Comparator;
import java.util.HashSet;
import java.util.List;
import java.util.Locale;
import java.util.Set;

import com.google.gson.Gson;
import com.google.gson.JsonDeserializationContext;
import com.google.gson.JsonDeserializer;
import com.google.gson.JsonElement;
import com.google.gson.JsonParseException;
import com.google.gson.annotations.JsonAdapter;
import com.google.gson.reflect.TypeToken;

import okhttp3.OkHttpClient;
import okhttp3.Request;
import okhttp3.Response;

public class GradleChecksumsFetcher {
    private static final OkHttpClient client = new OkHttpClient();

    private static String fetch(String url) throws IOException {
      Request request = new Request.Builder()
          .url(url)
          .build();

      try (Response response = client.newCall(request).execute()) {
        return response.body().string();
      }
    }

    static class VersionData {
        String version;
        String wrapperChecksumUrl;
        @JsonAdapter(BuildTimeAdapter.class)
        Instant buildTime;

        boolean snapshot;
        boolean nightly;
        boolean releaseNightly;

        private static class BuildTimeAdapter implements JsonDeserializer<Instant> {
            private static final DateTimeFormatter FORMATTER = DateTimeFormatter.ofPattern("yyyyMMddHHmmssxx", Locale.ROOT);

            @Override
            public Instant deserialize(JsonElement json, Type typeOfT, JsonDeserializationContext context)
                    throws JsonParseException {
                return FORMATTER.parse(json.getAsString(), Instant::from);
            }
        }
    }

    private static List<String> parseVersion(String v) {
        int i = v.indexOf('-');
        if (i != -1) {
            v = v.substring(0, i);
        }

        return List.of(v.split("\\."));
    }

    private static int compareVersions(String a, String b) {
        List<String> aParts = parseVersion(a);
        List<String> bParts = parseVersion(b);

        for (int i = 0; i < Math.min(aParts.size(), bParts.size()); i++) {
            int aPart = Integer.parseInt(aParts.get(i));
            int bPart = Integer.parseInt(bParts.get(i));

            if (aPart < bPart) {
                return -1;
            }
            if (aPart > bPart) {
                return 1;
            }
        }
        return Integer.compare(aParts.size(), bParts.size());
    }

    public static void main(String[] args) throws Exception {
        Gson gson = new Gson();
        List<VersionData> versions = gson.fromJson(fetch("https://services.gradle.org/versions/all"), new TypeToken<>() {});
        // First sort by version number, then by build time
        // That should (in most cases) achieve version ranges where all hashes only belong to one range
        Comparator<VersionData> comparator = (v1, v2) -> compareVersions(v1.version, v2.version);
        versions.sort(comparator.thenComparing(Comparator.comparing(v -> v.buildTime)));

        String firstVersionName = null;
        String lastVersionName = null;
        String lastHash = null;

        Set<String> seenHashes = new HashSet<>();

        for (VersionData version : versions) {
            if (version.wrapperChecksumUrl == null || version.snapshot || version.nightly || version.releaseNightly) {
                continue;
            }
            String versionName = version.version;
            String hash = fetch(version.wrapperChecksumUrl);

            if (lastHash == null) {
                firstVersionName = versionName;
                lastVersionName = versionName;
                lastHash = hash;
            } else {
                if (hash.equals(lastHash)) {
                    lastVersionName = versionName;
                } else {
                    if (firstVersionName.equals(lastVersionName)) {
                        System.out.println("// " + firstVersionName);
                    } else {
                        System.out.println("// " + firstVersionName + " - " + lastVersionName);
                    }
                    System.out.println("\"" + lastHash + "\",");

                    firstVersionName = versionName;
                    lastVersionName = versionName;
                    lastHash = hash;

                    // This acts mainly as assertion that the version sorting logic is correct
                    // There seems to be one case though where the version range `8.0-milestone-1 - 8.0-milestone-3`
                    // is using the same hash as a previous version range
                    if (!seenHashes.add(hash)) {
                        System.out.println("// WARNING: Duplicate hash: " + hash + "; for version " + versionName);
                    }
                }
            }
        }

        System.out.println("// " + firstVersionName + " - " + lastVersionName);
        System.out.println("\"" + lastHash + "\",");
    }

}

⚠️ I am not that familiar with TypeScript, so any feedback is appreciated!

[JLLeitschuh]

In general, this looks like it's headed in the right direction!

I think the biggest ask is tooling/automation to keep the list up-to-date more easily, and ideally, with a GitHub action

[JLLeitschuh]

Is there a way to pull this from a resource file instead of hard-coding it into the source code.

Ideally, there would be a resource file, and there would also be a script that CI could automatically run to update this file and would auto-push then auto-release when there was an update

[bigdaz]

Yep, that would be much better. If the file was formatted as JSON, it could be trivially loaded into an object: that way we could include the Gradle version as well as the checksum in the metadata.

@Marcono1234 you can see here an example of loading a resource file in Typescript. It may not be the best way, but it works!
Then, you can use JSON.parse(fileContents) and use it as a typed Javascript object. Here are some examples.

[bigdaz]

This looks pretty good to me. If we can extract the checksums list into a resource file, then I'm OK if this needs to be maintained by hand (for now). New Gradle versions will continue to require a network call until the wrapper action is updated.

As @JLLeitschuh mentioned, it would be ideal if we had a job that ran in this repository to update the versions file when a new Gradle version is released. But we'll still need to release a new action version each time.

An additional optimization would be to store any "discovered" checksums in the GitHub Actions cache. We read the set of known versions from the resource, add any discovered versions, and write the JSON file directly to the cache.

[bigdaz]

Yep, that would be much better. If the file was formatted as JSON, it could be trivially loaded into an object: that way we could include the Gradle version as well as the checksum in the metadata.

@Marcono1234 you can see here an example of loading a resource file in Typescript. It may not be the best way, but it works!
Then, you can use JSON.parse(fileContents) and use it as a typed Javascript object. Here are some examples.

[bigdaz]

@Marcono1234 The changes to dist/index.js make the PR very difficult to merge. Can you please update the commit to remove that change?

[Marcono1234]

Thanks for the feedback! I will try to adjust this in the next days.

Would it make sense though to use a plaintext / custom text format where for example each line represents a checksum, except if it is blank or starts with #? For example something like:

# 1.0
87e50531ca7aab675f5bb65755ef78328afd64cf0877e37ad876047a8a014055
# 1.1
22c56a9780daeee00e5bf31621f991b68e73eff6fe8afca628a1fe2c50c6038e
# 1.2
5c91fa893665f3051eae14578fac2df14e737423387e75ffbeccd35f335a3d8b
...

JSON has the disadvantage that it doesn't support comments, and just having a large list of checksums without any indication to which versions they belong might make reviewing it difficult.

However, with #145 maybe it would make sense to use JSON instead of a custom text format, but then include the version numbers. For example:

{
  "87e50531ca7aab675f5bb65755ef78328afd64cf0877e37ad876047a8a014055": [
    "1.0"
  ],
  "22c56a9780daeee00e5bf31621f991b68e73eff6fe8afca628a1fe2c50c6038e": [
    "1.1",
  ]
}

[bigdaz]

However, with #145 maybe it would make sense to use JSON instead of a custom text format, but then include the version numbers.

Yes that's what I meant. Something like:

[
 { "version": "1.0", "checksum": "....." },
 { "version": "1.1", "checksum": "....." },
  ... etc ..
]

That way you could use JSON.parse to read this directly into an array of Typescript object with a version and checksum attribute. Take a look at the links I posted for examples.

Once you have this array, you can get the list of checksums on their own using arrayOfObjects.map { it.checksum }.

[Marcono1234]

I have performed the following changes now:

• Included checksums as JSON file
  The JSON file is imported as module instead of using path.resolve(__dirname, ...). This seems more reliable to me because depending on whether unit tests or the action (from index.js) is run __dirname differs (I assume), and therefore both src/... and dist/... would need to have the same nesting level for this to work.
  I am not sure though if importing the JSON as module could become a performance problem (possibly during development only).
• Changed known KNOWN_VALID_CHECKSUMS to be a Map<string, Set<string>>, mapping from checksum to set of version names. This is for compatibility with #145; could then for example calculate the checksum for the JAR, and check whether the expected version is in the set for that checksum.
• Added GitHub workflow for updating checksums file and creating a pull request
  This runs every week but can also be run manually. Here is an example how the created pull request looks like: https://github.com/Marcono1234/wrapper-validation-action/pull/6.

I hope that is ok like this. Feedback is appreciated!

[Marcono1234]

Maybe the branch name should use a prefix such as bot/ to prevent accidentally manually editing this branch, which might then be overwritten by this workflow.

What do you think?

[JLLeitschuh]

Sounds completely reasonable to prefix it with bot/

[bigdaz]

Thanks @Marcono1234 . I'm not going to have capacity to take this further until Feb 13 at the earliest. But it's on my list!

[JLLeitschuh]

Awesome. Was thinking about writing this myself and wasn't looking forward to it. Solid bit of work here!

Javascript is definitely not my strongest area, ngl. 😆

[JLLeitschuh]

This was what I was missing when I took an early crack at this! 🎉

[JLLeitschuh]

Can you add a unit test that verifies that this is never empty, and always contains some reasonable versions as a spot-check that this logic never breaks?

[bigdaz]

I've never really worked on this code, and I don't want to delay the merge. So I'm going to approve (and merge) based on @JLLeitschuh approval.
I'd like to include this in the first RC of v2.0.0, since that mitigates some of the risk of the change. (People have to explicitly update to get the new version).

[bigdaz]

This has been merged manually. I need to find a better process for merging external PRs.

[bigdaz]

'd like to include this in the first RC of v2.0.0, since that mitigates some of the risk of the change. (People have to explicitly update to get the new version).

Actually, I forgot that I already released v2.0.0-rc.1 🤦🏼 .
This change will need to wait for a v2.1.0 release.

[JLLeitschuh]

@Marcono1234 seriously, thank you so much for working this problem out. Really truly. You can see by the number of issues that @bigdaz closed how much pain our network connection logic has caused over the years.

You've just significantly increased the stability and usability of this action for all of the users of it.

Truly, thank you so much for your contribution. I'm incredibly appreciative you took the time to contribute it.

[Marcono1234]

Thanks for the kind words and the feedback!

I have addressed the feedback (adding tests, and bot/ branch prefix) in #178.

[bigdaz]

I've just released v2.1.0 containing this change. Thanks again!