Conversation
Agent-Logs-Url: https://github.com/ZayrexDev/ACGPicDownload/sessions/84502a8e-82b0-4633-b0ee-def3925ff21d Co-authored-by: ZayrexDev <73475219+ZayrexDev@users.noreply.github.com>
…fetch behavior Agent-Logs-Url: https://github.com/ZayrexDev/ACGPicDownload/sessions/84502a8e-82b0-4633-b0ee-def3925ff21d Co-authored-by: ZayrexDev <73475219+ZayrexDev@users.noreply.github.com>
Change return type to an immutable list when appending.
complete fetch todo parts
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors Pixiv artwork JSON handling by removing the stored “original JSON” blob from PixivArtwork, switching naming/full-result JSON output to re-serialize the PixivArtwork object, and expands the Pixiv CLI fetch subcommand to support additional modes and related-artwork traversal.
Changes:
- Removed
origJsonpersistence fromPixivArtworkand updated consumers to useJSONObject.from(artwork)instead. - Updated Pixiv CLI subcommands to support saving/loading
PixivArtworklists and added richer fetch modes (user/ranking/search) including optional related-artwork expansion. - Replaced several
new URL(...)constructions withURI.create(...).toURL()and adjusted Pixiv download full-result JSON output.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/xyz/zcraft/acgpicdownload/util/pixiv/PixivFetchUtil.java | Stops attaching raw JSON blobs to PixivArtwork instances during fetch/parse flows. |
| src/main/java/xyz/zcraft/acgpicdownload/util/pixiv/PixivArtwork.java | Makes several fields transient, adds isComplete(), and marks derived getters as non-serializable. |
| src/main/java/xyz/zcraft/acgpicdownload/util/pixiv/PixivAccount.java | Marks token transient to avoid serialization. |
| src/main/java/xyz/zcraft/acgpicdownload/util/pixiv/NamingRule.java | Switches naming argument source from origJson to JSONObject.from(artwork). |
| src/main/java/xyz/zcraft/acgpicdownload/util/dl/DownloadUtil.java | Uses URI.create(...).toURL() and serializes full-result JSON from PixivArtwork instead of origJson. |
| src/main/java/xyz/zcraft/acgpicdownload/Main.java | Uses getFirst()/removeFirst() for argument handling under Java 21. |
| src/main/java/xyz/zcraft/acgpicdownload/commands/pixiv/sub/Save.java | Saves List<PixivArtwork> directly to JSON instead of saving origJson objects. |
| src/main/java/xyz/zcraft/acgpicdownload/commands/pixiv/sub/Load.java | Loads PixivArtwork list from JSON and changes append behavior. |
| src/main/java/xyz/zcraft/acgpicdownload/commands/pixiv/sub/Fetch.java | Implements/expands Pixiv fetch modes and adds related-artwork expansion. |
| src/main/java/xyz/zcraft/acgpicdownload/commands/pixiv/sub/Complete.java | Switches incompleteness detection to !isComplete(). |
Comments suppressed due to low confidence (2)
src/main/java/xyz/zcraft/acgpicdownload/commands/pixiv/sub/Complete.java:74
- result and failedList are plain LinkedList instances that are mutated concurrently by multiple worker threads (result.add(...) and failedList.add(...)). This is not thread-safe and can corrupt the lists or throw exceptions. Use a thread-safe collection (e.g., ConcurrentLinkedQueue) or synchronize mutations / collect per-thread results and merge after the executor finishes.
final List<PixivArtwork> result = new LinkedList<>();
previous.stream().filter(p -> p.getLikeCount() != 0 || p.getBookmarkData() != null).forEach(result::add);
final List<PixivArtwork> list = previous.stream().filter(a -> !a.isComplete()).toList();
final List<PixivArtwork> failedList = new LinkedList<>();
out.info("Got " + list.size() + " artworks to complete data.");
AtomicInteger completed = new AtomicInteger();
AtomicInteger failed = new AtomicInteger();
try (ThreadPoolExecutor tpe = (ThreadPoolExecutor) Executors.newFixedThreadPool(threads)) {
list.forEach(t -> tpe.execute(() -> {
int tries = 1;
while (tries <= retries) {
try {
result.add(PixivFetchUtil.getArtwork(t.getId(), profile.cookie(), profile.proxyHost(), profile.proxyPort()));
completed.getAndIncrement();
return;
} catch (IOException e) {
tries++;
}
}
failedList.add(t);
failed.getAndIncrement();
}));
src/main/java/xyz/zcraft/acgpicdownload/util/dl/DownloadUtil.java:88
- URI.create(r.getUrl()).toURL() can throw IllegalArgumentException (unchecked) and will not be caught by the current catch(IOException) retry path, so invalid URLs will fail without retries and may escape as runtime exceptions. Consider validating/wrapping URI.create failures into IOException so the existing retry and error reporting behaves consistently.
URL url = URI.create(r.getUrl()).toURL();
URLConnection c = url.openConnection();
if (referer != null) {
c.setRequestProperty("referer", referer);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+466
to
+480
| private void fetchRelatedArtworks(List<PixivArtwork> artworks, int depth, Profile profile) throws IOException { | ||
| if (depth <= 0) return; | ||
| List<PixivArtwork> currentLayer = new LinkedList<>(artworks); | ||
| for (int i = 0; i < depth; i++) { | ||
| List<PixivArtwork> nextLayer = new LinkedList<>(); | ||
| for (PixivArtwork artwork : currentLayer) { | ||
| try { | ||
| nextLayer.addAll(PixivFetchUtil.getRelated(artwork, RELATED_ARTWORKS_LIMIT, profile.cookie(), profile.proxyHost(), profile.proxyPort())); | ||
| } catch (IOException e) { | ||
| out.warn(String.format("Failed to fetch related artworks for id=%s: %s", artwork.getId(), e.getMessage())); | ||
| } | ||
| } | ||
| artworks.addAll(nextLayer); | ||
| currentLayer = nextLayer; | ||
| } |
| var result = new LinkedList<PixivArtwork>(); | ||
| result.addAll(previous); | ||
| result.addAll(list); | ||
| return List.copyOf(result); |
| @@ -48,7 +48,7 @@ public List<PixivArtwork> invoke(List<String> argList, Profile profile, List<Pix | |||
| final List<PixivArtwork> result = new LinkedList<>(); | |||
|
|
|||
| previous.stream().filter(p -> p.getLikeCount() != 0 || p.getBookmarkData() != null).forEach(result::add); | |||
Comment on lines
30
to
36
| public static void download(File file, String link, String referer) throws IOException { | ||
| InputStream is = null; | ||
| FileOutputStream fos = null; | ||
| try { | ||
| URL url = new URL(link); | ||
| URL url = URI.create(link).toURL(); | ||
| HttpURLConnection c = (HttpURLConnection) url.openConnection(); | ||
|
|
Comment on lines
254
to
259
| for (int i = 0, pagesSize = pages.size(); i < pagesSize; i++) { | ||
| a.setProgress(i); | ||
| String s = pages.get(i); | ||
| URL url = new URL(s); | ||
| URL url = URI.create(s).toURL(); | ||
| URLConnection c = url.openConnection(); | ||
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.