Skip to content

Commit

Permalink
Merge pull request #316 from avimanyum/fix_all_bug
Browse files Browse the repository at this point in the history 
Catch exceptions while finding files with image
  • Loading branch information
@afalko
afalko authored Feb 23, 2022
2 parents bd06630 + 353c4e1 commit e18ae3e
Show file tree
Hide file tree
Showing 4 changed files with 271 additions and 28 deletions.
66 changes: 51 additions & 15 deletions ...image-update/src/main/java/com/salesforce/dockerfileimageupdate/subcommands/impl/All.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import com.salesforce.dockerfileimageupdate.subcommands.ExecutableWithNamespace;
import com.salesforce.dockerfileimageupdate.utils.Constants;
import com.salesforce.dockerfileimageupdate.utils.DockerfileGitHubUtil;
import com.salesforce.dockerfileimageupdate.utils.ProcessingErrors;
import com.salesforce.dockerfileimageupdate.utils.PullRequests;
import net.sourceforge.argparse4j.inf.Namespace;
import org.kohsuke.github.*;
Expand All @@ -23,6 +24,7 @@

import java.io.IOException;
import java.util.*;
import java.util.concurrent.atomic.AtomicInteger;

@SubCommand(help="updates all repositories' Dockerfiles",
requiredParams = {Constants.STORE})
Expand All @@ -45,29 +47,63 @@ public void execute(final Namespace ns, final DockerfileGitHubUtil dockerfileGit
// the org gets included in the search query.
orgsToIncludeInSearch.put(ns.get(Constants.GIT_ORG), true);
}
List<ProcessingErrors> imagesThatCouldNotBeProcessed = new LinkedList<>();
AtomicInteger numberOfImagesToProcess = new AtomicInteger();
for (Map.Entry<String, JsonElement> imageToTag : imageToTagStore) {
numberOfImagesToProcess.getAndIncrement();
String image = imageToTag.getKey();
String tag = imageToTag.getValue().getAsString();
PullRequests pullRequests = getPullRequests();
GitHubPullRequestSender pullRequestSender = getPullRequestSender(dockerfileGitHubUtil, ns);
GitForkBranch gitForkBranch = getGitForkBranch(image, tag, ns);
try {
PullRequests pullRequests = getPullRequests();
GitHubPullRequestSender pullRequestSender = getPullRequestSender(dockerfileGitHubUtil, ns);
GitForkBranch gitForkBranch = getGitForkBranch(image, tag, ns);

log.info("Finding Dockerfiles with the image name {}...", image);
log.info("Finding Dockerfiles with the image name {}...", image);

Optional<List<PagedSearchIterable<GHContent>>> contentsWithImage =
this.dockerfileGitHubUtil.findFilesWithImage(image, orgsToIncludeInSearch, gitApiSearchLimit);
Optional<List<PagedSearchIterable<GHContent>>> contentsWithImage =
this.dockerfileGitHubUtil.findFilesWithImage(image, orgsToIncludeInSearch, gitApiSearchLimit);

if (contentsWithImage.isPresent()) {
contentsWithImage.get().forEach(pagedSearchIterable -> {
try {
pullRequests.prepareToCreate(ns, pullRequestSender,
pagedSearchIterable, gitForkBranch, dockerfileGitHubUtil);
} catch (IOException e) {
log.error("Could not send pull request.");
}
});
if (contentsWithImage.isPresent()) {
contentsWithImage.get().forEach(pagedSearchIterable -> {
try {
pullRequests.prepareToCreate(ns, pullRequestSender,
pagedSearchIterable, gitForkBranch, dockerfileGitHubUtil);
} catch (IOException e) {
log.error("Could not send pull request for image {}.", image);
processErrors(image, tag, e, imagesThatCouldNotBeProcessed);
}
});
}
} catch (GHException | HttpException e){
log.error("Could not perform Github search for the image {}. Trying to proceed...", image);
processErrors(image, tag, e, imagesThatCouldNotBeProcessed);
}
}
printSummary(imagesThatCouldNotBeProcessed, numberOfImagesToProcess);
}

