Fork review for upstream #765: module repository loading#5
Fork review for upstream #765: module repository loading#5Shallow-dusty wants to merge 10 commits into
Conversation
Use the working backup repository API endpoint for module JSON, surface failed HTTP responses, and load full module details when README content is missing. Keeps the browser action on the public modules.lsposed.org web page because the backup host only serves the JSON API. Builds on the approach from JingMatrix#747 by byemaxx. Co-authored-by: Qing <44231502+byemaxx@users.noreply.github.com>
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dbb09630dd
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| loadLocalData(false); | ||
| loaded = true; |
There was a problem hiding this comment.
When a mirror returns HTTP 200 with a malformed or incompatible modules.json, loadLocalData(false) catches the Gson/stream exception internally, so execution returns here and loaded is set to true. That stops the new fallback loop before trying the next mirrors and leaves the just-written bad response in repo.json, so users whose first mirror has a bad cached/deployed response can lose the cached repo instead of falling back to modules.lsposed.org.
Useful? React with 👍 / 👎.
|
|
||
| public void loadRemoteReleases(String packageName) { | ||
| App.getOkHttpClient().newCall(new Request.Builder().url(String.format(repoUrl + "module/%s.json", packageName)).build()).enqueue(new Callback() { | ||
| loadRemoteReleases(packageName, 0); |
There was a problem hiding this comment.
After the repo index or a previous detail request succeeds on a fallback mirror, repoUrl is updated, but this entrypoint always restarts module-detail loads at repoUrls[0]. In environments where the first mirror is blocked or timing out while a later mirror works, every README/release load, including the new automatic README fetch, waits for that failing mirror before reaching the usable one; start from the current repoUrl instead of always index 0.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d08a313709
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| updateLatestVersion(repoModules, channel); | ||
| onlineModules = modules; |
There was a problem hiding this comment.
In the successful loadRemoteData() path, replaceRepoModules() now relies on updateLatestVersion() to fire onRepoLoaded(), but that notification happens before onlineModules is swapped. During the normal startup background refresh (loadLocalData(true) -> loadRemoteData()), listeners such as RepoFragment.refresh() and RepoItemFragment.refreshModuleFromRepo() read the old cached map and there is no later success notification after line 149, so the UI can keep showing stale module data until another manual refresh. Assign the new map before notifying, or emit a post-assignment onRepoLoaded() for successful remote loads.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f4ad0161a3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| private void loadRemoteModuleIfReadmeMissing() { | ||
| var currentModule = refreshModuleFromRepo(); | ||
| if (currentModule == null || currentModule.getName() == null) return; | ||
| if (remoteModuleLoadRequested || currentModule.releasesLoaded || hasReadme(currentModule)) return; |
There was a problem hiding this comment.
When the automatic detail fetch succeeds, remoteModuleLoadRequested stays true. If a repo-list refresh then replaces module with the summary object that still has no readme (for example the normal startup refresh completing after the detail request), this guard prevents loadRemoteModuleIfReadmeMissing() from fetching the detail JSON again, so the Readme tab is rendered as empty until the user manually loads releases. Clear this flag on successful onModuleReleasesLoaded() so a later summary refresh can rehydrate the readme.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 83553ab56b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| private void loadRemoteModuleIfReadmeMissing() { | ||
| var currentModule = refreshModuleFromRepo(); | ||
| if (currentModule == null || currentModule.getName() == null) return; | ||
| if (remoteModuleLoadRequested || currentModule.releasesLoaded || hasReadme(currentModule)) return; |
There was a problem hiding this comment.
When the automatic detail fetch is started and the fragment view is destroyed before the OkHttp callback runs, onDestroyView() removes this listener, so neither onThrowable nor onModuleReleasesLoaded clears remoteModuleLoadRequested. Returning to the same back-stack fragment then hits this guard and skips the readme/detail fetch even though no request is in flight, leaving modules with missing cached READMEs stuck on the empty placeholder until the user manually loads releases.
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 74f60aeb4d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (module == null || module.getName() == null) return module; | ||
| var updatedModule = RepoLoader.getInstance().getOnlineModule(module.getName()); | ||
| if (updatedModule != null) { | ||
| module = updatedModule; |
There was a problem hiding this comment.
When a module detail request has already populated this.module with README/full releases, a later repo refresh can replace RepoLoader's entry with the summary object from modules.json (the code below exists specifically for summaries that lack README data). This assignment then discards the already-loaded detail object, so the README/releases can flicker back to empty/truncated data and require another network request; if that retry fails, the user loses content that was already loaded. Please preserve or merge the detailed instance when it has README data or releasesLoaded instead of blindly overwriting it.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 145ea213aa
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| refreshModuleFromRepo(); | ||
| loadRemoteModuleIfReadmeMissing(); | ||
| if (releaseAdapter != null) { | ||
| runAsync(releaseAdapter::loadItems); |
There was a problem hiding this comment.
When a repo refresh arrives after this fragment has already fetched full module details, refreshModuleFromRepo() intentionally keeps the detailed module, but this reload still calls ReleaseAdapter.loadItems(), which first reads RepoLoader.getReleases(module.getName()). The refresh has just replaced RepoLoader's map with the modules.json summary, so the adapter is repopulated with the truncated summary release list while module.releasesLoaded remains true, removing the load-more row and hiding older releases. Prefer the detailed module when it is already loaded, or skip this reload in that case.
Useful? React with 👍 / 👎.
|
@codex review |
1 similar comment
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 145ea213aa
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| refreshModuleFromRepo(); | ||
| loadRemoteModuleIfReadmeMissing(); | ||
| if (releaseAdapter != null) { | ||
| runAsync(releaseAdapter::loadItems); |
There was a problem hiding this comment.
When a repo refresh arrives after this fragment has already loaded full module details, refreshModuleFromRepo() keeps module.releasesLoaded == true, but this reload calls ReleaseAdapter.loadItems(), which reads from RepoLoader.getReleases() first. The loader entry has just been replaced with the modules.json summary, so the adapter can show only the truncated summary releases while getItemCount() still suppresses the load-more row based on the detailed module, leaving users unable to load the missing releases after a background/full refresh.
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
1 participant
Fork-only Codex review surface for JingMatrix/Vector upstream PR JingMatrix#765 follow-up.\n\nHead under review: dbb0963 Retry module detail mirrors on parse failures.\n\nValidation already run locally: git diff --check; :app:compileDebugJavaWithJavac; :app:assembleDebug.\n\nThis PR is only for pre-upstream review and should not be merged.