Refactor database-fetcher.ts into a class and simplify

[robertbrignull]

This PR tries to make the database-fetcher.ts file easier to work with, and also sets up for future refactoring in this area.

I highly recommend ignoring whitespace when reviewing! It has also been split into commits if you find that makes it easier.

The main change is creating the DatabaseFetcher class, rather than having all functions be defined at the top level and exported. My reasoning for doing this is:

• There's a lot of common arguments to methods. They all take some combination of app (or commandManager), databaseManager, storagePath, and cli. These are values that don't change and are needed by every method, so it makes sense to pass them in once when constructing the DatabaseFetcher object.
• It makes mocking easier because you can use mockedObject and pass that in, rather than having to know exactly which methods the code calls and spying on them. It should be a lot less brittle to code changes now, and safer because it'll fail with an error message instead of calling unexpected code.

I then do a little bit of refactoring around the skeleton query wizard because I spotted an opportunity to delete a method and reuse code.

This PR is also hopefully setting up for more work in this area. Right now I haven't changed much about what is done in database-fetcher.ts, but my next plan is to maybe separate it into two areas. The idea would be to separate the fetching of databases from a known source (i.e. URL or github NWO) and prompting for which database to import, which is currently spread between DatabaseFetcher and DatabaseUI and src/databases/github-databases/download.ts.

Checklist

• CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
• Issues have been created for any UI or other user-facing changes made by this pull request.
• [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

[robertbrignull]

I've dismissed this alert as a false positive. There's nothing wrong that I can see with the checkForFailingResponse function or anything else we do with this response.

I'm not sure why it wasn't present on the main branch already as this is just code that's been moved around.

[robertbrignull]

This had some fairly complex merge conflicts and I realised a bunch of the tests were broken. Since nobody has reviewed yet I've tried to rebase and keep all the commits clean. Hopefully I got everything right while rebasing 🤞🏼

[koesie10]

LGTM, but this will have quite some conflicts with #3433.

[robertbrignull]

Thanks. I'll address your comments but then wait for #3433 to be merged so I can fix up the conflicts manually. That's probably easier than fixing them on that PR.

[robertbrignull]

I've merged in latest main. There were conflicts in:

• src/databases/database-fetcher.ts
• src/databases/local-databases-ui.ts
• test/vscode-tests/cli-integration/databases/database-fetcher.test.ts
• test/vscode-tests/global.helper.ts

I fixed the conflicts largely by manually re-applying the changes from https://github.com/github/vscode-codeql/pull/3433/files and https://github.com/github/vscode-codeql/pull/3486/files.

The only change that wasn't clear how to fix was around the ensureZippedSourceLocation function which now gets used from DatabaseManager. I got around this by moving it to a different file instead of keeping it as a method of DatabaseFetcher.

It's quite hard to see everything that changed during the merge so this may need another full review if you want to be certain. To help with this you can compare to 66de756...ff889b7 which is the diff before the merge.

[koesie10]

Thanks, LGTM. I found two minor unexpected changes, but once those are fixed I think this can be merged. I didn't test it locally, but since all tests pass this should be fine.