Merge pull request #301 from avimanyum/refactor_all_command

Refactor all subcommd

dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/storage/GitHubJsonStore.java

@@ -1,8 +1,7 @@
 package com.salesforce.dockerfileimageupdate.storage;
 
 import com.google.gson.*;
-import com.salesforce.dockerfileimageupdate.utils.Constants;
-import com.salesforce.dockerfileimageupdate.utils.GitHubUtil;
+import com.salesforce.dockerfileimageupdate.utils.*;
 import org.kohsuke.github.GHContent;
 import org.kohsuke.github.GHMyself;
 import org.kohsuke.github.GHRepository;
@@ -13,6 +12,7 @@
 import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.nio.file.Paths;
+import java.util.*;
 
 public class GitHubJsonStore {
     private static final Logger log = LoggerFactory.getLogger(GitHubJsonStore.class);
@@ -88,4 +88,31 @@ protected String getAndModifyJsonString(JsonElement json, String img, String tag
         Gson gson = new GsonBuilder().setPrettyPrinting().create();
         return gson.toJson(json);
     }
+
+    public Set<Map.Entry<String, JsonElement>> parseStoreToImagesMap(DockerfileGitHubUtil dockerfileGitHubUtil, String storeName)
+            throws IOException, InterruptedException {
+        GHMyself myself = dockerfileGitHubUtil.getMyself();
+        String login = myself.getLogin();
+        GHRepository store = dockerfileGitHubUtil.getRepo(Paths.get(login, storeName).toString());
+
+        GHContent storeContent = dockerfileGitHubUtil.tryRetrievingContent(store, Constants.STORE_JSON_FILE,
+                store.getDefaultBranch());
+
+        if (storeContent == null) {
+            return Collections.emptySet();
+        }
+
+        JsonElement json;
+        try (InputStream stream = storeContent.read(); InputStreamReader streamR = new InputStreamReader(stream)) {
+            try {
+                json = JsonParser.parseReader(streamR);
+            } catch (JsonParseException e) {
+                log.warn("Not a JSON format store.");
+                return Collections.emptySet();
+            }
+        }
+
+        JsonElement imagesJson = json.getAsJsonObject().get("images");
+        return imagesJson.getAsJsonObject().entrySet();
+    }
 }

dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/subcommands/impl/Parent.java

@@ -8,26 +8,22 @@
 
 package com.salesforce.dockerfileimageupdate.subcommands.impl;
 
-import com.google.common.collect.Multimap;
+
 import com.salesforce.dockerfileimageupdate.SubCommand;
 import com.salesforce.dockerfileimageupdate.model.GitForkBranch;
-import com.salesforce.dockerfileimageupdate.model.GitHubContentToProcess;
-import com.salesforce.dockerfileimageupdate.model.PullRequestInfo;
 import com.salesforce.dockerfileimageupdate.process.ForkableRepoValidator;
 import com.salesforce.dockerfileimageupdate.process.GitHubPullRequestSender;
 import com.salesforce.dockerfileimageupdate.subcommands.ExecutableWithNamespace;
 import com.salesforce.dockerfileimageupdate.utils.Constants;
 import com.salesforce.dockerfileimageupdate.utils.DockerfileGitHubUtil;
-import com.salesforce.dockerfileimageupdate.utils.ResultsProcessor;
+import com.salesforce.dockerfileimageupdate.utils.PullRequests;
 import net.sourceforge.argparse4j.inf.Namespace;
 import org.kohsuke.github.GHContent;
-import org.kohsuke.github.GHRepository;
 import org.kohsuke.github.PagedSearchIterable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
-import java.util.ArrayList;
 import java.util.List;
 import java.util.Optional;
 
@@ -55,101 +51,41 @@ public void execute(final Namespace ns, DockerfileGitHubUtil dockerfileGitHubUti
                     + "be skipped.", Constants.SKIP_PR_CREATION);
             return;
         }
-        GitHubPullRequestSender pullRequestSender =
-                new GitHubPullRequestSender(dockerfileGitHubUtil, new ForkableRepoValidator(dockerfileGitHubUtil),
-                        ns.get(Constants.GIT_REPO_EXCLUDES));
-
-        GitForkBranch gitForkBranch =
-                new GitForkBranch(ns.get(Constants.IMG), ns.get(Constants.TAG), ns.get(Constants.GIT_BRANCH));
-
+        PullRequests pullRequests = getPullRequests();
+        GitHubPullRequestSender pullRequestSender = getPullRequestSender(dockerfileGitHubUtil, ns);
+        GitForkBranch gitForkBranch = getGitForkBranch(ns);
         log.info("Finding Dockerfiles with the given image...");
 
         Integer gitApiSearchLimit = ns.get(Constants.GIT_API_SEARCH_LIMIT);
         Optional<List<PagedSearchIterable<GHContent>>> contentsWithImage = dockerfileGitHubUtil.getGHContents(ns.get(Constants.GIT_ORG), img, gitApiSearchLimit);