protected void printSummary(List<ProcessingErrors> imagesThatCouldNotBeProcessed, AtomicInteger numberOfImagesToProcess) {
AtomicInteger numberOfImagesFailedToProcess = new AtomicInteger(imagesThatCouldNotBeProcessed.size());
AtomicInteger numberOfImagesSuccessfullyProcessed = new AtomicInteger(numberOfImagesToProcess.get() - numberOfImagesFailedToProcess.get());
log.info("The total number of images to process from image tag store: {}", numberOfImagesToProcess.get());
log.info("The total number of images that were successfully processed: {}", numberOfImagesSuccessfullyProcessed.get());
if (numberOfImagesFailedToProcess.get() > 0) {
log.warn("The total number of images that failed to be processed: {}. The following list shows the images that could not be processed.", numberOfImagesFailedToProcess.get());
imagesThatCouldNotBeProcessed.forEach(imageThatCouldNotBeProcessed -> {
if (imageThatCouldNotBeProcessed.getFailure().isPresent()) {
log.warn("Image: {}, Exception: {}", imageThatCouldNotBeProcessed.getImageNameAndTag(), imageThatCouldNotBeProcessed.getFailure());
} else {
log.warn("Image: {}, Exception: {}", imageThatCouldNotBeProcessed.getImageNameAndTag(), "Failure reason not known.");
}
}
);
}
}

protected void processErrors(String image, String tag, Exception e, List<ProcessingErrors> imagesThatCouldNotBeProcessed) {
String imageNameAndTag = image + ":" + tag;
ProcessingErrors processingErrors = new ProcessingErrors(imageNameAndTag, Optional.of(e));
imagesThatCouldNotBeProcessed.add(processingErrors);
}

