From 865b49c554f32293548c3cb8eb317cf55fd0dc10 Mon Sep 17 00:00:00 2001 From: Josh Stark Date: Wed, 15 May 2019 20:25:05 +0100 Subject: [PATCH 1/2] Improve error handling for async tasks. Determine correct versioned latest tag --- build.gradle | 2 +- .../fleet/dockerhub/DockerHubV2Client.java | 47 ++++++++++++++----- .../linuxserver/fleet/thread/FleetTask.java | 34 +++++--------- 3 files changed, 49 insertions(+), 34 deletions(-) diff --git a/build.gradle b/build.gradle index c78c784..18c64c1 100644 --- a/build.gradle +++ b/build.gradle @@ -11,7 +11,7 @@ repositories { mavenCentral() } -version = '1.3.0' +version = '1.3.1' sourceSets { diff --git a/src/main/java/io/linuxserver/fleet/dockerhub/DockerHubV2Client.java b/src/main/java/io/linuxserver/fleet/dockerhub/DockerHubV2Client.java index ec8d963..3fa44e9 100644 --- a/src/main/java/io/linuxserver/fleet/dockerhub/DockerHubV2Client.java +++ b/src/main/java/io/linuxserver/fleet/dockerhub/DockerHubV2Client.java @@ -22,10 +22,7 @@ import io.linuxserver.fleet.rest.HttpException; import io.linuxserver.fleet.rest.RestClient; import io.linuxserver.fleet.rest.RestResponse; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; +import java.util.*; /** * Client class to interface with Docker Hub's V2 API. @@ -33,6 +30,7 @@ import java.util.Map; public class DockerHubV2Client implements DockerHubClient { static final String DOCKERHUB_BASE_URI = "https://hub.docker.com/v2"; + static final int DEFAULT_PAGE_SIZE = 1000; private final RestClient restClient; private final DockerHubAuthenticator authenticator; @@ -70,8 +68,7 @@ public class DockerHubV2Client implements DockerHubClient { try { - String url = DOCKERHUB_BASE_URI + "/repositories/" + repositoryName; - + String url = DOCKERHUB_BASE_URI + "/repositories/" + repositoryName + "?page_size=" + DEFAULT_PAGE_SIZE; while (url != null) { RestResponse response = doCall(url, DockerHubV2ImageListResult.class); @@ -112,13 +109,24 @@ public class DockerHubV2Client implements DockerHubClient { try { - String absoluteUrl = DOCKERHUB_BASE_URI + "/repositories/" + repositoryName + "/" + imageName + "/tags" ; + List tags = new ArrayList<>(); - RestResponse restResponse = doCall(absoluteUrl, DockerHubV2TagListResult.class); + String absoluteUrl = DOCKERHUB_BASE_URI + "/repositories/" + repositoryName + "/" + imageName + "/tags?page_size=" + DEFAULT_PAGE_SIZE; + while (absoluteUrl != null) { - List results = restResponse.getPayload().getResults(); - if (!results.isEmpty()) - return results.get(0); + RestResponse response = doCall(absoluteUrl, DockerHubV2TagListResult.class); + + if (isResponseOK(response)) { + + DockerHubV2TagListResult payload = response.getPayload(); + + tags.addAll(payload.getResults()); + absoluteUrl = payload.getNext(); + } + } + + if (!tags.isEmpty()) + return determineLatestVersionedTag(tags); return null; @@ -161,4 +169,21 @@ public class DockerHubV2Client implements DockerHubClient { private boolean isResponseUnauthorised(RestResponse restResponse) { return restResponse.getStatusCode() == 401; } + + private DockerHubV2Tag determineLatestVersionedTag(List results) { + + Optional trueLatest = results.stream().filter(tag -> "latest".equals(tag.getName())).findFirst(); + + if (trueLatest.isPresent()) { + + DockerHubV2Tag trueLatestTag = trueLatest.get(); + Optional versionedLatestTag = results.stream() + .filter(tag -> !tag.equals(trueLatestTag)) + .filter(tag -> tag.getFullSize() == trueLatestTag.getFullSize()).findFirst(); + + return versionedLatestTag.orElse(trueLatestTag); + } + + return results.get(0); + } } diff --git a/src/main/java/io/linuxserver/fleet/thread/FleetTask.java b/src/main/java/io/linuxserver/fleet/thread/FleetTask.java index 54af2d3..68f7a91 100644 --- a/src/main/java/io/linuxserver/fleet/thread/FleetTask.java +++ b/src/main/java/io/linuxserver/fleet/thread/FleetTask.java @@ -17,24 +17,26 @@ package io.linuxserver.fleet.thread; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + public abstract class FleetTask implements Runnable { - private TaskListener taskListener; + private final Logger logger; - public void setTaskListener(TaskListener listener) { - this.taskListener = listener; - } - - protected TaskListener getTaskListener() { - return taskListener; + FleetTask() { + logger = LoggerFactory.getLogger(getClass().getSimpleName()); } @Override public void run() { - onStart(toString() + " has started."); - executeTask(); - onEnd(toString() + " has finished."); + try { + executeTask(); + } catch (Exception e) { + logger.error("run() Caught unhandled exception during task execution.", e); + } + } protected abstract void executeTask(); @@ -43,16 +45,4 @@ public abstract class FleetTask implements Runnable { public String toString() { return getClass().getSimpleName(); } - - private void onStart(String message) { - - if (taskListener != null) - taskListener.onTaskStart(message); - } - - private void onEnd(String message) { - - if (taskListener != null) - taskListener.onTaskEnd(message); - } } From ae1518c759823d98bf0c1fbb7c3610b76df16749 Mon Sep 17 00:00:00 2001 From: Josh Stark Date: Sun, 19 May 2019 09:18:12 +0100 Subject: [PATCH 2/2] Moved tag filtering to separate class. Ensure state is reverted even on exception --- .../fleet/delegate/DockerHubDelegate.java | 17 ++++++-- .../fleet/dockerhub/DockerHubClient.java | 2 +- .../fleet/dockerhub/DockerHubV2Client.java | 28 ++----------- .../fleet/dockerhub/model/DockerHubV2Tag.java | 24 +++++++++++ .../fleet/dockerhub/util/DockerTagFinder.java | 42 +++++++++++++++++++ .../sync/DefaultLoggingSyncListener.java | 4 +- .../sync/DefaultSynchronisationState.java | 9 ++-- 7 files changed, 91 insertions(+), 35 deletions(-) create mode 100644 src/main/java/io/linuxserver/fleet/dockerhub/util/DockerTagFinder.java diff --git a/src/main/java/io/linuxserver/fleet/delegate/DockerHubDelegate.java b/src/main/java/io/linuxserver/fleet/delegate/DockerHubDelegate.java index 5395717..1f8e880 100644 --- a/src/main/java/io/linuxserver/fleet/delegate/DockerHubDelegate.java +++ b/src/main/java/io/linuxserver/fleet/delegate/DockerHubDelegate.java @@ -20,6 +20,7 @@ package io.linuxserver.fleet.delegate; import io.linuxserver.fleet.dockerhub.DockerHubClient; import io.linuxserver.fleet.dockerhub.model.DockerHubV2Image; import io.linuxserver.fleet.dockerhub.model.DockerHubV2Tag; +import io.linuxserver.fleet.dockerhub.util.DockerTagFinder; import io.linuxserver.fleet.model.DockerHubImage; import java.time.LocalDateTime; @@ -31,9 +32,12 @@ import java.util.List; public class DockerHubDelegate { private final DockerHubClient dockerHubClient; + private final DockerTagFinder dockerTagFinder; public DockerHubDelegate(DockerHubClient dockerHubClient) { + this.dockerHubClient = dockerHubClient; + this.dockerTagFinder = new DockerTagFinder(); } public List fetchAllRepositories() { @@ -52,14 +56,19 @@ public class DockerHubDelegate { return images; } + public List fetchAllTagsForImage(String repositoryName, String imageName) { + return dockerHubClient.fetchAllTagsForImage(repositoryName, imageName); + } + public String fetchLatestImageTag(String repositoryName, String imageName) { - DockerHubV2Tag dockerHubV2Tag = dockerHubClient.fetchLatestTagForImage(repositoryName, imageName); + List tags = fetchAllTagsForImage(repositoryName, imageName); - if (null != dockerHubV2Tag) - return dockerHubV2Tag.getName(); + if (tags.isEmpty()) { + return null; + } - return null; + return dockerTagFinder.findVersionedTagMatchingBranch(tags, "latest").getName(); } private DockerHubImage convertApiImageToInternalImage(DockerHubV2Image apiImage) { diff --git a/src/main/java/io/linuxserver/fleet/dockerhub/DockerHubClient.java b/src/main/java/io/linuxserver/fleet/dockerhub/DockerHubClient.java index 8a40891..a3f934c 100644 --- a/src/main/java/io/linuxserver/fleet/dockerhub/DockerHubClient.java +++ b/src/main/java/io/linuxserver/fleet/dockerhub/DockerHubClient.java @@ -31,5 +31,5 @@ public interface DockerHubClient { DockerHubV2Image fetchImageFromRepository(String repositoryName, String imageName); - DockerHubV2Tag fetchLatestTagForImage(String repositoryName, String imageName); + List fetchAllTagsForImage(String repositoryName, String imageName); } diff --git a/src/main/java/io/linuxserver/fleet/dockerhub/DockerHubV2Client.java b/src/main/java/io/linuxserver/fleet/dockerhub/DockerHubV2Client.java index 3fa44e9..1b1f243 100644 --- a/src/main/java/io/linuxserver/fleet/dockerhub/DockerHubV2Client.java +++ b/src/main/java/io/linuxserver/fleet/dockerhub/DockerHubV2Client.java @@ -29,8 +29,8 @@ import java.util.*; */ public class DockerHubV2Client implements DockerHubClient { - static final String DOCKERHUB_BASE_URI = "https://hub.docker.com/v2"; - static final int DEFAULT_PAGE_SIZE = 1000; + private static final int DEFAULT_PAGE_SIZE = 1000; + static final String DOCKERHUB_BASE_URI = "https://hub.docker.com/v2"; private final RestClient restClient; private final DockerHubAuthenticator authenticator; @@ -105,7 +105,7 @@ public class DockerHubV2Client implements DockerHubClient { } @Override - public DockerHubV2Tag fetchLatestTagForImage(String repositoryName, String imageName) { + public List fetchAllTagsForImage(String repositoryName, String imageName) { try { @@ -125,10 +125,7 @@ public class DockerHubV2Client implements DockerHubClient { } } - if (!tags.isEmpty()) - return determineLatestVersionedTag(tags); - - return null; + return tags; } catch (HttpException e) { throw new DockerHubException("Unable to get tags for " + repositoryName + "/" + imageName, e); @@ -169,21 +166,4 @@ public class DockerHubV2Client implements DockerHubClient { private boolean isResponseUnauthorised(RestResponse restResponse) { return restResponse.getStatusCode() == 401; } - - private DockerHubV2Tag determineLatestVersionedTag(List results) { - - Optional trueLatest = results.stream().filter(tag -> "latest".equals(tag.getName())).findFirst(); - - if (trueLatest.isPresent()) { - - DockerHubV2Tag trueLatestTag = trueLatest.get(); - Optional versionedLatestTag = results.stream() - .filter(tag -> !tag.equals(trueLatestTag)) - .filter(tag -> tag.getFullSize() == trueLatestTag.getFullSize()).findFirst(); - - return versionedLatestTag.orElse(trueLatestTag); - } - - return results.get(0); - } } diff --git a/src/main/java/io/linuxserver/fleet/dockerhub/model/DockerHubV2Tag.java b/src/main/java/io/linuxserver/fleet/dockerhub/model/DockerHubV2Tag.java index 02d3c36..5e8dd28 100644 --- a/src/main/java/io/linuxserver/fleet/dockerhub/model/DockerHubV2Tag.java +++ b/src/main/java/io/linuxserver/fleet/dockerhub/model/DockerHubV2Tag.java @@ -34,4 +34,28 @@ public class DockerHubV2Tag { public long getFullSize() { return fullSize; } + + @Override + public String toString() { + return name; + } + + @Override + public int hashCode() { + return name.hashCode(); + } + + @Override + public boolean equals(Object other) { + + if (!(other instanceof DockerHubV2Tag)) { + return false; + } + + if (other == this) { + return true; + } + + return name.equals(((DockerHubV2Tag) other).name); + } } diff --git a/src/main/java/io/linuxserver/fleet/dockerhub/util/DockerTagFinder.java b/src/main/java/io/linuxserver/fleet/dockerhub/util/DockerTagFinder.java new file mode 100644 index 0000000..f1d2430 --- /dev/null +++ b/src/main/java/io/linuxserver/fleet/dockerhub/util/DockerTagFinder.java @@ -0,0 +1,42 @@ +/* + * Copyright (c) 2019 LinuxServer.io + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +package io.linuxserver.fleet.dockerhub.util; + +import io.linuxserver.fleet.dockerhub.model.DockerHubV2Tag; + +import java.util.List; +import java.util.Optional; + +public class DockerTagFinder { + + public DockerHubV2Tag findVersionedTagMatchingBranch(List tags, String namedBranch) { + + Optional trueLatest = tags.stream().filter(tag -> namedBranch.equals(tag.getName())).findFirst(); + + if (trueLatest.isPresent()) { + + DockerHubV2Tag trueLatestTag = trueLatest.get(); + Optional versionedLatestTag = tags.stream() + .filter(tag -> !tag.equals(trueLatestTag) && tag.getFullSize() == trueLatestTag.getFullSize()).findFirst(); + + return versionedLatestTag.orElse(trueLatestTag); + } + + return tags.get(0); + } +} diff --git a/src/main/java/io/linuxserver/fleet/sync/DefaultLoggingSyncListener.java b/src/main/java/io/linuxserver/fleet/sync/DefaultLoggingSyncListener.java index 25d6fe8..dbefc7b 100644 --- a/src/main/java/io/linuxserver/fleet/sync/DefaultLoggingSyncListener.java +++ b/src/main/java/io/linuxserver/fleet/sync/DefaultLoggingSyncListener.java @@ -28,7 +28,7 @@ public class DefaultLoggingSyncListener implements SynchronisationListener { @Override public void onSynchronisationStart() { - LOGGER.info("Sync started"); + LOGGER.info("Sync started."); } @Override @@ -38,7 +38,7 @@ public class DefaultLoggingSyncListener implements SynchronisationListener { @Override public void onImageUpdated(ImageUpdateEvent event) { - LOGGER.info("({}/{}) {}.", event.getCurrentPosition(), event.getTotalImages(), event.getImage().getName()); + } @Override diff --git a/src/main/java/io/linuxserver/fleet/sync/DefaultSynchronisationState.java b/src/main/java/io/linuxserver/fleet/sync/DefaultSynchronisationState.java index 0650814..72d1f93 100644 --- a/src/main/java/io/linuxserver/fleet/sync/DefaultSynchronisationState.java +++ b/src/main/java/io/linuxserver/fleet/sync/DefaultSynchronisationState.java @@ -82,10 +82,10 @@ public class DefaultSynchronisationState implements SynchronisationState { if (isProcessIdle()) { - onStart(context); - try { + onStart(context); + List repositories = context.getDockerHubDelegate().fetchAllRepositories(); onRepositoryScanned(context, repositories); @@ -98,9 +98,10 @@ public class DefaultSynchronisationState implements SynchronisationState { LOGGER.error("Synchronisation process failed on the first step. Will skip for now.", e); onSkip(context); - } - onFinish(context); + } finally { + onFinish(context); + } } else { onSkip(context);