From 3a58a0d2743f7d1e3f099e6eb2f2074591f30d7b Mon Sep 17 00:00:00 2001 From: samdgupi Date: Thu, 9 Nov 2023 12:16:59 +0530 Subject: [PATCH] Merge pull request #15385 from cdapio/multi-push-operation Multi push operation --- .../app/sourcecontrol/PullAppsOperation.java | 7 +- .../app/sourcecontrol/PushAppsOperation.java | 123 ++++++++++ .../PushAppsOperationFactory.java | 34 +++ .../app/sourcecontrol/PushAppsRequest.java | 76 ++++++ .../operation/OperationException.java | 5 + .../SourceControlManagementServiceTest.java | 11 +- .../sourcecontrol/PullAppsOperationTest.java | 10 +- .../sourcecontrol/PushAppsOperationTest.java | 228 ++++++++++++++++++ .../cdap/sourcecontrol/RepositoryManager.java | 154 ++++++------ .../InMemorySourceControlOperationRunner.java | 178 +++++++++++--- .../MultiPushAppOperationRequest.java | 62 +++++ .../RemoteSourceControlOperationRunner.java | 11 +- .../SourceControlOperationRunner.java | 18 +- .../sourcecontrol/RepositoryManagerTest.java | 121 ++++++++-- ...emorySourceControlOperationRunnerTest.java | 148 +++++++++--- 15 files changed, 994 insertions(+), 192 deletions(-) create mode 100644 cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/sourcecontrol/PushAppsOperation.java create mode 100644 cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/sourcecontrol/PushAppsOperationFactory.java create mode 100644 cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/sourcecontrol/PushAppsRequest.java create mode 100644 cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/sourcecontrol/PushAppsOperationTest.java create mode 100644 cdap-source-control/src/main/java/io/cdap/cdap/sourcecontrol/operationrunner/MultiPushAppOperationRequest.java diff --git a/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/sourcecontrol/PullAppsOperation.java b/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/sourcecontrol/PullAppsOperation.java index 394425128f96..d768f965bdcc 100644 --- a/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/sourcecontrol/PullAppsOperation.java +++ b/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/sourcecontrol/PullAppsOperation.java @@ -57,7 +57,7 @@ public class PullAppsOperation implements LongRunningOperation { /** * Only request is passed using AssistedInject. See {@link PullAppsOperationFactory} * - * @param request contains apps to push + * @param request contains apps to pull * @param runner runs git operations. The reason we do not use * {@link io.cdap.cdap.sourcecontrol.operationrunner.SourceControlOperationRunner} rather than * concrete implementation is because the git operations should always run inMemory. @@ -88,7 +88,7 @@ public ListenableFuture> run(LongRunningOperationContext ); // pull and deploy applications one at a time - scmOpRunner.pull(pullReq, response -> { + scmOpRunner.multiPull(pullReq, response -> { appTobeDeployed.set(new ApplicationReference(context.getRunId().getNamespace(), response.getApplicationName())); try { @@ -106,7 +106,8 @@ public ListenableFuture> run(LongRunningOperationContext "Failed to deploy applications", appTobeDeployed.get() != null ? ImmutableList.of( new OperationResourceScopedError(appTobeDeployed.get().toString(), e.getMessage())) - : Collections.emptyList() + : Collections.emptyList(), + e ); } diff --git a/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/sourcecontrol/PushAppsOperation.java b/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/sourcecontrol/PushAppsOperation.java new file mode 100644 index 000000000000..0f7d9eec8300 --- /dev/null +++ b/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/sourcecontrol/PushAppsOperation.java @@ -0,0 +1,123 @@ +/* + * Copyright © 2023 Cask Data, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ + +package io.cdap.cdap.internal.app.sourcecontrol; + +import com.google.common.util.concurrent.Futures; +import com.google.common.util.concurrent.ListenableFuture; +import com.google.inject.Inject; +import com.google.inject.assistedinject.Assisted; +import io.cdap.cdap.common.BadRequestException; +import io.cdap.cdap.common.NotFoundException; +import io.cdap.cdap.internal.operation.LongRunningOperation; +import io.cdap.cdap.internal.operation.LongRunningOperationContext; +import io.cdap.cdap.internal.operation.OperationException; +import io.cdap.cdap.proto.app.UpdateMultiSourceControlMetaReqeust; +import io.cdap.cdap.proto.app.UpdateSourceControlMetaRequest; +import io.cdap.cdap.proto.id.NamespaceId; +import io.cdap.cdap.proto.operation.OperationResource; +import io.cdap.cdap.proto.sourcecontrol.RepositoryConfig; +import io.cdap.cdap.sourcecontrol.ApplicationManager; +import io.cdap.cdap.sourcecontrol.NoChangesToPushException; +import io.cdap.cdap.sourcecontrol.SourceControlException; +import io.cdap.cdap.sourcecontrol.operationrunner.InMemorySourceControlOperationRunner; +import io.cdap.cdap.sourcecontrol.operationrunner.MultiPushAppOperationRequest; +import io.cdap.cdap.sourcecontrol.operationrunner.PushAppResponse; +import java.io.IOException; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; + +/** + * Defines operation for doing SCM Push for connected repositories. + **/ +public class PushAppsOperation implements LongRunningOperation { + + private final PushAppsRequest request; + + private final InMemorySourceControlOperationRunner scmOpRunner; + private final ApplicationManager applicationManager; + + /** + * Only request is passed using AssistedInject. See {@link PushAppsOperationFactory} + * + * @param request contains apps to push + * @param runner runs git operations. The reason we do not use + * {@link io.cdap.cdap.sourcecontrol.operationrunner.SourceControlOperationRunner} rather than + * concrete implementation is because the git operations should always run inMemory. + * @param applicationManager provides utilities to provide app-fabric exposed + * functionalities. + */ + @Inject + PushAppsOperation(@Assisted PushAppsRequest request, + InMemorySourceControlOperationRunner runner, + ApplicationManager applicationManager) { + this.request = request; + this.applicationManager = applicationManager; + this.scmOpRunner = runner; + } + + @Override + public ListenableFuture> run(LongRunningOperationContext context) + throws OperationException { + RepositoryConfig repositoryConfig = request.getConfig(); + NamespaceId namespaceId = context.getRunId().getNamespaceId(); + MultiPushAppOperationRequest pushReq = new MultiPushAppOperationRequest( + namespaceId, + repositoryConfig, + request.getApps(), + request.getCommitDetails() + ); + + Collection responses; + + try { + responses = scmOpRunner.multiPush(pushReq, applicationManager); + context.updateOperationResources(getResources(namespaceId, responses)); + } catch (SourceControlException | NoChangesToPushException e) { + throw new OperationException("Failed to push applications.", Collections.emptyList(), e); + } + + try { + // update git metadata for the pushed application + applicationManager.updateSourceControlMeta(namespaceId, getUpdateMetaRequest(responses)); + } catch (NotFoundException | BadRequestException | IOException | SourceControlException e) { + throw new OperationException("Failed to update git metadata.", Collections.emptySet(), e); + } + + // TODO(samik) Update this after along with the runner implementation + return Futures.immediateFuture(getResources(namespaceId, responses)); + } + + private UpdateMultiSourceControlMetaReqeust getUpdateMetaRequest( + Collection responses) { + List reqs = responses.stream() + .map(response -> new UpdateSourceControlMetaRequest( + response.getName(), response.getVersion(), response.getFileHash())) + .collect(Collectors.toList()); + return new UpdateMultiSourceControlMetaReqeust(reqs); + } + + private Set getResources(NamespaceId namespaceId, + Collection responses) { + return responses.stream() + .map(response -> new OperationResource( + namespaceId.app(response.getName(), response.getVersion()).toString())) + .collect(Collectors.toSet()); + } +} diff --git a/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/sourcecontrol/PushAppsOperationFactory.java b/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/sourcecontrol/PushAppsOperationFactory.java new file mode 100644 index 000000000000..6a3ce0f3e53d --- /dev/null +++ b/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/sourcecontrol/PushAppsOperationFactory.java @@ -0,0 +1,34 @@ +/* + * Copyright © 2023 Cask Data, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ + +package io.cdap.cdap.internal.app.sourcecontrol; + + +/** + * Factory interface for creating {@link PushAppsOperation}. + * This interface is for Guice assisted binding, hence there will be no concrete implementation of it. + */ +public interface PushAppsOperationFactory { + + /** + * Returns an implementation of {@link PushAppsOperation} that operates on the given {@link + * PushAppsRequest}. + * + * @param request contains list of apps to pull + * @return a new instance of {@link PushAppsOperation}. + */ + PushAppsOperation create(PushAppsRequest request); +} diff --git a/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/sourcecontrol/PushAppsRequest.java b/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/sourcecontrol/PushAppsRequest.java new file mode 100644 index 000000000000..65ffcb48a0a7 --- /dev/null +++ b/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/app/sourcecontrol/PushAppsRequest.java @@ -0,0 +1,76 @@ +/* + * Copyright © 2023 Cask Data, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ + +package io.cdap.cdap.internal.app.sourcecontrol; + +import com.google.common.base.Objects; +import io.cdap.cdap.proto.sourcecontrol.RepositoryConfig; +import io.cdap.cdap.sourcecontrol.CommitMeta; +import java.util.Set; + +/** + * Request type for {@link PullAppsOperation}. + */ +public class PushAppsRequest { + + private final Set apps; + private final RepositoryConfig config; + + private final CommitMeta commitDetails; + + /** + * Default Constructor. + * + * @param apps Set of apps to push. + */ + public PushAppsRequest(Set apps, RepositoryConfig config, CommitMeta commitDetails) { + this.apps = apps; + this.config = config; + this.commitDetails = commitDetails; + } + + public Set getApps() { + return apps; + } + + public RepositoryConfig getConfig() { + return config; + } + + public CommitMeta getCommitDetails() { + return commitDetails; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + + PushAppsRequest that = (PushAppsRequest) o; + return Objects.equal(this.getApps(), that.getApps()) + && Objects.equal(this.getConfig(), that.getConfig()) + && Objects.equal(this.getCommitDetails(), that.getCommitDetails()); + } + + @Override + public int hashCode() { + return Objects.hashCode(getApps(), getConfig(), getCommitDetails()); + } +} diff --git a/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/operation/OperationException.java b/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/operation/OperationException.java index 283b68df1202..f5e3f1c18dd5 100644 --- a/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/operation/OperationException.java +++ b/cdap-app-fabric/src/main/java/io/cdap/cdap/internal/operation/OperationException.java @@ -32,6 +32,11 @@ public OperationException(String message, Collection errors, Throwable cause) { + super(message, cause); + this.errors = errors; + } + public OperationError toOperationError() { return new OperationError(getMessage(), errors); } diff --git a/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/services/SourceControlManagementServiceTest.java b/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/services/SourceControlManagementServiceTest.java index c560e66eb13f..0ce6f99b9b6c 100644 --- a/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/services/SourceControlManagementServiceTest.java +++ b/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/services/SourceControlManagementServiceTest.java @@ -45,11 +45,13 @@ import io.cdap.cdap.proto.sourcecontrol.SourceControlMeta; import io.cdap.cdap.security.impersonation.CurrentUGIProvider; import io.cdap.cdap.security.impersonation.UGIProvider; +import io.cdap.cdap.sourcecontrol.ApplicationManager; import io.cdap.cdap.sourcecontrol.AuthenticationConfigException; import io.cdap.cdap.sourcecontrol.NoChangesToPullException; import io.cdap.cdap.sourcecontrol.NoChangesToPushException; import io.cdap.cdap.sourcecontrol.SourceControlException; import io.cdap.cdap.sourcecontrol.operationrunner.MultiPullAppOperationRequest; +import io.cdap.cdap.sourcecontrol.operationrunner.MultiPushAppOperationRequest; import io.cdap.cdap.sourcecontrol.operationrunner.NamespaceRepository; import io.cdap.cdap.sourcecontrol.operationrunner.PullAppOperationRequest; import io.cdap.cdap.sourcecontrol.operationrunner.PullAppResponse; @@ -495,6 +497,13 @@ public PushAppResponse push(PushAppOperationRequest pushAppOperationRequest) return null; } + @Override + public List multiPush(MultiPushAppOperationRequest pushRequest, + ApplicationManager appManager) + throws NoChangesToPushException, AuthenticationConfigException { + return null; + } + @Override public PullAppResponse pull(PullAppOperationRequest pullRequest) throws NotFoundException, AuthenticationConfigException { @@ -502,7 +511,7 @@ public PullAppResponse pull(PullAppOperationRequest pullRequest) } @Override - public void pull(MultiPullAppOperationRequest pullRequest, Consumer> consumer) + public void multiPull(MultiPullAppOperationRequest pullRequest, Consumer> consumer) throws NotFoundException, AuthenticationConfigException { } diff --git a/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/sourcecontrol/PullAppsOperationTest.java b/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/sourcecontrol/PullAppsOperationTest.java index 168e7e66daf5..db6eb053087e 100644 --- a/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/sourcecontrol/PullAppsOperationTest.java +++ b/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/sourcecontrol/PullAppsOperationTest.java @@ -39,7 +39,6 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.Paths; import java.util.Collection; import java.util.HashSet; import java.util.Set; @@ -75,10 +74,11 @@ public void setUp() throws Exception { RepositoryManager mockRepositoryManager = Mockito.mock(RepositoryManager.class); Mockito.doReturn(mockRepositoryManager).when(mockRepositoryManagerFactory) .create(Mockito.any(), Mockito.any()); - Mockito.doReturn(Paths.get("")).when(mockRepositoryManager).getRepositoryRoot(); - Mockito.doReturn(repositoryBase.getRoot().toPath()).when(mockRepositoryManager) - .getRepositoryRoot(); - Mockito.doReturn(repositoryBase.getRoot().toPath()).when(mockRepositoryManager).getBasePath(); + + Path rootPath = repositoryBase.getRoot().toPath(); + Mockito.doReturn(rootPath).when(mockRepositoryManager).getRepositoryRoot(); + Mockito.doReturn(rootPath).when(mockRepositoryManager).getBasePath(); + Mockito.doReturn("testHash") .when(mockRepositoryManager) .getFileHash(Mockito.any(), Mockito.any()); diff --git a/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/sourcecontrol/PushAppsOperationTest.java b/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/sourcecontrol/PushAppsOperationTest.java new file mode 100644 index 000000000000..78aeee93bf34 --- /dev/null +++ b/cdap-app-fabric/src/test/java/io/cdap/cdap/internal/app/sourcecontrol/PushAppsOperationTest.java @@ -0,0 +1,228 @@ +/* + * Copyright © 2023 Cask Data, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ + +package io.cdap.cdap.internal.app.sourcecontrol; + + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import com.google.gson.Gson; +import com.google.inject.AbstractModule; +import com.google.inject.Injector; +import io.cdap.cdap.common.NotFoundException; +import io.cdap.cdap.common.conf.CConfiguration; +import io.cdap.cdap.internal.AppFabricTestHelper; +import io.cdap.cdap.internal.operation.LongRunningOperationContext; +import io.cdap.cdap.internal.operation.OperationException; +import io.cdap.cdap.proto.ApplicationDetail; +import io.cdap.cdap.proto.id.ApplicationId; +import io.cdap.cdap.proto.id.ApplicationReference; +import io.cdap.cdap.proto.id.NamespaceId; +import io.cdap.cdap.proto.id.OperationRunId; +import io.cdap.cdap.proto.operation.OperationResource; +import io.cdap.cdap.proto.sourcecontrol.AuthConfig; +import io.cdap.cdap.proto.sourcecontrol.AuthType; +import io.cdap.cdap.proto.sourcecontrol.PatConfig; +import io.cdap.cdap.proto.sourcecontrol.Provider; +import io.cdap.cdap.proto.sourcecontrol.RepositoryConfig; +import io.cdap.cdap.sourcecontrol.ApplicationManager; +import io.cdap.cdap.sourcecontrol.CommitMeta; +import io.cdap.cdap.sourcecontrol.RepositoryManager; +import io.cdap.cdap.sourcecontrol.RepositoryManagerFactory; +import io.cdap.cdap.sourcecontrol.operationrunner.InMemorySourceControlOperationRunner; +import io.cdap.cdap.sourcecontrol.operationrunner.PushAppResponse; +import java.nio.file.Path; +import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashSet; +import java.util.Set; +import java.util.function.BiFunction; +import java.util.stream.Collectors; +import org.junit.Assert; +import org.junit.Before; +import org.junit.ClassRule; +import org.junit.Test; +import org.junit.rules.TemporaryFolder; +import org.mockito.Mockito; + +public class PushAppsOperationTest { + + private static final ApplicationDetail testApp1Details = new ApplicationDetail( + "testApp", "v1", "description1", null, null, "conf1", new ArrayList<>(), + new ArrayList<>(), new ArrayList<>(), null, null); + private static final ApplicationDetail testApp2Details = new ApplicationDetail( + "testApp2", "v2", "description2", null, null, "conf2", new ArrayList<>(), + new ArrayList<>(), new ArrayList<>(), null, null); + + private static final Gson GSON = new Gson(); + + private final PushAppsRequest req = new PushAppsRequest( + ImmutableSet.of(testApp1Details.getName(), testApp2Details.getName()), + new RepositoryConfig.Builder() + .setProvider(Provider.GITHUB) + .setLink("test") + .setDefaultBranch("") + .setAuth(new AuthConfig(AuthType.PAT, new PatConfig("test", "test"))) + .build(), + new CommitMeta("", "", 0, "test_commit") + ); + + private InMemorySourceControlOperationRunner opRunner; + private LongRunningOperationContext context; + + private final Path path1 = Paths.get(testApp1Details.getName() + ".json"); + private final Path path2 = Paths.get(testApp2Details.getName() + ".json"); + + private static final RepositoryManager mockRepositoryManager = Mockito.mock( + RepositoryManager.class); + + + @ClassRule + public static TemporaryFolder repositoryBase = new TemporaryFolder(); + + @Before + public void setUp() throws Exception { + RepositoryManagerFactory mockRepositoryManagerFactory = Mockito.mock( + RepositoryManagerFactory.class); + Mockito.doReturn(mockRepositoryManager).when(mockRepositoryManagerFactory) + .create(Mockito.any(), Mockito.any()); + Mockito.doNothing().when(mockRepositoryManager).close(); + + Path rootPath = repositoryBase.getRoot().toPath(); + Mockito.doReturn(rootPath).when(mockRepositoryManager).getRepositoryRoot(); + Mockito.doReturn(rootPath).when(mockRepositoryManager).getBasePath(); + Mockito.doReturn(path1.getFileName()).when(mockRepositoryManager) + .getFileRelativePath(path1.getFileName().toString()); + Mockito.doReturn(path2.getFileName()).when(mockRepositoryManager) + .getFileRelativePath(path2.getFileName().toString()); + + this.context = Mockito.mock(LongRunningOperationContext.class); + Mockito.doReturn(new OperationRunId(NamespaceId.DEFAULT.getNamespace(), "id")) + .when(this.context).getRunId(); + + Injector injector = AppFabricTestHelper.getInjector(CConfiguration.create(), + new AbstractModule() { + @Override + protected void configure() { + bind(RepositoryManagerFactory.class).toInstance(mockRepositoryManagerFactory); + } + }); + + this.opRunner = injector.getInstance(InMemorySourceControlOperationRunner.class); + } + + @Test + public void testRunSuccess() throws Exception { + ApplicationManager mockManager = Mockito.mock(ApplicationManager.class); + PushAppsOperation operation = new PushAppsOperation(req, opRunner, mockManager); + + Mockito.doReturn(testApp1Details).when(mockManager).get( + new ApplicationReference(NamespaceId.DEFAULT.getNamespace(), testApp1Details.getName())); + + Mockito.doReturn(testApp2Details).when(mockManager).get( + new ApplicationReference(NamespaceId.DEFAULT.getNamespace(), testApp2Details.getName())); + + PushAppResponse mockTestApp1Response = new PushAppResponse( + testApp1Details.getName(), testApp1Details.getAppVersion(), "file-hash1"); + PushAppResponse mockTestApp2Response = new PushAppResponse( + testApp2Details.getName(), testApp2Details.getAppVersion(), "file-hash2"); + + Mockito.doReturn(ImmutableList.of(mockTestApp1Response, mockTestApp2Response)) + .when(mockRepositoryManager) + .commitAndPush(Mockito.any(CommitMeta.class), Mockito.anyCollection(), Mockito.any(BiFunction.class)); + + Set gotResources = operation.run(context).get(); + + verifyCreatedResources(gotResources); + } + + @Test(expected = OperationException.class) + public void testRunFailedAtFirstApp() throws Exception { + ApplicationManager mockManager = Mockito.mock(ApplicationManager.class); + LongRunningOperationContext mockContext = Mockito.mock(LongRunningOperationContext.class); + PushAppsOperation operation = new PushAppsOperation(req, opRunner, mockManager); + + Mockito.doThrow(new NotFoundException("")).when(mockManager) + .get(Mockito.any()); + + operation.run(context).get(); + } + + @Test(expected = OperationException.class) + public void testRunFailedAtSecondApp() throws Exception { + ApplicationManager mockManager = Mockito.mock(ApplicationManager.class); + LongRunningOperationContext mockContext = Mockito.mock(LongRunningOperationContext.class); + PushAppsOperation operation = new PushAppsOperation(req, opRunner, mockManager); + + Mockito.doReturn(testApp1Details).doThrow(new NotFoundException("")).when(mockManager) + .get(Mockito.any()); + + operation.run(context).get(); + } + + @Test + public void testRunFailedWhenMarkingLatest() throws Exception { + ApplicationManager mockManager = Mockito.mock(ApplicationManager.class); + PushAppsOperation operation = new PushAppsOperation(req, opRunner, mockManager); + + Mockito.doReturn(testApp1Details).when(mockManager).get( + new ApplicationReference(NamespaceId.DEFAULT.getNamespace(), testApp1Details.getName())); + + Mockito.doReturn(testApp2Details).when(mockManager).get( + new ApplicationReference(NamespaceId.DEFAULT.getNamespace(), testApp2Details.getName())); + + PushAppResponse mockTestApp1Response = new PushAppResponse( + testApp1Details.getName(), testApp1Details.getAppVersion(), "file-hash1"); + PushAppResponse mockTestApp2Response = new PushAppResponse( + testApp2Details.getName(), testApp2Details.getAppVersion(), "file-hash2"); + + Mockito.doReturn(ImmutableList.of(mockTestApp1Response, mockTestApp2Response)) + .when(mockRepositoryManager) + .commitAndPush(Mockito.any(CommitMeta.class), Mockito.anyCollection(), Mockito.any(BiFunction.class)); + + Mockito.doThrow(new NotFoundException("")).when(mockManager) + .updateSourceControlMeta(Mockito.any(), Mockito.any()); + + Set gotResources = new HashSet<>(); + Mockito.doAnswer(i -> { + gotResources.addAll( + (Collection) i.getArguments()[0]); + return null; + }).when(context).updateOperationResources(Mockito.any()); + try { + operation.run(context).get(); + Assert.fail(); + } catch (OperationException e) { + // expected + } + + verifyCreatedResources(gotResources); + } + + private void verifyCreatedResources(Set gotResources) { + Set expectedAppIds = ImmutableSet.of( + new ApplicationId(NamespaceId.DEFAULT.getNamespace(), testApp1Details.getName(), + testApp1Details.getAppVersion()), + new ApplicationId(NamespaceId.DEFAULT.getNamespace(), testApp2Details.getName(), + testApp2Details.getAppVersion()) + ); + Set createdAppIds = gotResources.stream() + .map(r -> ApplicationId.fromString(r.getResourceUri())).collect(Collectors.toSet()); + Assert.assertEquals(expectedAppIds, createdAppIds); + } + +} diff --git a/cdap-source-control/src/main/java/io/cdap/cdap/sourcecontrol/RepositoryManager.java b/cdap-source-control/src/main/java/io/cdap/cdap/sourcecontrol/RepositoryManager.java index 4d4bfe7cdb71..06fd4f55f835 100644 --- a/cdap-source-control/src/main/java/io/cdap/cdap/sourcecontrol/RepositoryManager.java +++ b/cdap-source-control/src/main/java/io/cdap/cdap/sourcecontrol/RepositoryManager.java @@ -39,12 +39,15 @@ import java.nio.file.LinkOption; import java.nio.file.Path; import java.nio.file.Paths; +import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.Date; import java.util.List; import java.util.Map; import java.util.UUID; import java.util.concurrent.TimeUnit; +import java.util.function.BiFunction; import java.util.function.Supplier; import java.util.stream.Stream; import javax.annotation.Nullable; @@ -58,6 +61,8 @@ import org.eclipse.jgit.api.TransportCommand; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.api.errors.TransportException; +import org.eclipse.jgit.dircache.DirCache; +import org.eclipse.jgit.dircache.DirCacheEntry; import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository; import org.eclipse.jgit.lib.Constants; @@ -75,8 +80,8 @@ import org.slf4j.LoggerFactory; /** - * A git repository manager that is responsible for handling interfacing with - * git. It provides version control operations. This is not thread safe. + * A git repository manager that is responsible for handling interfacing with git. It provides + * version control operations. This is not thread safe. */ public class RepositoryManager implements AutoCloseable { @@ -99,8 +104,8 @@ public class RepositoryManager implements AutoCloseable { * @param cConf CDAP configurations. * @param namespace The CDAP namespace to whcih the Git repository is linked. * @param repoConfig The repository configuration for the CDAP namespace. - * @param metricsCollectionService A metrics service to emit metrics related - * to SCM operations. + * @param metricsCollectionService A metrics service to emit metrics related to SCM + * operations. */ public RepositoryManager(final SecureStore secureStore, final CConfiguration cConf, final NamespaceId namespace, @@ -123,9 +128,8 @@ public RepositoryManager(final SecureStore secureStore, } /** - * Returns the base path in the git repo to store CDAP applications. If an - * optional Path prefix is provided in the repository configuration, the - * returned path includes it. + * Returns the base path in the git repo to store CDAP applications. If an optional Path prefix is + * provided in the repository configuration, the returned path includes it. * * @return the path for the repository base directory. */ @@ -140,8 +144,7 @@ public Path getBasePath() { } /** - * Gets the relative path of a file in git repository based on the user - * configured path prefix. + * Gets the relative path of a file in git repository based on the user configured path prefix. * * @param fileName The filename * @return the relative {@link Path} @@ -158,11 +161,10 @@ public Path getFileRelativePath(final String fileName) { /** * Validates the provided configuration. * - * @param secureStore A secure store for fetching credentials if - * required. + * @param secureStore A secure store for fetching credentials if required. * @param sourceControlConfig Configuration for source control operations. - * @throws RepositoryConfigValidationException when provided repository - * configuration is invalid. + * @throws RepositoryConfigValidationException when provided repository configuration is + * invalid. */ public static void validateConfig(final SecureStore secureStore, final SourceControlConfig sourceControlConfig) @@ -211,24 +213,24 @@ public static void validateConfig(final SecureStore secureStore, } /** - * Commits and pushes the changes of a given file under the repository root - * path. + * Commits and pushes the changes of given files under the repository root path. + * Processes the hashes to a list of given type S * - * @param commitMeta Details for the commit including author, committer and - * commit message - * @param fileChanged The relative path to repository root where the file is - * updated - * @return the hash of the written file. - * @throws GitAPIException when the underlying git commands fail - * @throws NoChangesToPushException when there's no file changes for the - * commit - * @throws GitOperationException when failed to get filehash due to IOException - * or the {@link PushResult} status is not OK - * @throws SourceControlException when failed to get the fileHash before - * push + * @param Type of elements of the output collection + * @param Type of elements of the input collection, must implement {@link Supplier} + * @param commitMeta Details for the commit including author, committer and commit message + * @param filesChanged Supplies relative paths to repository root of the updated files + * @param hashConsumer a {@link BiFunction} that takes a value of the input collection element type (S), + * a filehash and returns a value of the output collection element type (T) + * @return a {@link Collection} of elements of type T + * @throws GitAPIException when the underlying git commands fail + * @throws NoChangesToPushException when there's no file changes for the commit + * @throws GitOperationException when failed to get filehash due to IOException or the + * {@link PushResult} status is not OK + * @throws SourceControlException when failed to get the fileHash before push */ - public String commitAndPush(final CommitMeta commitMeta, - final Path fileChanged) + public > Collection commitAndPush(CommitMeta commitMeta, Collection filesChanged, + BiFunction hashConsumer) throws NoChangesToPushException, GitAPIException { validateInitialized(); final Stopwatch stopwatch = new Stopwatch().start(); @@ -240,25 +242,21 @@ public String commitAndPush(final CommitMeta commitMeta, "No changes have been made for the applications to push."); } - git.add().addFilepattern(fileChanged.toString()).call(); - - RevCommit commit = getCommitCommand(commitMeta).call(); - - String fileHash; - try { - fileHash = getFileHash(fileChanged, commit); - } catch (IOException e) { - throw new GitOperationException( - String.format("Failed to get fileHash for %s", fileChanged), - e); + List output = new ArrayList<>(filesChanged.size()); + for (S fileChanged : filesChanged) { + DirCache cache = git.add().addFilepattern(fileChanged.get().toString()).call(); + DirCacheEntry entry = cache.getEntry(fileChanged.get().toString()); + // This should not happen usually as we have just added the entry + if (entry == null) { + throw new SourceControlException( + String.format( + "Failed to get fileHash for %s because some paths are not " + + "found in Git tree", fileChanged.get())); + } + output.add(hashConsumer.apply(fileChanged, entry.getObjectId().name())); } - if (fileHash == null) { - throw new SourceControlException( - String.format( - "Failed to get fileHash for %s, because the path is not " - + "found in Git tree", fileChanged)); - } + getCommitCommand(commitMeta).call(); PushCommand pushCommand = createCommand(git::push, sourceControlConfig, credentialsProvider); @@ -269,7 +267,7 @@ public String commitAndPush(final CommitMeta commitMeta, if (rru.getStatus() != RemoteRefUpdate.Status.OK && rru.getStatus() != RemoteRefUpdate.Status.UP_TO_DATE) { throw new GitOperationException( - String.format("Push failed for %s: %s", fileChanged, + String.format("Push failed for %s: %s", filesChanged, rru.getStatus())); } } @@ -278,7 +276,7 @@ public String commitAndPush(final CommitMeta commitMeta, metricsContext.event( SourceControlManagement.COMMIT_PUSH_LATENCY_MILLIS, stopwatch.stop().elapsedTime(TimeUnit.MILLISECONDS)); - return fileHash; + return output; } private CommitCommand getCommitCommand(final CommitMeta commitMeta) { @@ -294,16 +292,14 @@ private CommitCommand getCommitCommand(final CommitMeta commitMeta) { } /** - * Initializes the Git repository by cloning remote. If the repository is - * already cloned, it resets the git repository to the state in the remote. - * All local changes will be lost. + * Initializes the Git repository by cloning remote. If the repository is already cloned, it + * resets the git repository to the state in the remote. All local changes will be lost. * * @return the commit ID of the present HEAD. - * @throws GitAPIException when a Git operation fails. - * @throws IOException when file or network I/O fails. - * @throws AuthenticationConfigException when there is a failure while - * fetching authentication credentials - * for Git. + * @throws GitAPIException when a Git operation fails. + * @throws IOException when file or network I/O fails. + * @throws AuthenticationConfigException when there is a failure while fetching authentication + * credentials for Git. */ public String cloneRemote() throws IOException, AuthenticationConfigException, GitAPIException { @@ -371,20 +367,19 @@ public Path getRepositoryRoot() { } /** - * Returns the Git Hash - * of the requested file path in the provided commit. It is the caller's - * responsibility to handle paths that refer to directories or symlinks. + * Returns the Git Hash of the requested + * file path in the provided commit. It is the caller's responsibility to handle paths that refer + * to directories or symlinks. * - * @param relativePath The path relative to the repository base path (returned - * by {@link RepositoryManager#getRepositoryRoot()}) on - * the filesystem. - * @param commitHash The commit ID hash for which to get the file hash. + * @param relativePath The path relative to the repository base path (returned by + * {@link RepositoryManager#getRepositoryRoot()}) on the filesystem. + * @param commitHash The commit ID hash for which to get the file hash. * @return The git file hash of the requested file. - * @throws IOException when file or commit isn't found, there are - * circular symlinks or other file IO errors. - * @throws NotFoundException when the file isn't committed to Git. - * @throws IllegalStateException when {@link RepositoryManager#cloneRemote()} - * isn't called before this. + * @throws IOException when file or commit isn't found, there are circular symlinks or other + * file IO errors. + * @throws NotFoundException when the file isn't committed to Git. + * @throws IllegalStateException when {@link RepositoryManager#cloneRemote()} isn't called + * before this. */ public String getFileHash(final Path relativePath, final String commitHash) throws IOException, NotFoundException, @@ -524,17 +519,14 @@ ObjectId resolveHead() throws IOException { } /** - * Gets the {@link RevCommit} for a {@link ObjectId} from the local git - * repository. + * Gets the {@link RevCommit} for a {@link ObjectId} from the local git repository. * - * @param commitId the ID of the commit to be fetched. - * @param fetchCommitsIfRequired Whether to fetch commits from remote - * repository when commit isn't found locally. + * @param commitId the ID of the commit to be fetched. + * @param fetchCommitsIfRequired Whether to fetch commits from remote repository when commit + * isn't found locally. * @return the {@link RevCommit} - * @throws IOException when the commit for the provided commitId isn't - * found. - * @throws GitAPIException when there are failures while fetching commits from - * the remote. + * @throws IOException when the commit for the provided commitId isn't found. + * @throws GitAPIException when there are failures while fetching commits from the remote. */ private RevCommit getRevCommit(final ObjectId commitId, final boolean fetchCommitsIfRequired) @@ -582,8 +574,8 @@ private void fetchRemote(final RemoteConfig remote) throws GitAPIException { } /** - * Does a hard reset to the - * remote branch being tracked. + * Does a hard reset to the remote branch being + * tracked. * * @param remoteBranchName branch to reset to. * @return the commit ID of the new head. @@ -605,8 +597,8 @@ private String resetToCleanBranch(final String remoteBranchName) * * @param directoryPath the path of the root directory. * @return the size of the directory in bytes. - * @throws IOException when there are failures while reading the file or {@link UncheckedIOException} - * is thrown out and we throw to the cause. + * @throws IOException when there are failures while reading the file or + * {@link UncheckedIOException} is thrown out and we throw to the cause. */ @VisibleForTesting static long calculateDirectorySize(Path directoryPath) throws IOException { diff --git a/cdap-source-control/src/main/java/io/cdap/cdap/sourcecontrol/operationrunner/InMemorySourceControlOperationRunner.java b/cdap-source-control/src/main/java/io/cdap/cdap/sourcecontrol/operationrunner/InMemorySourceControlOperationRunner.java index c48ef85e0e22..8152795869bc 100644 --- a/cdap-source-control/src/main/java/io/cdap/cdap/sourcecontrol/operationrunner/InMemorySourceControlOperationRunner.java +++ b/cdap-source-control/src/main/java/io/cdap/cdap/sourcecontrol/operationrunner/InMemorySourceControlOperationRunner.java @@ -16,6 +16,7 @@ package io.cdap.cdap.sourcecontrol.operationrunner; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.util.concurrent.AbstractIdleService; import com.google.gson.Gson; @@ -29,6 +30,8 @@ import io.cdap.cdap.common.utils.FileUtils; import io.cdap.cdap.proto.ApplicationDetail; import io.cdap.cdap.proto.artifact.AppRequest; +import io.cdap.cdap.proto.id.ApplicationReference; +import io.cdap.cdap.sourcecontrol.ApplicationManager; import io.cdap.cdap.sourcecontrol.AuthenticationConfigException; import io.cdap.cdap.sourcecontrol.CommitMeta; import io.cdap.cdap.sourcecontrol.ConfigFileWriteException; @@ -49,9 +52,13 @@ import java.nio.file.LinkOption; import java.nio.file.Path; import java.util.ArrayList; +import java.util.Collection; +import java.util.Iterator; +import java.util.LinkedList; import java.util.List; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; +import java.util.function.Supplier; import org.eclipse.jgit.api.errors.GitAPIException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -83,21 +90,56 @@ public PushAppResponse push(PushAppOperationRequest pushRequest) throws NoChange pushRequest.getNamespaceId(), pushRequest.getRepositoryConfig()) ) { - try { - repositoryManager.cloneRemote(); - } catch (GitAPIException | IOException e) { - throw new GitOperationException(String.format("Failed to clone remote repository: %s", - e.getMessage()), e); - } + cloneAndCreateBaseDir(repositoryManager); + ApplicationDetail app = pushRequest.getApp(); + writeAppDetail(repositoryManager, app); - LOG.info("Pushing application configs for : {}", pushRequest.getApp().getName()); + List responses = new ArrayList<>(commitAndPush(repositoryManager, + pushRequest.getCommitDetails(), + ImmutableList.of(new PushFile( + app.getName(), + app.getAppVersion(), + repositoryManager.getFileRelativePath(generateConfigFileName(app.getName())) + ) + ) + )); - //TODO: CDAP-20371, Add retry logic here in case the head at remote moved while we are doing push - return writeAppDetailAndPush( - repositoryManager, - pushRequest.getApp(), - pushRequest.getCommitDetails() - ); + // it should never be empty as in case of any error we will get an exception + return responses.get(0); + } + } + + @Override + public Collection multiPush(MultiPushAppOperationRequest pushRequest, + ApplicationManager appManager) + throws NoChangesToPushException, AuthenticationConfigException { + try ( + RepositoryManager repositoryManager = repoManagerFactory.create( + pushRequest.getNamespaceId(), + pushRequest.getRepositoryConfig()) + ) { + cloneAndCreateBaseDir(repositoryManager); + + LOG.debug("Pushing application configs for {} apps.", pushRequest.getApps().size()); + + List filesToPush = new LinkedList<>(); + + for (String appToPush : pushRequest.getApps()) { + LOG.trace("Pushing application configs for : {}.", appToPush); + ApplicationReference appRef = new ApplicationReference(pushRequest.getNamespaceId(), + appToPush); + try { + ApplicationDetail detail = appManager.get(appRef); + writeAppDetail(repositoryManager, detail); + filesToPush.add(new PushFile(detail.getName(), detail.getAppVersion(), + repositoryManager.getFileRelativePath(generateConfigFileName(detail.getName())))); + } catch (IOException | NotFoundException e) { + throw new SourceControlException( + String.format("Failed to fetch details for app %s", appRef), e); + } + } + + return commitAndPush(repositoryManager, pushRequest.getCommitDetails(), filesToPush); } } @@ -105,7 +147,7 @@ public PushAppResponse push(PushAppOperationRequest pushRequest) throws NoChange public PullAppResponse pull(PullAppOperationRequest pullRequest) throws NotFoundException, AuthenticationConfigException { AtomicReference> response = new AtomicReference<>(); - pull( + multiPull( new MultiPullAppOperationRequest( pullRequest.getRepositoryConfig(), pullRequest.getApp().getNamespaceId(), @@ -118,7 +160,7 @@ public PullAppResponse pull(PullAppOperationRequest pullRequest) } @Override - public void pull(MultiPullAppOperationRequest pullRequest, Consumer> consumer) + public void multiPull(MultiPullAppOperationRequest pullRequest, Consumer> consumer) throws SourceControlAppConfigNotFoundException, AuthenticationConfigException { LOG.info("Cloning remote to pull applications {}", pullRequest.getApps()); @@ -137,7 +179,8 @@ public void pull(MultiPullAppOperationRequest pullRequest, Consumer pullSingle(RepositoryManager repositoryManager, String commitId, - String applicationName) throws SourceControlException, SourceControlAppConfigNotFoundException { + String applicationName) + throws SourceControlException, SourceControlAppConfigNotFoundException { String configFileName = generateConfigFileName(applicationName); Path appRelativePath = repositoryManager.getFileRelativePath(configFileName); Path filePathToRead = validateAppConfigRelativePath(repositoryManager, appRelativePath); @@ -164,20 +207,12 @@ private PullAppResponse pullSingle(RepositoryManager repositoryManager, Strin } } - /** - * Atomic operation of writing application and push, return the push response. - * - * @param repositoryManager {@link RepositoryManager} to conduct git operations - * @param appToPush {@link ApplicationDetail} to push - * @param commitDetails {@link CommitMeta} from user input - * @return {@link PushAppResponse} - * @throws NoChangesToPushException if there's no change between the application in namespace and git repository - * @throws SourceControlException for failures while writing config file or doing git operations - */ - private PushAppResponse writeAppDetailAndPush(RepositoryManager repositoryManager, - ApplicationDetail appToPush, - CommitMeta commitDetails) - throws NoChangesToPushException { + private void cloneAndCreateBaseDir(RepositoryManager repositoryManager) { + try { + repositoryManager.cloneRemote(); + } catch (GitAPIException | IOException e) { + throw new GitOperationException("Failed to clone remote repository.", e); + } try { // Creates the base directory if it does not exist. This method does not throw an exception if the directory // already exists. This is for the case that the repo is new and user configured prefix path. @@ -185,7 +220,17 @@ private PushAppResponse writeAppDetailAndPush(RepositoryManager repositoryManage } catch (IOException e) { throw new SourceControlException("Failed to create repository base directory", e); } + } + /** + * Write {@link ApplicationDetail} to config file in git repository. + * + * @param repositoryManager {@link RepositoryManager} to conduct git operations + * @param appToPush application details to write + * @throws SourceControlException for failures while writing config file or doing git + * operations + */ + private void writeAppDetail(RepositoryManager repositoryManager, ApplicationDetail appToPush) { String configFileName = generateConfigFileName(appToPush.getName()); Path appRelativePath = repositoryManager.getFileRelativePath(configFileName); @@ -207,21 +252,43 @@ private PushAppResponse writeAppDetailAndPush(RepositoryManager repositoryManage ); } - LOG.debug("Wrote application configs for {} in file {}", appToPush.getName(), appRelativePath); + LOG.debug("Wrote application configs for {} in file {}", appToPush.getName(), + appRelativePath); + } + /** + * Atomic operation of writing commit and push, return push responses. + * + * @param repositoryManager {@link RepositoryManager} to conduct git operations + * @param commitMeta {@link CommitMeta} from user input + * @param filesToPush an {@link Iterator} of objects of the generic type T. + * @throws NoChangesToPushException if there's no effective change between applications in + * namespace and git repository + * @throws SourceControlException for failures while writing config file or doing git + * operations + */ + //TODO: CDAP-20371, Add retry logic here in case the head at remote moved while we are doing push + private Collection commitAndPush(RepositoryManager repositoryManager, + CommitMeta commitMeta, List filesToPush) throws NoChangesToPushException { + + // TODO: CDAP-20383, handle NoChangesToPushException + // Define the case that the application to push does not have any changes try { - // TODO: CDAP-20383, handle NoChangesToPushException - // Define the case that the application to push does not have any changes - String gitFileHash = repositoryManager.commitAndPush(commitDetails, appRelativePath); - return new PushAppResponse(appToPush.getName(), appToPush.getAppVersion(), gitFileHash); + return repositoryManager.commitAndPush(commitMeta, filesToPush, + (PushFile pushFile, String hash) -> new PushAppResponse( + pushFile.getAppName(), + pushFile.getAppVersion(), + hash + )); } catch (GitAPIException e) { - throw new GitOperationException(String.format("Failed to push config to git: %s", e.getMessage()), e); + throw new GitOperationException( + String.format("Failed to push configs to git: %s", e.getMessage()), e); } } /** - * Generate config file name from app name. - * Currently, it only adds `.json` as extension with app name being the filename. + * Generate config file name from app name. Currently, it only adds `.json` as extension with app + * name being the filename. * * @param appName Name of the application * @return The file name we want to store application config in @@ -308,5 +375,40 @@ protected void startUp() throws Exception { protected void shutDown() throws Exception { // No-op. } + + /** + * Container class to encapsulate files to push information. + */ + public static class PushFile implements Supplier { + + private final String appName; + private final String appVersion; + private final Path path; + + /** + * Constructs an object encapsulating information for a file to push. + * @param appName name of the application to push + * @param appVersion version of the application to push + * @param path filepath of the application json to push + */ + public PushFile(String appName, String appVersion, Path path) { + this.appName = appName; + this.appVersion = appVersion; + this.path = path; + } + + public String getAppName() { + return appName; + } + + public String getAppVersion() { + return appVersion; + } + + @Override + public Path get() { + return path; + } + } } diff --git a/cdap-source-control/src/main/java/io/cdap/cdap/sourcecontrol/operationrunner/MultiPushAppOperationRequest.java b/cdap-source-control/src/main/java/io/cdap/cdap/sourcecontrol/operationrunner/MultiPushAppOperationRequest.java new file mode 100644 index 000000000000..79b4875a618b --- /dev/null +++ b/cdap-source-control/src/main/java/io/cdap/cdap/sourcecontrol/operationrunner/MultiPushAppOperationRequest.java @@ -0,0 +1,62 @@ +/* + * Copyright © 2023 Cask Data, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ + +package io.cdap.cdap.sourcecontrol.operationrunner; + +import io.cdap.cdap.proto.id.NamespaceId; +import io.cdap.cdap.proto.sourcecontrol.RepositoryConfig; +import io.cdap.cdap.sourcecontrol.CommitMeta; +import java.util.Set; + +/** + * Information required by {@link SourceControlOperationRunner} to execute the task of + * pushing multiple applications to linked repository. + */ +public class MultiPushAppOperationRequest { + private final NamespaceId namespace; + private final RepositoryConfig repoConfig; + private final Set apps; + private final CommitMeta commitDetails; + + /** + * Default constructor. + */ + public MultiPushAppOperationRequest(NamespaceId namespace, + RepositoryConfig repoConfig, + Set apps, + CommitMeta commitDetails) { + this.namespace = namespace; + this.repoConfig = repoConfig; + this.apps = apps; + this.commitDetails = commitDetails; + } + + public NamespaceId getNamespaceId() { + return namespace; + } + + public RepositoryConfig getRepositoryConfig() { + return repoConfig; + } + + public Set getApps() { + return apps; + } + + public CommitMeta getCommitDetails() { + return commitDetails; + } +} diff --git a/cdap-source-control/src/main/java/io/cdap/cdap/sourcecontrol/operationrunner/RemoteSourceControlOperationRunner.java b/cdap-source-control/src/main/java/io/cdap/cdap/sourcecontrol/operationrunner/RemoteSourceControlOperationRunner.java index 274872560a81..97878b81d9a3 100644 --- a/cdap-source-control/src/main/java/io/cdap/cdap/sourcecontrol/operationrunner/RemoteSourceControlOperationRunner.java +++ b/cdap-source-control/src/main/java/io/cdap/cdap/sourcecontrol/operationrunner/RemoteSourceControlOperationRunner.java @@ -29,6 +29,7 @@ import io.cdap.cdap.common.conf.Constants; import io.cdap.cdap.common.internal.remote.RemoteClientFactory; import io.cdap.cdap.common.internal.remote.RemoteTaskExecutor; +import io.cdap.cdap.sourcecontrol.ApplicationManager; import io.cdap.cdap.sourcecontrol.AuthenticationConfigException; import io.cdap.cdap.sourcecontrol.NoChangesToPushException; import io.cdap.cdap.sourcecontrol.SourceControlAppConfigNotFoundException; @@ -38,6 +39,7 @@ import io.cdap.cdap.sourcecontrol.worker.PushAppTask; import io.cdap.common.http.HttpRequestConfig; import java.nio.charset.StandardCharsets; +import java.util.Collection; import java.util.function.Consumer; import javax.inject.Inject; import org.slf4j.Logger; @@ -97,6 +99,13 @@ public PushAppResponse push(PushAppOperationRequest pushRequest) throws NoChange } } + @Override + public Collection multiPush(MultiPushAppOperationRequest pushRequest, + ApplicationManager appManager) + throws NoChangesToPushException, AuthenticationConfigException { + throw new UnsupportedOperationException("multi push not supported for RemoteSourceControlOperationRunner"); + } + @Override public PullAppResponse pull(PullAppOperationRequest pullRequest) throws NotFoundException, AuthenticationConfigException { @@ -117,7 +126,7 @@ public PullAppResponse pull(PullAppOperationRequest pullRequest) throws NotFo } @Override - public void pull(MultiPullAppOperationRequest pullRequest, Consumer> consumer) + public void multiPull(MultiPullAppOperationRequest pullRequest, Consumer> consumer) throws NotFoundException, AuthenticationConfigException { throw new UnsupportedOperationException("multi pull not supported for RemoteSourceControlOperationRunner"); } diff --git a/cdap-source-control/src/main/java/io/cdap/cdap/sourcecontrol/operationrunner/SourceControlOperationRunner.java b/cdap-source-control/src/main/java/io/cdap/cdap/sourcecontrol/operationrunner/SourceControlOperationRunner.java index 2daeb50c9150..8f332ab1599e 100644 --- a/cdap-source-control/src/main/java/io/cdap/cdap/sourcecontrol/operationrunner/SourceControlOperationRunner.java +++ b/cdap-source-control/src/main/java/io/cdap/cdap/sourcecontrol/operationrunner/SourceControlOperationRunner.java @@ -18,9 +18,11 @@ import com.google.common.util.concurrent.Service; import io.cdap.cdap.common.NotFoundException; +import io.cdap.cdap.sourcecontrol.ApplicationManager; import io.cdap.cdap.sourcecontrol.AuthenticationConfigException; import io.cdap.cdap.sourcecontrol.NoChangesToPushException; import io.cdap.cdap.sourcecontrol.SourceControlException; +import java.util.Collection; import java.util.function.Consumer; /** @@ -40,6 +42,20 @@ public interface SourceControlOperationRunner extends Service { PushAppResponse push(PushAppOperationRequest pushRequest) throws NoChangesToPushException, AuthenticationConfigException; + /** + * Push application configs to remote git repository. + * + * @param pushRequest {@link MultiPushAppOperationRequest} of the applications to be pushed + * @param appManager {@link ApplicationManager} to fetch the app details + * @return file-paths and file-hashes for the updated configs. + * @throws AuthenticationConfigException when there is an error while creating the authentication credentials to + * call remote Git. + * @throws SourceControlException when the push operation fails for any other reason. + */ + Collection multiPush(MultiPushAppOperationRequest pushRequest, + ApplicationManager appManager) + throws NoChangesToPushException, AuthenticationConfigException; + /** * Gets an application spec from a Git repository. * @@ -66,7 +82,7 @@ PullAppResponse pull(PullAppOperationRequest pullRequest) throws NotFoundExce * @throws IllegalArgumentException when the fetched application json or file path is invalid. * @throws SourceControlException when the operation fails for any other reason. */ - void pull(MultiPullAppOperationRequest pullRequest, Consumer> consumer) + void multiPull(MultiPullAppOperationRequest pullRequest, Consumer> consumer) throws NotFoundException, AuthenticationConfigException; /** diff --git a/cdap-source-control/src/test/java/io/cdap/cdap/sourcecontrol/RepositoryManagerTest.java b/cdap-source-control/src/test/java/io/cdap/cdap/sourcecontrol/RepositoryManagerTest.java index 644cee29c745..af12b0a92c79 100644 --- a/cdap-source-control/src/test/java/io/cdap/cdap/sourcecontrol/RepositoryManagerTest.java +++ b/cdap-source-control/src/test/java/io/cdap/cdap/sourcecontrol/RepositoryManagerTest.java @@ -16,6 +16,7 @@ package io.cdap.cdap.sourcecontrol; +import com.google.common.collect.ImmutableSet; import io.cdap.cdap.api.metrics.MetricsCollectionService; import io.cdap.cdap.api.metrics.MetricsContext; import io.cdap.cdap.api.security.store.SecureStore; @@ -39,14 +40,18 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.nio.file.attribute.PosixFilePermissions; +import java.util.Collection; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; +import java.util.function.Supplier; import java.util.stream.Collectors; import java.util.stream.StreamSupport; import org.eclipse.jgit.api.Git; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.revwalk.RevCommit; +import org.eclipse.jgit.treewalk.TreeWalk; import org.eclipse.jgit.util.SystemReader; import org.junit.Assert; import org.junit.Before; @@ -359,12 +364,24 @@ public void testCommitAndPushSuccess() throws Exception { manager.cloneRemote(); CommitMeta commitMeta = new CommitMeta("author", "committer", 100, "message"); - Path filePath = manager.getBasePath().resolve("file1"); + Path filePath1 = manager.getBasePath().resolve("file1"); + Path filePath2 = manager.getBasePath().resolve("file2"); String fileContent = "content"; - Files.write(filePath, fileContent.getBytes(StandardCharsets.UTF_8)); - manager.commitAndPush(commitMeta, - manager.getBasePath().relativize(filePath)); + Files.write(filePath1, fileContent.getBytes(StandardCharsets.UTF_8)); + Files.write(filePath2, fileContent.getBytes(StandardCharsets.UTF_8)); + + Set filePaths = ImmutableSet.of( + new PathSupplier(manager.getBasePath().relativize(filePath1)), + new PathSupplier(manager.getBasePath().relativize(filePath2)) + ); + + Collection suppliers = manager.commitAndPush(commitMeta, filePaths, + (path, hash) -> { + path.setHash(hash); + return path; + }); + verifyCommit(suppliers, fileContent, commitMeta); // Verify metrics. Mockito.verify(mockMetricsContext).event( Mockito.eq(SourceControlManagement.CLONE_REPOSITORY_SIZE_BYTES), @@ -375,7 +392,6 @@ public void testCommitAndPushSuccess() throws Exception { Mockito.verify(mockMetricsContext).event( Mockito.eq(SourceControlManagement.COMMIT_PUSH_LATENCY_MILLIS), Mockito.anyLong()); - verifyCommit(filePath, fileContent, commitMeta); } } @@ -398,11 +414,16 @@ public void testCommitAndPushSourceControlFailure() throws Exception { String fileContent = "content"; Files.write(filePath, fileContent.getBytes(StandardCharsets.UTF_8)); - manager.commitAndPush(commitMeta, Paths.get("fileThatDoesNotExist.json")); + manager.commitAndPush( + commitMeta, + ImmutableSet.of(new PathSupplier(Paths.get("file1")), + new PathSupplier(Paths.get("fileThatDoesNotExist.json"))), + (path, hash) -> "" + ); } } - @Test(expected = NoChangesToPushException.class) + @Test public void testCommitAndPushNoChangeSuccess() throws Exception { String pathPrefix = "prefix"; String serverUrl = gitServer.getServerUrl(); @@ -410,7 +431,6 @@ public void testCommitAndPushNoChangeSuccess() throws Exception { Provider.GITHUB) .setLink(serverUrl + "ignored") .setDefaultBranch("develop") - .setPathPrefix(pathPrefix) .setAuth(AUTH_CONFIG) .build(); @@ -419,9 +439,34 @@ public void testCommitAndPushNoChangeSuccess() throws Exception { manager.cloneRemote(); CommitMeta commitMeta = new CommitMeta("author", "committer", 100, "message"); - manager.commitAndPush(commitMeta, Paths.get(pathPrefix)); - verifyNoCommit(); + Path filePath1 = manager.getBasePath().resolve("file1"); + Path filePath2 = manager.getBasePath().resolve("file2"); + String fileContent = "content"; + Files.write(filePath1, fileContent.getBytes(StandardCharsets.UTF_8)); + Files.write(filePath2, fileContent.getBytes(StandardCharsets.UTF_8)); + + Path file2 = Paths.get("file2"); + Path file1 = Paths.get("file1"); + manager.commitAndPush(commitMeta, + ImmutableSet.of(new PathSupplier(file1), new PathSupplier(file2)), + (path, hash) -> "" + ); + + Files.write(filePath1, fileContent.getBytes(StandardCharsets.UTF_8)); + Files.write(filePath2, fileContent.getBytes(StandardCharsets.UTF_8)); + + try { + manager.commitAndPush(commitMeta, + ImmutableSet.of(new PathSupplier(file1), new PathSupplier(file2)), + (path, hash) -> "" + ); + Assert.fail(); + } catch (NoChangesToPushException e) { + // expected + } + + verifyCommitCount(2); } } @@ -445,8 +490,8 @@ public void testCommitAndPushFails() throws Exception { Files.write(filePath, fileContent.getBytes(StandardCharsets.UTF_8)); gitServer.after(); - manager.commitAndPush(commitMeta, - manager.getBasePath().relativize(filePath)); + manager.commitAndPush(commitMeta, ImmutableSet.of(new PathSupplier(manager.getBasePath().relativize(filePath))), + (path, hash) -> ""); } } @@ -466,29 +511,24 @@ public void testDirectorySizeMb() throws IOException { RepositoryManager.calculateDirectorySize(root)); } - private void verifyNoCommit() throws GitAPIException, IOException { + private void verifyCommitCount(int count) throws GitAPIException, IOException { Path tempDirPath = baseTempFolder.newFolder("temp-local-git-verify") .toPath(); Git localGit = getClonedGit(tempDirPath, gitServer); List commits = StreamSupport.stream( localGit.log().all().call().spliterator(), false) .collect(Collectors.toList()); - Assert.assertEquals(1, commits.size()); + Assert.assertEquals(count, commits.size()); } - private void verifyCommit(Path pathFromRepoRoot, String expectedContent, - CommitMeta commitMeta) + private void verifyCommit(Collection pathSuppliers, String expectedContent, CommitMeta commitMeta) throws GitAPIException, IOException { Path tempDirPath = baseTempFolder.newFolder("temp-local-git-verify") .toPath(); Git localGit = getClonedGit(tempDirPath, gitServer); - Path filePath = tempDirPath.resolve(pathFromRepoRoot); - - String actualContent = new String(Files.readAllBytes(filePath), - StandardCharsets.UTF_8); - Assert.assertEquals(expectedContent, actualContent); + // verify commit List commits = StreamSupport.stream(localGit.log().all().call().spliterator(), false) .collect(Collectors.toList()); @@ -501,6 +541,19 @@ private void verifyCommit(Path pathFromRepoRoot, String expectedContent, Assert.assertEquals(commitMeta.getCommitter(), latestCommit.getCommitterIdent().getName()); + for (PathSupplier pathSupplier : pathSuppliers) { + // verify content + Path filePath = tempDirPath.resolve(pathSupplier.get()); + String actualContent = new String(Files.readAllBytes(filePath), + StandardCharsets.UTF_8); + Assert.assertEquals(expectedContent, actualContent); + + // verify hash + String gotHash = TreeWalk.forPath(localGit.getRepository(), pathSupplier.get().toString(), + latestCommit.getTree()).getObjectId(0).getName(); + Assert.assertEquals(pathSupplier.getHash(), gotHash); + } + localGit.close(); DirUtils.deleteDirectoryContents(tempDirPath.toFile()); } @@ -539,7 +592,8 @@ private void commitFileToLocalRepository(Path filePath, String contents, throws IOException, NoChangesToPushException, GitAPIException { Files.write(manager.getRepositoryRoot().resolve(filePath), contents.getBytes(StandardCharsets.UTF_8)); - manager.commitAndPush(commitMeta, filePath); + manager.commitAndPush(commitMeta, ImmutableSet.of(new PathSupplier(filePath)), + (path, hash) -> ""); } private void installHook(RepositoryManager manager) throws IOException { @@ -552,4 +606,27 @@ private void installHook(RepositoryManager manager) throws IOException { Files.write(gitHookPath, "touch hook_executed\nexit 0".getBytes( StandardCharsets.UTF_8)); } + + private static class PathSupplier implements Supplier { + + final Path path; + String hash; + + private PathSupplier(Path path) { + this.path = path; + } + + public void setHash(String hash) { + this.hash = hash; + } + + private String getHash() { + return hash; + } + + @Override + public Path get() { + return path; + } + } } diff --git a/cdap-source-control/src/test/java/io/cdap/cdap/sourcecontrol/operationrunner/InMemorySourceControlOperationRunnerTest.java b/cdap-source-control/src/test/java/io/cdap/cdap/sourcecontrol/operationrunner/InMemorySourceControlOperationRunnerTest.java index 32df67ae538f..2cef65a68b67 100644 --- a/cdap-source-control/src/test/java/io/cdap/cdap/sourcecontrol/operationrunner/InMemorySourceControlOperationRunnerTest.java +++ b/cdap-source-control/src/test/java/io/cdap/cdap/sourcecontrol/operationrunner/InMemorySourceControlOperationRunnerTest.java @@ -16,6 +16,7 @@ package io.cdap.cdap.sourcecontrol.operationrunner; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Sets; import com.google.gson.Gson; import com.google.gson.GsonBuilder; @@ -28,6 +29,7 @@ import io.cdap.cdap.proto.sourcecontrol.PatConfig; import io.cdap.cdap.proto.sourcecontrol.Provider; import io.cdap.cdap.proto.sourcecontrol.RepositoryConfig; +import io.cdap.cdap.sourcecontrol.ApplicationManager; import io.cdap.cdap.sourcecontrol.AuthenticationConfigException; import io.cdap.cdap.sourcecontrol.CommitMeta; import io.cdap.cdap.sourcecontrol.GitOperationException; @@ -43,8 +45,12 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; import java.util.Comparator; +import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.stream.Collectors; import org.junit.Assert; import org.junit.Before; @@ -52,10 +58,11 @@ import org.mockito.Mockito; public class InMemorySourceControlOperationRunnerTest extends SourceControlTestBase { + private static final String FAKE_COMMIT_HASH = "5905258bb958ceda80b6a37938050ad876920f10"; private static final ApplicationDetail testAppDetails = new ApplicationDetail( - TEST_APP_NAME, "v1", "description1", null, null, "conf1", new ArrayList<>(), - new ArrayList<>(), new ArrayList<>(), null, null); + TEST_APP_NAME, "v1", "description1", null, null, "conf1", new ArrayList<>(), + new ArrayList<>(), new ArrayList<>(), null, null); private static final ApplicationDetail testApp2Details = new ApplicationDetail( TEST_APP2_NAME, "v1", "description1", null, null, "conf1", new ArrayList<>(), new ArrayList<>(), new ArrayList<>(), null, null); @@ -67,31 +74,36 @@ public class InMemorySourceControlOperationRunnerTest extends SourceControlTestB .setAuth(new AuthConfig(AuthType.PAT, new PatConfig("GITHUB_TOKEN_NAME", null))) .build(); private static final CommitMeta testCommit = - new CommitMeta("author1", "committer1", 123, "message1"); + new CommitMeta("author1", "committer1", 123, "message1"); private static final Gson GSON = new GsonBuilder().setPrettyPrinting().create(); private static final NamespaceId NAMESPACE = NamespaceId.DEFAULT; private static final PushAppOperationRequest pushContext = - new PushAppOperationRequest(NAMESPACE, testRepoConfig, testAppDetails, testCommit); - private static final NamespaceRepository nameSpaceRepository = new NamespaceRepository(NAMESPACE, testRepoConfig); + new PushAppOperationRequest(NAMESPACE, testRepoConfig, testAppDetails, testCommit); + private static final NamespaceRepository nameSpaceRepository = new NamespaceRepository(NAMESPACE, + testRepoConfig); private InMemorySourceControlOperationRunner operationRunner; private RepositoryManager mockRepositoryManager; @Before public void setUp() throws Exception { - RepositoryManagerFactory mockRepositoryManagerFactory = Mockito.mock(RepositoryManagerFactory.class); + RepositoryManagerFactory mockRepositoryManagerFactory = Mockito.mock( + RepositoryManagerFactory.class); this.mockRepositoryManager = Mockito.mock(RepositoryManager.class); - Mockito.doReturn(mockRepositoryManager).when(mockRepositoryManagerFactory).create(Mockito.any(), Mockito.any()); + Mockito.doReturn(mockRepositoryManager).when(mockRepositoryManagerFactory) + .create(Mockito.any(), Mockito.any()); Mockito.doReturn(FAKE_COMMIT_HASH).when(mockRepositoryManager).cloneRemote(); this.operationRunner = new InMemorySourceControlOperationRunner(mockRepositoryManagerFactory); Path appRelativePath = Paths.get(PATH_PREFIX, testAppDetails.getName() + ".json"); - Mockito.doReturn(appRelativePath).when(mockRepositoryManager).getFileRelativePath(Mockito.any()); + Mockito.doReturn(appRelativePath).when(mockRepositoryManager) + .getFileRelativePath(Mockito.any()); } - private boolean verifyConfigFileContent(Path repoDirPath) throws IOException { + private boolean verifyConfigFileContent(Path repoDirPath, ApplicationDetail detail) + throws IOException { String fileData = new String(Files.readAllBytes( - repoDirPath.resolve(String.format("%s.json", testAppDetails.getName()))), StandardCharsets.UTF_8); - return fileData.equals(GSON.toJson(testAppDetails)); + repoDirPath.resolve(String.format("%s.json", detail.getName()))), StandardCharsets.UTF_8); + return fileData.equals(GSON.toJson(detail)); } @Test @@ -102,12 +114,53 @@ public void testPushSuccess() throws Exception { Mockito.doReturn(tmpRepoDirPath).when(mockRepositoryManager).getRepositoryRoot(); Mockito.doReturn(baseRepoDirPath).when(mockRepositoryManager).getBasePath(); - Mockito.doReturn("file Hash").when(mockRepositoryManager).commitAndPush(Mockito.anyObject(), - Mockito.any()); + Mockito.doReturn( + ImmutableSet.of( + new PushAppResponse(pushContext.getApp().getName(), + pushContext.getApp().getAppVersion(), "file-hash") + )) + .when(mockRepositoryManager).commitAndPush(Mockito.anyObject(), + Mockito.any(), Mockito.any()); operationRunner.push(pushContext); - Assert.assertTrue(verifyConfigFileContent(baseRepoDirPath)); + Assert.assertTrue(verifyConfigFileContent(baseRepoDirPath, testAppDetails)); + } + + @Test + public void testMultiPushSuccess() throws Exception { + setupPushTest(); + Path tmpRepoDirPath = baseTempFolder.newFolder().toPath(); + Path baseRepoDirPath = tmpRepoDirPath.resolve(PATH_PREFIX); + + ApplicationManager mockManager = Mockito.mock(ApplicationManager.class); + Mockito.doReturn(testAppDetails).when(mockManager) + .get(new ApplicationReference(NAMESPACE, TEST_APP_NAME)); + Mockito.doReturn(testApp2Details).when(mockManager) + .get(new ApplicationReference(NAMESPACE, TEST_APP2_NAME)); + + Mockito.doReturn(tmpRepoDirPath).when(mockRepositoryManager).getRepositoryRoot(); + Mockito.doReturn(baseRepoDirPath).when(mockRepositoryManager).getBasePath(); + Set expectedResponses = ImmutableSet.of( + new PushAppResponse(testAppDetails.getName(), testAppDetails.getAppVersion(), "file-hash1"), + new PushAppResponse(testApp2Details.getName(), testApp2Details.getAppVersion(), + "file-hash2") + ); + + Mockito.doReturn(expectedResponses) + .when(mockRepositoryManager) + .commitAndPush(Mockito.anyObject(), Mockito.any(), Mockito.any()); + + MultiPushAppOperationRequest request = + new MultiPushAppOperationRequest(NAMESPACE, testRepoConfig, + ImmutableSet.of(TEST_APP_NAME, TEST_APP2_NAME), testCommit); + + Collection gotResponses = operationRunner.multiPush(request, mockManager); + + Assert.assertEquals(expectedResponses, new HashSet<>(gotResponses)); + + Assert.assertTrue(verifyConfigFileContent(baseRepoDirPath, testAppDetails)); + Assert.assertTrue(verifyConfigFileContent(baseRepoDirPath, testApp2Details)); } @Test(expected = SourceControlException.class) @@ -145,8 +198,9 @@ public void testPushFailedInvalidSymlinkPath() throws Exception { Mockito.doReturn(tmpRepoDirPath).when(mockRepositoryManager).getRepositoryRoot(); Mockito.doReturn(baseRepoDirPath).when(mockRepositoryManager).getBasePath(); - Mockito.doReturn("file Hash").when(mockRepositoryManager).commitAndPush(Mockito.anyObject(), - Mockito.any()); + Mockito.doReturn(Collections.emptyList()) + .when(mockRepositoryManager).commitAndPush(Mockito.anyObject(), + Mockito.any(), Mockito.any()); Path target = tmpRepoDirPath.resolve("target"); Files.createDirectories(baseRepoDirPath); @@ -157,7 +211,8 @@ public void testPushFailedInvalidSymlinkPath() throws Exception { @Test(expected = AuthenticationConfigException.class) public void testPushFailedToClone() throws Exception { - Mockito.doThrow(new AuthenticationConfigException("config not exists")).when(mockRepositoryManager).cloneRemote(); + Mockito.doThrow(new AuthenticationConfigException("config not exists")) + .when(mockRepositoryManager).cloneRemote(); operationRunner.push(pushContext); } @@ -170,7 +225,7 @@ public void testPushNoChanges() throws Exception { Mockito.doReturn(tmpRepoDirPath).when(mockRepositoryManager).getRepositoryRoot(); Mockito.doReturn(baseRepoDirPath).when(mockRepositoryManager).getBasePath(); Mockito.doThrow(new NoChangesToPushException("no changes to push")) - .when(mockRepositoryManager).commitAndPush(Mockito.any(), Mockito.any()); + .when(mockRepositoryManager).commitAndPush(Mockito.any(), Mockito.any(), Mockito.any()); operationRunner.push(pushContext); } @@ -196,18 +251,18 @@ public void testListSuccess() throws Exception { Mockito.doReturn(FAKE_COMMIT_HASH).when(mockRepositoryManager).cloneRemote(); Mockito.doReturn(tmpRepoDirPath).when(mockRepositoryManager).getBasePath(); Mockito.doReturn(file1.getFileName()).when(mockRepositoryManager) - .getFileRelativePath(file1.getFileName().toString()); + .getFileRelativePath(file1.getFileName().toString()); Mockito.doReturn(app1.getFileHash()).when(mockRepositoryManager) - .getFileHash(file1.getFileName(), FAKE_COMMIT_HASH); + .getFileHash(file1.getFileName(), FAKE_COMMIT_HASH); Mockito.doReturn(file2.getFileName()).when(mockRepositoryManager) - .getFileRelativePath(file2.getFileName().toString()); + .getFileRelativePath(file2.getFileName().toString()); Mockito.doReturn(app2.getFileHash()).when(mockRepositoryManager) - .getFileHash(file2.getFileName(), FAKE_COMMIT_HASH); + .getFileHash(file2.getFileName(), FAKE_COMMIT_HASH); Mockito.doNothing().when(mockRepositoryManager).close(); List sortedListedApps = - operationRunner.list(nameSpaceRepository).getApps().stream() - .sorted(Comparator.comparing(RepositoryApp::getName)).collect(Collectors.toList()); + operationRunner.list(nameSpaceRepository).getApps().stream() + .sorted(Comparator.comparing(RepositoryApp::getName)).collect(Collectors.toList()); Assert.assertEquals(2, sortedListedApps.size()); Assert.assertEquals(sortedListedApps.get(0), app1); @@ -241,8 +296,10 @@ public void testListFailedToGetHash() throws Exception { Mockito.doReturn(tmpRepoDirPath).when(mockRepositoryManager).getBasePath(); Path file1 = tmpRepoDirPath.resolve("file1.some.json"); Files.write(file1, new byte[]{}); - Mockito.doReturn(file1.getFileName()).when(mockRepositoryManager).getFileRelativePath(Mockito.any()); - Mockito.doThrow(new IOException()).when(mockRepositoryManager).getFileHash(file1.getFileName(), FAKE_COMMIT_HASH); + Mockito.doReturn(file1.getFileName()).when(mockRepositoryManager) + .getFileRelativePath(Mockito.any()); + Mockito.doThrow(new IOException()).when(mockRepositoryManager) + .getFileHash(file1.getFileName(), FAKE_COMMIT_HASH); operationRunner.list(nameSpaceRepository); } @@ -251,7 +308,8 @@ public void testListFailedToGetHash() throws Exception { public void testPullSuccess() throws Exception { setupPullTest(); ApplicationReference appRef = new ApplicationReference(NAMESPACE, TEST_APP_NAME); - PullAppResponse response = operationRunner.pull(new PullAppOperationRequest(appRef, testRepoConfig)); + PullAppResponse response = operationRunner.pull( + new PullAppOperationRequest(appRef, testRepoConfig)); validatePullResponse(response, TEST_APP_NAME); Mockito.verify(mockRepositoryManager, Mockito.times(1)).cloneRemote(); Mockito.verify(mockRepositoryManager, Mockito.times(1)).close(); @@ -261,8 +319,9 @@ public void testPullSuccess() throws Exception { public void testMultiPullSuccess() throws Exception { setupPullTest(); List> responses = new ArrayList<>(); - operationRunner.pull( - new MultiPullAppOperationRequest(testRepoConfig, NAMESPACE, Sets.newHashSet(TEST_APP_NAME, TEST_APP2_NAME)), + operationRunner.multiPull( + new MultiPullAppOperationRequest(testRepoConfig, NAMESPACE, + Sets.newHashSet(TEST_APP_NAME, TEST_APP2_NAME)), responses::add ); Assert.assertEquals(responses.size(), 2); @@ -281,8 +340,8 @@ public void testPullFailedToReadHash() throws Exception { setupPullTest(); ApplicationReference appRef = new ApplicationReference(NAMESPACE, TEST_APP_NAME); Mockito.doThrow(new NotFoundException("object not found")) - .when(mockRepositoryManager) - .getFileHash(Mockito.any(Path.class), Mockito.any(String.class)); + .when(mockRepositoryManager) + .getFileHash(Mockito.any(Path.class), Mockito.any(String.class)); try { operationRunner.pull(new PullAppOperationRequest(appRef, testRepoConfig)); } finally { @@ -296,7 +355,7 @@ public void testPullFileNotFound() throws Exception { setupPullTest(); ApplicationReference appRef = new ApplicationReference(NAMESPACE, TEST_APP_NAME); Mockito.doReturn(Paths.get(PATH_PREFIX, "notpresent.json")) - .when(mockRepositoryManager).getFileRelativePath(Mockito.any()); + .when(mockRepositoryManager).getFileRelativePath(Mockito.any()); try { operationRunner.pull(new PullAppOperationRequest(appRef, testRepoConfig)); } finally { @@ -309,7 +368,8 @@ public void testPullFileNotFound() throws Exception { public void testPullCloneFailure() throws Exception { setupPullTest(); ApplicationReference appRef = new ApplicationReference(NAMESPACE, TEST_APP_NAME); - Mockito.doThrow(new IOException("secure store failure")).when(mockRepositoryManager).cloneRemote(); + Mockito.doThrow(new IOException("secure store failure")).when(mockRepositoryManager) + .cloneRemote(); try { operationRunner.pull(new PullAppOperationRequest(appRef, testRepoConfig)); } finally { @@ -319,8 +379,12 @@ public void testPullCloneFailure() throws Exception { } private void setupPushTest() { - Path appRelativePath = Paths.get(PATH_PREFIX, testAppDetails.getName() + ".json"); - Mockito.doReturn(appRelativePath).when(mockRepositoryManager).getFileRelativePath(Mockito.any()); + String fileName1 = testAppDetails.getName() + ".json"; + String fileName2 = testApp2Details.getName() + ".json"; + Mockito.doReturn(Paths.get(PATH_PREFIX, fileName1)).when(mockRepositoryManager) + .getFileRelativePath(fileName1); + Mockito.doReturn(Paths.get(PATH_PREFIX, fileName2)).when(mockRepositoryManager) + .getFileRelativePath(fileName2); } private void setupPullTest() throws Exception { @@ -330,14 +394,18 @@ private void setupPullTest() throws Exception { Mockito.doReturn(baseRepoDirPath).when(mockRepositoryManager).getBasePath(); Mockito.doReturn(FAKE_COMMIT_HASH).when(mockRepositoryManager).cloneRemote(); Mockito.doReturn(TEST_FILE_HASH) - .when(mockRepositoryManager) - .getFileHash(Mockito.eq(Paths.get(PATH_PREFIX, TEST_APP_NAME + ".json")), Mockito.eq(FAKE_COMMIT_HASH)); + .when(mockRepositoryManager) + .getFileHash(Mockito.eq(Paths.get(PATH_PREFIX, TEST_APP_NAME + ".json")), + Mockito.eq(FAKE_COMMIT_HASH)); Mockito.doReturn(TEST_FILE_HASH) .when(mockRepositoryManager) - .getFileHash(Mockito.eq(Paths.get(PATH_PREFIX, TEST_APP2_NAME + ".json")), Mockito.eq(FAKE_COMMIT_HASH)); + .getFileHash(Mockito.eq(Paths.get(PATH_PREFIX, TEST_APP2_NAME + ".json")), + Mockito.eq(FAKE_COMMIT_HASH)); Files.createDirectories(baseRepoDirPath); - Files.write(baseRepoDirPath.resolve(TEST_APP_NAME + ".json"), TEST_APP_SPEC.getBytes(StandardCharsets.UTF_8)); - Files.write(baseRepoDirPath.resolve(TEST_APP2_NAME + ".json"), TEST_APP_SPEC.getBytes(StandardCharsets.UTF_8)); + Files.write(baseRepoDirPath.resolve(TEST_APP_NAME + ".json"), + TEST_APP_SPEC.getBytes(StandardCharsets.UTF_8)); + Files.write(baseRepoDirPath.resolve(TEST_APP2_NAME + ".json"), + TEST_APP_SPEC.getBytes(StandardCharsets.UTF_8)); Mockito.doNothing().when(mockRepositoryManager).close(); Path appRelativePath = Paths.get(PATH_PREFIX, testAppDetails.getName() + ".json"); Path app2RelativePath = Paths.get(PATH_PREFIX, testApp2Details.getName() + ".json");