protected void loadDockerfileGithubUtil(DockerfileGitHubUtil _dockerfileGitHubUtil) {
Expand Down
22 changes: 11 additions & 11 deletions ...update/src/main/java/com/salesforce/dockerfileimageupdate/utils/DockerfileGitHubUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,18 +119,18 @@ public Optional<List<PagedSearchIterable<GHContent>>> findFilesWithImage(
if (totalCount > gitApiSearchLimit
&& orgsToIncludeOrExclude.size() == 1
&& orgsToIncludeOrExclude
.entrySet()
.stream()
.findFirst()
.get()
.getKey() != null
.entrySet()
.stream()
.findFirst()
.get()
.getKey() != null
&& orgsToIncludeOrExclude
.entrySet()
.stream()
.findFirst()
.get()
.getValue()
) {
.entrySet()
.stream()
.findFirst()
.get()
.getValue()
) {
String orgName = orgsToIncludeOrExclude
.entrySet()
.stream()
Expand Down
21 changes: 21 additions & 0 deletions ...age-update/src/main/java/com/salesforce/dockerfileimageupdate/utils/ProcessingErrors.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package com.salesforce.dockerfileimageupdate.utils;

import java.util.Optional;

public class ProcessingErrors {
private final String imageNameAndTag;
private final Optional<Exception> failure;

public ProcessingErrors(String imageNameAndTag, Optional<Exception> failure) {
this.imageNameAndTag = imageNameAndTag;
this.failure = failure;
}

public String getImageNameAndTag() {
return imageNameAndTag;
}

public Optional<Exception> getFailure() {
return failure;
}
}
190 changes: 188 additions & 2 deletions ...e-update/src/test/java/com/salesforce/dockerfileimageupdate/subcommands/impl/AllTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
import com.salesforce.dockerfileimageupdate.storage.*;
import com.salesforce.dockerfileimageupdate.utils.*;

import java.io.IOException;
import java.util.*;
import java.util.concurrent.atomic.AtomicInteger;

import net.sourceforge.argparse4j.inf.Namespace;
import org.kohsuke.github.*;
Expand All @@ -27,7 +29,7 @@
import static org.testng.Assert.assertEquals;

/**
* Created by minho.park on 7/19/16.
* Created by avimanyum on 01/31/22.
*/
public class AllTest {
@Test
Expand Down Expand Up @@ -73,10 +75,194 @@ public void testAllCommandSuccessful() throws Exception {
verify(all, times(1)).getPullRequests();
verify(pullRequests, times(1)).prepareToCreate(ns, pullRequestSender,
contentsWithImage, gitForkBranch, dockerfileGitHubUtil);
verify(all, times(0)).processErrors(anyString(), anyString(), any(), anyList());
verify(all, times(1)).printSummary(anyList(), any());
}

@Test
public void testGetCommon(){
public void testAllCommandSkipsSendingPRsIfSearchReturnsEmpty() throws Exception {
Map<String, Object> nsMap = ImmutableMap.of(Constants.IMG,
"image", Constants.TAG,
"tag", Constants.STORE,
"store");
Namespace ns = new Namespace(nsMap);
All all = spy(new All());
DockerfileGitHubUtil dockerfileGitHubUtil = mock(DockerfileGitHubUtil.class);
GitHubJsonStore gitHubJsonStore = mock(GitHubJsonStore.class);
GitHubPullRequestSender pullRequestSender = mock(GitHubPullRequestSender.class);
GitForkBranch gitForkBranch = mock(GitForkBranch.class);
PullRequests pullRequests = mock(PullRequests.class);
Set<Map.Entry<String, JsonElement>> imageToTagStore = mock(Set.class);
when(dockerfileGitHubUtil.getGitHubJsonStore(anyString())).thenReturn(gitHubJsonStore);
when(gitHubJsonStore.parseStoreToImagesMap(dockerfileGitHubUtil, "store")).thenReturn(imageToTagStore);

Map.Entry<String, JsonElement> imageToTag = mock(Map.Entry.class);
Iterator<Map.Entry<String, JsonElement>> imageToTagStoreIterator = mock(Iterator.class);
when(imageToTagStoreIterator.next()).thenReturn(imageToTag);
when(imageToTagStoreIterator.hasNext()).thenReturn(true, false);
when(imageToTagStore.iterator()).thenReturn(imageToTagStoreIterator);
JsonElement jsonElement = mock(JsonElement.class);
when(imageToTag.getKey()).thenReturn("image1");
when(imageToTag.getValue()).thenReturn(jsonElement);
when(jsonElement.getAsString()).thenReturn("tag1");
when(all.getPullRequestSender(dockerfileGitHubUtil, ns)).thenReturn(pullRequestSender);
when(all.getGitForkBranch("image1", "tag1", ns)).thenReturn(gitForkBranch);
when(all.getPullRequests()).thenReturn(pullRequests);
PagedSearchIterable<GHContent> contentsWithImage = mock(PagedSearchIterable.class);
Optional<List<PagedSearchIterable<GHContent>>> optionalContentsWithImageList = Optional.empty();
doNothing().when(pullRequests).prepareToCreate(ns, pullRequestSender,
contentsWithImage, gitForkBranch, dockerfileGitHubUtil);
when(dockerfileGitHubUtil.findFilesWithImage(anyString(), anyMap(), anyInt())).thenReturn(optionalContentsWithImageList);


all.execute(ns, dockerfileGitHubUtil);
verify(all, times(1)).getGitForkBranch(anyString(), anyString(), any());
verify(all, times(1)).getPullRequestSender(dockerfileGitHubUtil, ns);
verify(all, times(1)).getPullRequests();
verify(pullRequests, times(0)).prepareToCreate(ns, pullRequestSender,
contentsWithImage, gitForkBranch, dockerfileGitHubUtil);
verify(all, times(0)).processErrors(anyString(), anyString(), any(), anyList());
verify(all, times(1)).printSummary(anyList(), any());
}

@Test
public void testAllCommandSkipsSendingPRsIfSearchRaisesException() throws Exception {
Map<String, Object> nsMap = ImmutableMap.of(Constants.IMG,
"image", Constants.TAG,
"tag", Constants.STORE,
"store");
Namespace ns = new Namespace(nsMap);
All all = spy(new All());
DockerfileGitHubUtil dockerfileGitHubUtil = mock(DockerfileGitHubUtil.class);
GitHubJsonStore gitHubJsonStore = mock(GitHubJsonStore.class);
GitHubPullRequestSender pullRequestSender = mock(GitHubPullRequestSender.class);
GitForkBranch gitForkBranch = mock(GitForkBranch.class);
PullRequests pullRequests = mock(PullRequests.class);
Set<Map.Entry<String, JsonElement>> imageToTagStore = mock(Set.class);
when(dockerfileGitHubUtil.getGitHubJsonStore(anyString())).thenReturn(gitHubJsonStore);
when(gitHubJsonStore.parseStoreToImagesMap(dockerfileGitHubUtil, "store")).thenReturn(imageToTagStore);

Map.Entry<String, JsonElement> imageToTag = mock(Map.Entry.class);
Iterator<Map.Entry<String, JsonElement>> imageToTagStoreIterator = mock(Iterator.class);
when(imageToTagStoreIterator.next()).thenReturn(imageToTag);
when(imageToTagStoreIterator.hasNext()).thenReturn(true, false);
when(imageToTagStore.iterator()).thenReturn(imageToTagStoreIterator);
JsonElement jsonElement = mock(JsonElement.class);
when(imageToTag.getKey()).thenReturn("image1");
when(imageToTag.getValue()).thenReturn(jsonElement);
when(jsonElement.getAsString()).thenReturn("tag1");
when(all.getPullRequestSender(dockerfileGitHubUtil, ns)).thenReturn(pullRequestSender);
when(all.getGitForkBranch("image1", "tag1", ns)).thenReturn(gitForkBranch);
when(all.getPullRequests()).thenReturn(pullRequests);
PagedSearchIterable<GHContent> contentsWithImage = mock(PagedSearchIterable.class);
doNothing().when(pullRequests).prepareToCreate(ns, pullRequestSender,
contentsWithImage, gitForkBranch, dockerfileGitHubUtil);
when(dockerfileGitHubUtil.findFilesWithImage(anyString(), anyMap(), anyInt())).thenThrow(new GHException("some exception"));


all.execute(ns, dockerfileGitHubUtil);
verify(all, times(1)).getGitForkBranch(anyString(), anyString(), any());
verify(all, times(1)).getPullRequestSender(dockerfileGitHubUtil, ns);
verify(all, times(1)).getPullRequests();
verify(pullRequests, times(0)).prepareToCreate(ns, pullRequestSender,
contentsWithImage, gitForkBranch, dockerfileGitHubUtil);
verify(all, times(1)).processErrors(anyString(), anyString(), any(), anyList());
verify(all, times(1)).printSummary(anyList(), any());
}

@Test
public void testAllCommandSkipsSendingPRsIfPRCreationRaisesException() throws Exception {
Map<String, Object> nsMap = ImmutableMap.of(Constants.IMG,
"image", Constants.TAG,
"tag", Constants.STORE,
"store");
Namespace ns = new Namespace(nsMap);
All all = spy(new All());
DockerfileGitHubUtil dockerfileGitHubUtil = mock(DockerfileGitHubUtil.class);
GitHubJsonStore gitHubJsonStore = mock(GitHubJsonStore.class);
GitHubPullRequestSender pullRequestSender = mock(GitHubPullRequestSender.class);
GitForkBranch gitForkBranch = mock(GitForkBranch.class);
PullRequests pullRequests = mock(PullRequests.class);
Set<Map.Entry<String, JsonElement>> imageToTagStore = mock(Set.class);
when(dockerfileGitHubUtil.getGitHubJsonStore(anyString())).thenReturn(gitHubJsonStore);
when(gitHubJsonStore.parseStoreToImagesMap(dockerfileGitHubUtil, "store")).thenReturn(imageToTagStore);

Map.Entry<String, JsonElement> imageToTag = mock(Map.Entry.class);
Iterator<Map.Entry<String, JsonElement>> imageToTagStoreIterator = mock(Iterator.class);
when(imageToTagStoreIterator.next()).thenReturn(imageToTag);
when(imageToTagStoreIterator.hasNext()).thenReturn(true, false);
when(imageToTagStore.iterator()).thenReturn(imageToTagStoreIterator);
JsonElement jsonElement = mock(JsonElement.class);
when(imageToTag.getKey()).thenReturn("image1");
when(imageToTag.getValue()).thenReturn(jsonElement);
when(jsonElement.getAsString()).thenReturn("tag1");
when(all.getPullRequestSender(dockerfileGitHubUtil, ns)).thenReturn(pullRequestSender);
when(all.getGitForkBranch("image1", "tag1", ns)).thenReturn(gitForkBranch);
when(all.getPullRequests()).thenReturn(pullRequests);
PagedSearchIterable<GHContent> contentsWithImage = mock(PagedSearchIterable.class);
List<PagedSearchIterable<GHContent>> contentsWithImageList = Collections.singletonList(contentsWithImage);
Optional<List<PagedSearchIterable<GHContent>>> optionalContentsWithImageList = Optional.of(contentsWithImageList);
doThrow(new IOException()).when(pullRequests).prepareToCreate(ns, pullRequestSender,
contentsWithImage, gitForkBranch, dockerfileGitHubUtil);
when(dockerfileGitHubUtil.findFilesWithImage(anyString(), anyMap(), anyInt())).thenReturn(optionalContentsWithImageList);


all.execute(ns, dockerfileGitHubUtil);
verify(all, times(1)).getGitForkBranch(anyString(), anyString(), any());
verify(all, times(1)).getPullRequestSender(dockerfileGitHubUtil, ns);
verify(all, times(1)).getPullRequests();
verify(pullRequests, times(1)).prepareToCreate(ns, pullRequestSender,
contentsWithImage, gitForkBranch, dockerfileGitHubUtil);
verify(all, times(1)).processErrors(anyString(), anyString(), any(), anyList());
verify(all, times(1)).printSummary(anyList(), any());
}

@Test
public void testProcessErrors() {
All all = spy(new All());
String image = "image1";
String tag = "tag";
Exception e = new Exception();
List<ProcessingErrors> processingErrorsList = new ArrayList<>();
AtomicInteger numberOfImagesFailedToProcess = new AtomicInteger(0);

all.processErrors(image, tag, e, processingErrorsList);

assertEquals(processingErrorsList.size(), 1);
}

@Test
public void testPrintSummaryWhenImagesWereMissed() {
ProcessingErrors processingErrors = mock(ProcessingErrors.class);
All all = spy(new All());
List<ProcessingErrors> processingErrorsList = Collections.singletonList(processingErrors);
AtomicInteger numberOfImagesToProcess = new AtomicInteger(2);
Exception failure = mock(Exception.class);
when(processingErrors.getFailure()).thenReturn(Optional.of(failure));

all.printSummary(processingErrorsList, numberOfImagesToProcess);

verify(processingErrors, times(1)).getImageNameAndTag();
verify(processingErrors, times(2)).getFailure();
assertEquals(numberOfImagesToProcess.get(), 2);
}

@Test
public void testPrintSummaryWhenAllImagesWereSuccessfullyProcessed() {
ProcessingErrors processingErrors = mock(ProcessingErrors.class);
All all = spy(new All());
List<ProcessingErrors> processingErrorsList = Collections.emptyList();
AtomicInteger numberOfImagesToProcess = new AtomicInteger(2);

all.printSummary(processingErrorsList, numberOfImagesToProcess);

verify(processingErrors, times(0)).getImageNameAndTag();
verify(processingErrors, times(0)).getFailure();
assertEquals(numberOfImagesToProcess.get(), 2);
}

@Test
public void testGetCommon() {
All all = new All();
PullRequests pullRequests = new PullRequests();
assertEquals(pullRequests.getClass(), all.getPullRequests().getClass());
Expand Down

0 comments on commit e18ae3e

Please sign in to comment.