+
         if (contentsWithImage.isPresent()) {
             List<PagedSearchIterable<GHContent>> contentsFoundWithImage = contentsWithImage.get();
             for (int i = 0; i < contentsFoundWithImage.size(); i++ ) {
-                Multimap<String, GitHubContentToProcess> pathToDockerfilesInParentRepo =
-                        pullRequestSender.forkRepositoriesFoundAndGetPathToDockerfiles(contentsFoundWithImage.get(i), gitForkBranch);
-
-
-                List<IOException> exceptions = new ArrayList<>();
-                List<String> skippedRepos = new ArrayList<>();
-
-                for (String currUserRepo : pathToDockerfilesInParentRepo.keySet()) {
-                    Optional<GitHubContentToProcess> forkWithContentPaths =
-                            pathToDockerfilesInParentRepo.get(currUserRepo).stream().findFirst();
-                    if (forkWithContentPaths.isPresent()) {
-                        try {
-                            changeDockerfiles(ns, pathToDockerfilesInParentRepo, forkWithContentPaths.get(), skippedRepos);
-                        } catch (IOException e) {
-                            log.error(String.format("Error changing Dockerfile for %s", forkWithContentPaths.get().getParent().getFullName()), e);
-                            exceptions.add(e);
-                        }
-                    } else {
-                        log.warn("Didn't find fork for {} so not changing Dockerfiles", currUserRepo);
-                    }
+                try {
+                    pullRequests.prepareToCreate(ns, pullRequestSender,
+                            contentsFoundWithImage.get(i), gitForkBranch, dockerfileGitHubUtil);
+                } catch (IOException e) {
+                    log.error("Could not send pull request.", e);
                 }
-
-                ResultsProcessor.processResults(skippedRepos, exceptions, log);
             }
         }
     }
 
-    protected void loadDockerfileGithubUtil(DockerfileGitHubUtil _dockerfileGitHubUtil) {
-        dockerfileGitHubUtil = _dockerfileGitHubUtil;
+    protected PullRequests getPullRequests(){
+        return new PullRequests();
     }
 
-    protected void changeDockerfiles(Namespace ns,
-                                     Multimap<String, GitHubContentToProcess> pathToDockerfilesInParentRepo,
-                                     GitHubContentToProcess gitHubContentToProcess,
-                                     List<String> skippedRepos) throws IOException,
-            InterruptedException {
-        // Should we skip doing a getRepository just to fill in the parent value? We already know this to be the parent...
-        GHRepository parent = gitHubContentToProcess.getParent();
-        GHRepository forkedRepo = gitHubContentToProcess.getFork();
-        // TODO: Getting a null pointer here for someone... probably just fixed this since we have parent
-        String parentName = parent.getFullName();
-
-        log.info("Fixing Dockerfiles in {} to PR to {}", forkedRepo.getFullName(), parent.getFullName());
-        GitForkBranch gitForkBranch = new GitForkBranch(ns.get(Constants.IMG), ns.get(Constants.TAG), ns.get(Constants.GIT_BRANCH));
-
-        dockerfileGitHubUtil.createOrUpdateForkBranchToParentDefault(parent, forkedRepo, gitForkBranch);
-
-        // loop through all the Dockerfiles in the same repo
-        boolean isContentModified = false;
-        boolean isRepoSkipped = true;
-        for (GitHubContentToProcess forkWithCurrentContentPath : pathToDockerfilesInParentRepo.get(parentName)) {
-            String pathToDockerfile = forkWithCurrentContentPath.getContentPath();
-            GHContent content = dockerfileGitHubUtil.tryRetrievingContent(forkedRepo, pathToDockerfile, gitForkBranch.getBranchName());
-            if (content == null) {
-                log.info("No Dockerfile found at path: '{}'", pathToDockerfile);
-            } else {
-                dockerfileGitHubUtil.modifyOnGithub(content, gitForkBranch.getBranchName(), gitForkBranch.getImageName(), gitForkBranch.getImageTag(),
-                        ns.get(Constants.GIT_ADDITIONAL_COMMIT_MESSAGE));
-                isContentModified = true;
-                isRepoSkipped = false;
-            }
-        }
+    protected GitForkBranch getGitForkBranch(Namespace ns){
+        return new GitForkBranch(ns.get(Constants.IMG), ns.get(Constants.TAG), ns.get(Constants.GIT_BRANCH));
+    }
 
-        if (isRepoSkipped) {
-            log.info("Skipping repo '{}' because contents of it's fork could not be retrieved. Moving ahead...",
-                    parentName);
-            skippedRepos.add(forkedRepo.getFullName());
-        }
+    protected GitHubPullRequestSender getPullRequestSender(DockerfileGitHubUtil dockerfileGitHubUtil, Namespace ns){
+        return new GitHubPullRequestSender(dockerfileGitHubUtil, new ForkableRepoValidator(dockerfileGitHubUtil),
+                ns.get(Constants.GIT_REPO_EXCLUDES));
+    }
 
-        if (isContentModified) {
-            PullRequestInfo pullRequestInfo =
-                    new PullRequestInfo(ns.get(Constants.GIT_PR_TITLE),
-                            gitForkBranch.getImageName(),
-                            gitForkBranch.getImageTag(),
-                            ns.get(Constants.GIT_PR_BODY));
-            // TODO: get the new PR number and cross post over to old ones
-            dockerfileGitHubUtil.createPullReq(parent,
-                    gitForkBranch.getBranchName(),
-                    forkedRepo,
-                    pullRequestInfo);
-            // TODO: Run through PRs in fork to see if they have head branches that match the prefix and close those?
-        }
+    protected void loadDockerfileGithubUtil(DockerfileGitHubUtil _dockerfileGitHubUtil) {
+        dockerfileGitHubUtil = _dockerfileGitHubUtil;
     }
 }

dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtil.java

@@ -9,12 +9,11 @@
 package com.salesforce.dockerfileimageupdate.utils;
 
 import com.google.common.collect.Multimap;
-import com.salesforce.dockerfileimageupdate.model.FromInstruction;
-import com.salesforce.dockerfileimageupdate.model.GitForkBranch;
-import com.salesforce.dockerfileimageupdate.model.PullRequestInfo;
+import com.salesforce.dockerfileimageupdate.model.*;
 import com.salesforce.dockerfileimageupdate.repository.GitHub;
 import com.salesforce.dockerfileimageupdate.search.GitHubImageSearchTermList;
 import com.salesforce.dockerfileimageupdate.storage.GitHubJsonStore;
+import net.sourceforge.argparse4j.inf.*;
 import org.kohsuke.github.*;
 import java.util.stream.Collectors;
 import org.slf4j.Logger;
@@ -437,4 +436,55 @@ public Optional<List<PagedSearchIterable<GHContent>>> getGHContents(String org,
         }
         return contentsWithImage;
     }
