Revert "Properly propagate failure when ref deletion fails" This reverts commit ced058a15a212b76dcd858cc5bdb6d883def88cb. Reason for revert: Ref deletion upon fetches was not properly implemented in stable-3.5 and therefore this fix should not be applied in this branch. Bug: Issue 288965464 Change-Id: I334812e07639ff4c6e4f89f9d3cce62a25b1246e
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/FetchRefReplicatedEvent.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/FetchRefReplicatedEvent.java index 2c18983..0eabf42 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/FetchRefReplicatedEvent.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/FetchRefReplicatedEvent.java
@@ -14,7 +14,6 @@ package com.googlesource.gerrit.plugins.replication.pull; -import com.google.common.annotations.VisibleForTesting; import com.google.gerrit.entities.Project; import com.googlesource.gerrit.plugins.replication.events.RemoteRefReplicationEvent; import java.util.Objects; @@ -75,9 +74,4 @@ public String getRefName() { return ref; } - - @VisibleForTesting - public RefUpdate.Result getRefUpdateResult() { - return refUpdateResult; - } }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/DeleteRefCommand.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/DeleteRefCommand.java index 6b2a0cf..949e032 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/DeleteRefCommand.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/DeleteRefCommand.java
@@ -35,7 +35,6 @@ import com.googlesource.gerrit.plugins.replication.pull.ReplicationState; import com.googlesource.gerrit.plugins.replication.pull.Source; import com.googlesource.gerrit.plugins.replication.pull.SourcesCollection; -import com.googlesource.gerrit.plugins.replication.pull.api.exception.DeleteRefException; import com.googlesource.gerrit.plugins.replication.pull.fetch.ApplyObject; import com.googlesource.gerrit.plugins.replication.pull.fetch.RefUpdateState; import java.io.IOException; @@ -107,7 +106,7 @@ try { Context.setLocalEvent(true); - RefUpdate.Result successResult = ensureSuccess(deleteRef(name, ref.get())); + deleteRef(name, ref.get()); eventDispatcher .get() @@ -117,17 +116,13 @@ refName, sourceUri, ReplicationState.RefFetchResult.SUCCEEDED, - successResult)); + RefUpdate.Result.FORCED)); } catch (PermissionBackendException e) { logger.atSevere().withCause(e).log( "Unexpected error while trying to delete ref '%s' on project %s and notifying it", refName, name); throw RestApiException.wrap(e.getMessage(), e); } catch (IOException e) { - RefUpdate.Result refUpdateResult = - e instanceof DeleteRefException - ? ((DeleteRefException) e).getResult() - : RefUpdate.Result.LOCK_FAILURE; eventDispatcher .get() .postEvent( @@ -136,7 +131,7 @@ refName, sourceUri, ReplicationState.RefFetchResult.FAILED, - refUpdateResult)); + RefUpdate.Result.LOCK_FAILURE)); String message = String.format( "RefUpdate lock failure for: sourceLabel=%s, project=%s, refName=%s", @@ -175,19 +170,4 @@ return new RefUpdateState(ref.getName(), result); } } - - private static RefUpdate.Result ensureSuccess(RefUpdateState refUpdateState) - throws DeleteRefException { - switch (refUpdateState.getResult()) { - case NOT_ATTEMPTED: - case REJECTED: - case REJECTED_CURRENT_BRANCH: - case REJECTED_MISSING_OBJECT: - case LOCK_FAILURE: - case IO_FAILURE: - case REJECTED_OTHER_REASON: - throw new DeleteRefException("Failed ref deletion", refUpdateState.getResult()); - } - return refUpdateState.getResult(); - } }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/exception/DeleteRefException.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/exception/DeleteRefException.java deleted file mode 100644 index a1d6876..0000000 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/exception/DeleteRefException.java +++ /dev/null
@@ -1,33 +0,0 @@ -// Copyright (C) 2024 The Android Open Source Project -// -// 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 com.googlesource.gerrit.plugins.replication.pull.api.exception; - -import java.io.IOException; -import org.eclipse.jgit.lib.RefUpdate; - -public class DeleteRefException extends IOException { - - private static final long serialVersionUID = 1L; - private final RefUpdate.Result result; - - public DeleteRefException(String msg, RefUpdate.Result result) { - super(msg); - this.result = result; - } - - public RefUpdate.Result getResult() { - return result; - } -}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/DeleteRefCommandTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/DeleteRefCommandTest.java index 213deec..9361a77 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/DeleteRefCommandTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/api/DeleteRefCommandTest.java
@@ -15,7 +15,6 @@ package com.googlesource.gerrit.plugins.replication.pull.api; import static com.google.common.truth.Truth.assertThat; -import static com.google.gerrit.testing.GerritJUnit.assertThrows; import static org.mockito.Mockito.any; import static org.mockito.Mockito.anyString; import static org.mockito.Mockito.never; @@ -38,11 +37,9 @@ import com.googlesource.gerrit.plugins.replication.pull.FetchRefReplicatedEvent; import com.googlesource.gerrit.plugins.replication.pull.LocalGitRepositoryManagerProvider; import com.googlesource.gerrit.plugins.replication.pull.PullReplicationStateLogger; -import com.googlesource.gerrit.plugins.replication.pull.ReplicationState; import com.googlesource.gerrit.plugins.replication.pull.Source; import com.googlesource.gerrit.plugins.replication.pull.SourcesCollection; import com.googlesource.gerrit.plugins.replication.pull.fetch.ApplyObject; -import java.io.IOException; import java.util.Optional; import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.RefDatabase; @@ -118,7 +115,12 @@ objectUnderTest.deleteRef(TEST_PROJECT_NAME, TEST_REF_NAME, TEST_SOURCE_LABEL); - assertFetchReplicatedEvent(ReplicationState.RefFetchResult.SUCCEEDED, Result.FORCED); + verify(eventDispatcher).postEvent(eventCaptor.capture()); + Event sentEvent = eventCaptor.getValue(); + assertThat(sentEvent).isInstanceOf(FetchRefReplicatedEvent.class); + FetchRefReplicatedEvent fetchEvent = (FetchRefReplicatedEvent) sentEvent; + assertThat(fetchEvent.getProjectNameKey()).isEqualTo(TEST_PROJECT_NAME); + assertThat(fetchEvent.getRefName()).isEqualTo(TEST_REF_NAME); } @Test @@ -139,28 +141,4 @@ verify(eventDispatcher, never()).postEvent(any()); } - - @Test - public void shouldThrowWhenRefDeletionFails() throws Exception { - when(source.isMirror()).thenReturn(true); - when(refUpdate.delete()).thenReturn(Result.LOCK_FAILURE); - - assertThrows( - IOException.class, - () -> objectUnderTest.deleteRef(TEST_PROJECT_NAME, TEST_REF_NAME, TEST_SOURCE_LABEL)); - - assertFetchReplicatedEvent(ReplicationState.RefFetchResult.FAILED, Result.LOCK_FAILURE); - } - - private void assertFetchReplicatedEvent( - ReplicationState.RefFetchResult refFetchResult, RefUpdate.Result result) throws Exception { - verify(eventDispatcher).postEvent(eventCaptor.capture()); - Event sentEvent = eventCaptor.getValue(); - assertThat(sentEvent).isInstanceOf(FetchRefReplicatedEvent.class); - FetchRefReplicatedEvent fetchEvent = (FetchRefReplicatedEvent) sentEvent; - assertThat(fetchEvent.getProjectNameKey()).isEqualTo(TEST_PROJECT_NAME); - assertThat(fetchEvent.getRefName()).isEqualTo(TEST_REF_NAME); - assertThat(fetchEvent.getStatus()).isEqualTo(refFetchResult.toString()); - assertThat(fetchEvent.getRefUpdateResult()).isEqualTo(result); - } }