Remove authlib-injector download code #4996
Conversation
Authlib-injector is already bundled into HMCL in HMCL-dev#4246. Due to this, `AuthlibInjectorDownloader` is no longer needed. This commit removes `AuthlibInjectorDownloader`, `AuthlibInjectorDownloadException`, `AuthlibInjectorExtractor`, `AuthlibInjectorArtifactInfo`, and `AuthlibInjectorArtifactProvider` classes, plus the `account.failed.injector_download_failure` i18n key. `AuthlibInjectorArtifactProvider` is replaced with `Supplier<Path>` to provide the jar path of authlib-injector.
yushijinhun
requested review from
Glavo and
Copilot
and removed request for
Copilot
…tor-download # Conflicts: # HMCL/src/main/java/org/jackhuang/hmcl/game/LauncherHelper.java
There was a problem hiding this comment.
Pull request overview
This PR removes the authlib-injector download infrastructure since authlib-injector is now bundled directly with HMCL (as mentioned in #4246). The change simplifies the codebase by replacing the complex download/extraction provider system with a straightforward Supplier<Path> approach.
Key Changes:
- Replaced
AuthlibInjectorArtifactProviderinterface withSupplier<Path>for providing authlib-injector JAR paths - Removed asynchronous download logic from
AuthlibInjectorAccount, simplifying the login flow to be synchronous - Implemented new extraction logic in
Accounts.resolveAuthlibInjectorArtifact()to extract bundled authlib-injector on first use
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
OfflineAccountFactory.java |
Updated constructor and method calls to use Supplier<Path> instead of AuthlibInjectorArtifactProvider |
OfflineAccount.java |
Simplified artifact handling by removing async operations and directly using the Path supplier |
BoundAuthlibInjectorAccountFactory.java |
Changed parameter type from AuthlibInjectorArtifactProvider to Supplier<Path> |
AuthlibInjectorAccountFactory.java |
Updated to use Supplier<Path> for artifact provision |
AuthlibInjectorAccount.java |
Removed async download logic, simplified to synchronous artifact resolution |
SimpleAuthlibInjectorArtifactProvider.java |
Deleted - no longer needed with bundled artifact approach |
AuthlibInjectorExtractor.java |
Deleted - extraction logic moved inline to Accounts class |
AuthlibInjectorDownloader.java |
Deleted - download functionality removed as artifact is bundled |
AuthlibInjectorDownloadException.java |
Deleted - exception no longer needed |
AuthlibInjectorArtifactProvider.java |
Deleted - interface replaced by Supplier<Path> |
AuthlibInjectorArtifactInfo.java |
Deleted - artifact metadata no longer needed |
Accounts.java |
Added resolveAuthlibInjectorArtifact() method to handle bundled artifact extraction on demand |
LauncherHelper.java |
Removed exception handling for AuthlibInjectorDownloadException |
I18N*.properties |
Removed account.failed.injector_download_failure key from all language files |
Comments suppressed due to low confidence (1)
HMCLCore/src/main/java/org/jackhuang/hmcl/auth/offline/OfflineAccount.java:108
- The call to authlibInjectorArtifactProvider.get() can throw UncheckedIOException (from the extraction logic in Accounts.resolveAuthlibInjectorArtifact), but this unchecked exception is not handled. Consider wrapping the call in a try-catch block and converting it to an AuthenticationException to maintain consistent error handling behavior.
public AuthInfo logIn() throws AuthenticationException {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
HMCLCore/src/main/java/org/jackhuang/hmcl/auth/offline/OfflineAccount.java
Show resolved
Hide resolved
...ore/src/main/java/org/jackhuang/hmcl/auth/authlibinjector/AuthlibInjectorAccountFactory.java
Show resolved
Hide resolved
...rc/main/java/org/jackhuang/hmcl/auth/authlibinjector/BoundAuthlibInjectorAccountFactory.java
Show resolved
Hide resolved
| if (artifact.isPresent() && prefetchedMeta.isPresent()) { | ||
| return new AuthlibInjectorAuthInfo(auth, artifact.get(), server, prefetchedMeta.get()); | ||
| if (prefetchedMeta.isPresent()) { | ||
| Path artifactPath = artifactProvider.get(); |
There was a problem hiding this comment.
The call to artifactProvider.get() can throw UncheckedIOException (from the extraction logic in Accounts.resolveAuthlibInjectorArtifact), but this unchecked exception is not handled. Consider wrapping the call in a try-catch block and converting it to an AuthenticationException to maintain consistent error handling behavior.
HMCLCore/src/main/java/org/jackhuang/hmcl/auth/authlibinjector/AuthlibInjectorAccount.java
Show resolved
Hide resolved
| } catch (Exception e) { | ||
| throw new AuthenticationException(e); | ||
| } | ||
| return new OfflineAuthInfo(authInfo, authlibInjectorArtifactProvider.get()); |
There was a problem hiding this comment.
这里是可以抛出 UncheckedIOException 的，这处修改让语义变化了。
|
要不把 |
解压报错应该非常罕见吧？从 JAR 内读取资源很难想象会出错，出错也是写入到磁盘上时候出错。 |
对，就是写入硬盘时可能会产生 |
2 participants
Authlib-injector is already bundled into HMCL in #4246. Due to this,
AuthlibInjectorDownloaderis no longer needed. This commit removesAuthlibInjectorDownloader,AuthlibInjectorDownloadException,AuthlibInjectorExtractor,AuthlibInjectorArtifactInfo, andAuthlibInjectorArtifactProviderclasses, plus theaccount.failed.injector_download_failurei18n key.AuthlibInjectorArtifactProvideris replaced withSupplier<Path>to provide the jar path of authlib-injector.