+
+    public void changeDockerfiles(Namespace ns,
+                                     Multimap<String, GitHubContentToProcess> pathToDockerfilesInParentRepo,
+                                     GitHubContentToProcess gitHubContentToProcess,
+                                     List<String> skippedRepos,
+                                     GitForkBranch gitForkBranch) throws IOException,
+            InterruptedException {
+        // Should we skip doing a getRepository just to fill in the parent value? We already know this to be the parent...
+        GHRepository parent = gitHubContentToProcess.getParent();
+        GHRepository forkedRepo = gitHubContentToProcess.getFork();
+        String parentName = parent.getFullName();
+
+        log.info("Fixing Dockerfiles in {} to PR to {}", forkedRepo.getFullName(), parent.getFullName());
+
+        createOrUpdateForkBranchToParentDefault(parent, forkedRepo, gitForkBranch);
+
+        // loop through all the Dockerfiles in the same repo
+        boolean isContentModified = false;
+        boolean isRepoSkipped = true;
+        for (GitHubContentToProcess forkWithCurrentContentPath : pathToDockerfilesInParentRepo.get(parentName)) {
+            String pathToDockerfile = forkWithCurrentContentPath.getContentPath();
+            GHContent content = tryRetrievingContent(forkedRepo, pathToDockerfile, gitForkBranch.getBranchName());
+            if (content == null) {
+                log.info("No Dockerfile found at path: '{}'", pathToDockerfile);
+            } else {
+                modifyOnGithub(content, gitForkBranch.getBranchName(), gitForkBranch.getImageName(), gitForkBranch.getImageTag(),
+                        ns.get(Constants.GIT_ADDITIONAL_COMMIT_MESSAGE));
+                isContentModified = true;
+                isRepoSkipped = false;
+            }
+        }
+
+        if (isRepoSkipped) {
+            log.info("Skipping repo '{}' because contents of it's fork could not be retrieved. Moving ahead...",
+                    parentName);
+            skippedRepos.add(forkedRepo.getFullName());
+        }
+
+        if (isContentModified) {
+            PullRequestInfo pullRequestInfo =
+                    new PullRequestInfo(ns.get(Constants.GIT_PR_TITLE),
+                            gitForkBranch.getImageName(),
+                            gitForkBranch.getImageTag(),
+                            ns.get(Constants.GIT_PR_BODY));
+
+            createPullReq(parent,
+                    gitForkBranch.getBranchName(),
+                    forkedRepo,
+                    pullRequestInfo);
+        }
+    }
 }

