Consider any HTTP 2xx response code from REST-API as success When executing remote REST-API for either ApplyObject or Fetch any 2xx status code (200, 201, 202, 203, 204, ...) should be considered as a completion and not reported as a failure or retried. Previously, the 202 was missed and potentially also others could be missed in the future. There is no reason for considering a 2xx as an unsuccessful result. Change-Id: Id2e55cfe441c73983001fc76156a47e5fe08d97c
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/client/HttpResult.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/client/HttpResult.java index 3efcacf..ec9d65f 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/client/HttpResult.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/client/HttpResult.java
@@ -15,9 +15,6 @@ package com.googlesource.gerrit.plugins.replication.pull.client; import static javax.servlet.http.HttpServletResponse.SC_CONFLICT; -import static javax.servlet.http.HttpServletResponse.SC_CREATED; -import static javax.servlet.http.HttpServletResponse.SC_NO_CONTENT; -import static javax.servlet.http.HttpServletResponse.SC_OK; import com.google.gerrit.entities.Project; import java.util.Optional; @@ -36,7 +33,7 @@ } public boolean isSuccessful() { - return responseCode == SC_CREATED || responseCode == SC_NO_CONTENT || responseCode == SC_OK; + return responseCode / 100 == 2; // Any 2xx response code is a success } public boolean isProjectMissing(Project.NameKey projectName) {
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/client/HttpResultTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/client/HttpResultTest.java new file mode 100644 index 0000000..d82f3a5 --- /dev/null +++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/client/HttpResultTest.java
@@ -0,0 +1,55 @@ +// Copyright (C) 2022 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.client; + +import static com.google.common.truth.Truth.assertThat; + +import java.util.Arrays; +import java.util.Optional; +import javax.servlet.http.HttpServletResponse; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +@RunWith(Parameterized.class) +public class HttpResultTest { + + @Parameterized.Parameters(name = "HTTP Status = {0} is successful: {1}") + public static Iterable<Object[]> data() { + return Arrays.asList( + new Object[][] { + {HttpServletResponse.SC_OK, true}, + {HttpServletResponse.SC_CREATED, true}, + {HttpServletResponse.SC_ACCEPTED, true}, + {HttpServletResponse.SC_NO_CONTENT, true}, + {HttpServletResponse.SC_BAD_REQUEST, false}, + {HttpServletResponse.SC_CONFLICT, false} + }); + } + + private Integer httpStatus; + private boolean isSuccessful; + + public HttpResultTest(Integer httpStatus, Boolean isSuccessful) { + this.httpStatus = httpStatus; + this.isSuccessful = isSuccessful; + } + + @Test + public void httpResultIsSuccessful() { + HttpResult httpResult = new HttpResult(httpStatus, Optional.empty()); + assertThat(httpResult.isSuccessful()).isEqualTo(isSuccessful); + } +}