dockerfile-image-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/PullRequests.java

@@ -0,0 +1,43 @@
+package com.salesforce.dockerfileimageupdate.utils;
+
+import com.google.common.collect.*;
+import com.salesforce.dockerfileimageupdate.model.*;
+import com.salesforce.dockerfileimageupdate.process.*;
+import net.sourceforge.argparse4j.inf.*;
+import org.kohsuke.github.*;
+import org.slf4j.*;
+
+import java.io.*;
+import java.util.*;
+
+public class PullRequests {
+    private static final Logger log = LoggerFactory.getLogger(PullRequests.class);
+    public void prepareToCreate(final Namespace ns,
+                                            GitHubPullRequestSender pullRequestSender,
+                                            PagedSearchIterable<GHContent> contentsFoundWithImage,
+                                            GitForkBranch gitForkBranch,
+                                            DockerfileGitHubUtil dockerfileGitHubUtil) throws IOException {
+        Multimap<String, GitHubContentToProcess> pathToDockerfilesInParentRepo =
+                pullRequestSender.forkRepositoriesFoundAndGetPathToDockerfiles(contentsFoundWithImage, gitForkBranch);
+        List<IOException> exceptions = new ArrayList<>();
+        List<String> skippedRepos = new ArrayList<>();
+        for (String currUserRepo : pathToDockerfilesInParentRepo.keySet()) {
+            Optional<GitHubContentToProcess> forkWithContentPaths =
+                    pathToDockerfilesInParentRepo.get(currUserRepo).stream().findFirst();
+            if (forkWithContentPaths.isPresent()) {
+                try {
+                    dockerfileGitHubUtil.changeDockerfiles(ns,
+                            pathToDockerfilesInParentRepo,
+                            forkWithContentPaths.get(), skippedRepos, gitForkBranch);
+                } catch (IOException | InterruptedException e) {
+                    log.error(String.format("Error changing Dockerfile for %s", forkWithContentPaths.get().getParent().getFullName()), e);
+                    exceptions.add((IOException) e);
+                }
+            } else {
+                log.warn("Didn't find fork for {} so not changing Dockerfiles", currUserRepo);
+            }
+        }
+        ResultsProcessor.processResults(skippedRepos, exceptions, log);
+    }
+}
+

dockerfile-image-update/src/test/java/com/salesforce/dockerfileimageupdate/storage/GitHubJsonStoreTest.java

@@ -2,11 +2,16 @@
 
 import com.google.gson.JsonElement;
 import com.google.gson.JsonParser;
-import com.salesforce.dockerfileimageupdate.utils.GitHubUtil;
+import com.salesforce.dockerfileimageupdate.utils.*;
+import org.kohsuke.github.*;
 import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
 
+import java.util.*;
+
+import static org.mockito.Matchers.anyString;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 import static org.testng.Assert.assertEquals;
 
 public class GitHubJsonStoreTest {
@@ -88,4 +93,23 @@ public void testGetAndModifyJsonString(String storeContent, String image, String
         String output = new GitHubJsonStore(gitHubUtil, "test").getAndModifyJsonString(json, image, tag);
         assertEquals(output, expectedOutput);
     }
+
+    @Test
+    public void testParseStoreToImagesMap() throws Exception {
+        GitHubJsonStore gitHubJsonStore = mock(GitHubJsonStore.class);
+        DockerfileGitHubUtil dockerfileGitHubUtil = mock(DockerfileGitHubUtil.class);
+        GHMyself ghMyself = mock(GHMyself.class);
+        when(dockerfileGitHubUtil.getMyself()).thenReturn(ghMyself);
+        String dummyLogin = "dummy_login";
+        when(ghMyself.getLogin()).thenReturn(dummyLogin);
+        GHRepository store = mock(GHRepository.class);
+        when(dockerfileGitHubUtil.getRepo(anyString())).thenReturn(store);
+        String defaultBranch = "default-branch";
+        when(store.getDefaultBranch()).thenReturn(defaultBranch);
+        Set<String> EmptySet = Collections.emptySet();
+        when(dockerfileGitHubUtil.tryRetrievingContent(store, "store.json", defaultBranch)).thenReturn(null);
+        Set<Map.Entry<String, JsonElement>> actualResult =
+                gitHubJsonStore.parseStoreToImagesMap(dockerfileGitHubUtil, "store");
+        assertEquals(actualResult, EmptySet);
+    }
 }