Remove custom servlet and associated tests
The custom servlet is redundant since the implementation of the REST
endpoint using the RestApiServlet. Remove it and the associated tests.
Also adjust the integration test to use the new endpoint.
Change-Id: I16af77e6cfef149deae81c9c40c537dfbd78dbc9
diff --git a/BUILD b/BUILD
index da50b99..416a3b6 100644
--- a/BUILD
+++ b/BUILD
@@ -33,6 +33,5 @@
visibility = ["//visibility:public"],
exports = PLUGIN_DEPS + PLUGIN_TEST_DEPS + [
":readonly__plugin",
- "@mockito//jar",
],
)
diff --git a/WORKSPACE b/WORKSPACE
index fd32218..276cebb 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -24,7 +24,3 @@
# Load snapshot Plugin API
#gerrit_api_maven_local()
-
-load("//:external_plugin_deps.bzl", "external_plugin_deps")
-
-external_plugin_deps()
diff --git a/external_plugin_deps.bzl b/external_plugin_deps.bzl
deleted file mode 100644
index 6650dc0..0000000
--- a/external_plugin_deps.bzl
+++ /dev/null
@@ -1,33 +0,0 @@
-load("//tools/bzl:maven_jar.bzl", "maven_jar")
-
-def external_plugin_deps():
- maven_jar(
- name = "mockito",
- artifact = "org.mockito:mockito-core:2.21.0",
- sha1 = "cdd1d0d5b2edbd2a7040735ccf88318c031f458b",
- deps = [
- "@byte_buddy//jar",
- "@byte_buddy_agent//jar",
- "@objenesis//jar",
- ],
- )
-
- BYTE_BUDDY_VER = "1.8.15"
-
- maven_jar(
- name = "byte_buddy",
- artifact = "net.bytebuddy:byte-buddy:" + BYTE_BUDDY_VER,
- sha1 = "cb36fe3c70ead5fcd016856a7efff908402d86b8",
- )
-
- maven_jar(
- name = "byte_buddy_agent",
- artifact = "net.bytebuddy:byte-buddy-agent:" + BYTE_BUDDY_VER,
- sha1 = "a2dbe3457401f65ad4022617fbb3fc0e5f427c7d",
- )
-
- maven_jar(
- name = "objenesis",
- artifact = "org.objenesis:objenesis:2.6",
- sha1 = "639033469776fd37c08358c6b92a4761feb2af4b",
- )
diff --git a/src/main/java/com/googlesource/gerrit/plugins/readonly/Constants.java b/src/main/java/com/googlesource/gerrit/plugins/readonly/Constants.java
deleted file mode 100644
index aca4da0..0000000
--- a/src/main/java/com/googlesource/gerrit/plugins/readonly/Constants.java
+++ /dev/null
@@ -1,20 +0,0 @@
-// Copyright (C) 2018 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.readonly;
-
-public class Constants {
- public static final String ENDPOINT = "/readonly";
- public static final String GIT_UPLOAD_PACK_PROTOCOL = "/git-upload-pack";
-}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/readonly/HttpModule.java b/src/main/java/com/googlesource/gerrit/plugins/readonly/HttpModule.java
index 4ae1374..be76f40 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/readonly/HttpModule.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/readonly/HttpModule.java
@@ -22,7 +22,6 @@
public class HttpModule extends HttpPluginModule {
@Override
protected void configureServlets() {
- install(ReadOnlyServlet.module());
DynamicSet.bind(binder(), AllRequestFilter.class).to(ReadOnly.class).in(Scopes.SINGLETON);
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/readonly/ReadOnly.java b/src/main/java/com/googlesource/gerrit/plugins/readonly/ReadOnly.java
index 824ddc1..3e2d585 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/readonly/ReadOnly.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/readonly/ReadOnly.java
@@ -14,8 +14,6 @@
package com.googlesource.gerrit.plugins.readonly;
-import static com.googlesource.gerrit.plugins.readonly.Constants.ENDPOINT;
-import static com.googlesource.gerrit.plugins.readonly.Constants.GIT_UPLOAD_PACK_PROTOCOL;
import static javax.servlet.http.HttpServletResponse.SC_SERVICE_UNAVAILABLE;
import com.google.common.collect.ImmutableList;
@@ -38,17 +36,17 @@
@Singleton
class ReadOnly extends AllRequestFilter implements CommitValidationListener {
+ private static final String GIT_UPLOAD_PACK_PROTOCOL = "/git-upload-pack";
+
private final ReadOnlyState state;
private final ReadOnlyConfig config;
private final String endpoint;
- private final String endpoint2;
@Inject
ReadOnly(ReadOnlyState state, ReadOnlyConfig config, @PluginName String pluginName) {
this.state = state;
this.config = config;
- this.endpoint = pluginName + ENDPOINT;
- this.endpoint2 = String.format("/config/server/%s~readonly", pluginName);
+ this.endpoint = String.format("/config/server/%s~readonly", pluginName);
}
@Override
@@ -77,7 +75,6 @@
String method = request.getMethod();
String servletPath = request.getServletPath();
return !servletPath.endsWith(endpoint)
- && !servletPath.endsWith(endpoint2)
&& (("POST".equals(method) && !servletPath.endsWith(GIT_UPLOAD_PACK_PROTOCOL))
|| "PUT".equals(method)
|| "DELETE".equals(method));
diff --git a/src/main/java/com/googlesource/gerrit/plugins/readonly/ReadOnlyServlet.java b/src/main/java/com/googlesource/gerrit/plugins/readonly/ReadOnlyServlet.java
deleted file mode 100644
index 39b2fce..0000000
--- a/src/main/java/com/googlesource/gerrit/plugins/readonly/ReadOnlyServlet.java
+++ /dev/null
@@ -1,105 +0,0 @@
-// Copyright (C) 2018 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.readonly;
-
-import static com.googlesource.gerrit.plugins.readonly.Constants.ENDPOINT;
-import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN;
-import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
-import static javax.servlet.http.HttpServletResponse.SC_NO_CONTENT;
-import static javax.servlet.http.HttpServletResponse.SC_SERVICE_UNAVAILABLE;
-
-import com.google.gerrit.httpd.plugins.HttpPluginModule;
-import com.google.gerrit.server.CurrentUser;
-import com.google.inject.Inject;
-import com.google.inject.Provider;
-import com.google.inject.Singleton;
-import java.io.IOException;
-import javax.servlet.http.HttpServlet;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-@Singleton
-public class ReadOnlyServlet extends HttpServlet {
- private static final Logger log = LoggerFactory.getLogger(ReadOnlyServlet.class);
- private static final long serialVersionUID = -1L;
-
- private final Provider<CurrentUser> user;
- private final ReadOnlyState state;
-
- public static HttpPluginModule module() {
- return new HttpPluginModule() {
- @Override
- protected void configureServlets() {
- serve(ENDPOINT).with(ReadOnlyServlet.class);
- }
- };
- }
-
- @Inject
- ReadOnlyServlet(Provider<CurrentUser> user, ReadOnlyState state) {
- this.user = user;
- this.state = state;
- }
-
- @Override
- protected void doGet(HttpServletRequest req, HttpServletResponse rsp) {
- if (state.isReadOnly()) {
- sendError(rsp, SC_SERVICE_UNAVAILABLE);
- return;
- }
- rsp.setStatus(SC_NO_CONTENT);
- }
-
- @Override
- protected void doPost(HttpServletRequest req, HttpServletResponse rsp) {
- if (!user.get().getCapabilities().canAdministrateServer()) {
- sendError(rsp, SC_FORBIDDEN);
- return;
- }
- try {
- state.setReadOnly(true);
- rsp.setStatus(SC_NO_CONTENT);
- } catch (IOException e) {
- log.error("Failed to enable read-only mode", e);
- sendError(rsp, SC_INTERNAL_SERVER_ERROR);
- }
- }
-
- @Override
- protected void doDelete(HttpServletRequest req, HttpServletResponse rsp) {
- if (!user.get().getCapabilities().canAdministrateServer()) {
- sendError(rsp, SC_FORBIDDEN);
- return;
- }
- try {
- state.setReadOnly(false);
- rsp.setStatus(SC_NO_CONTENT);
- } catch (IOException e) {
- log.error("Failed to disable read-only mode", e);
- sendError(rsp, SC_INTERNAL_SERVER_ERROR);
- }
- }
-
- private static void sendError(HttpServletResponse rsp, int statusCode) {
- try {
- rsp.sendError(statusCode);
- } catch (IOException e) {
- rsp.setStatus(SC_INTERNAL_SERVER_ERROR);
- log.error("Failed to send error response", e);
- }
- }
-}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/readonly/ReadOnlyIT.java b/src/test/java/com/googlesource/gerrit/plugins/readonly/ReadOnlyIT.java
index 0df182b..55a7641 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/readonly/ReadOnlyIT.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/readonly/ReadOnlyIT.java
@@ -161,9 +161,9 @@
private void setReadOnly(boolean readOnly) throws Exception {
if (readOnly) {
- adminRestSession.post("/plugins/readonly/readonly").assertNoContent();
+ adminRestSession.put("/config/server/readonly~readonly").assertOK();
} else {
- adminRestSession.delete("/plugins/readonly/readonly").assertNoContent();
+ adminRestSession.delete("/config/server/readonly~readonly").assertOK();
}
}
}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/readonly/ReadOnlyServletTest.java b/src/test/java/com/googlesource/gerrit/plugins/readonly/ReadOnlyServletTest.java
deleted file mode 100644
index 2d5357f..0000000
--- a/src/test/java/com/googlesource/gerrit/plugins/readonly/ReadOnlyServletTest.java
+++ /dev/null
@@ -1,170 +0,0 @@
-// Copyright (C) 2018 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.readonly;
-
-import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN;
-import static javax.servlet.http.HttpServletResponse.SC_INTERNAL_SERVER_ERROR;
-import static javax.servlet.http.HttpServletResponse.SC_NO_CONTENT;
-import static javax.servlet.http.HttpServletResponse.SC_SERVICE_UNAVAILABLE;
-import static org.mockito.Mockito.doThrow;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
-
-import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.account.CapabilityControl;
-import com.google.gerrit.server.config.SitePaths;
-import com.google.inject.Provider;
-import java.io.IOException;
-import javax.servlet.http.HttpServletResponse;
-import org.junit.Before;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.rules.TemporaryFolder;
-import org.junit.runner.RunWith;
-import org.mockito.Mock;
-import org.mockito.junit.MockitoJUnitRunner;
-
-@RunWith(MockitoJUnitRunner.class)
-public class ReadOnlyServletTest {
- @Rule public TemporaryFolder tempFolder = new TemporaryFolder();
-
- @Mock private Provider<CurrentUser> currentUserProviderMock;
- @Mock private CurrentUser currentUserMock;
- @Mock private CapabilityControl capabilityControlMock;
-
- private ReadOnlyServlet servlet;
- private SitePaths site;
-
- @Before
- public void setUp() throws Exception {
- when(currentUserProviderMock.get()).thenReturn(currentUserMock);
- when(currentUserMock.getCapabilities()).thenReturn(capabilityControlMock);
- when(capabilityControlMock.canAdministrateServer()).thenReturn(true);
- site = new SitePaths(tempFolder.getRoot().toPath());
- tempFolder.newFolder("etc");
- ReadOnlyState state = new ReadOnlyState(site);
- servlet = new ReadOnlyServlet(currentUserProviderMock, state);
- }
-
- @Test
- public void notReadOnlyByDefault() throws Exception {
- assertNotReadOnly();
- }
-
- @Test
- public void enableReadOnly() throws Exception {
- assertNotReadOnly();
-
- // enable read-only
- HttpServletResponse responseMock = mock(HttpServletResponse.class);
- servlet.doPost(null, responseMock);
- verify(responseMock).setStatus(SC_NO_CONTENT);
- assertIsReadOnly();
-
- // enabling read-only again should not change anything
- responseMock = mock(HttpServletResponse.class);
- servlet.doPost(null, responseMock);
- verify(responseMock).setStatus(SC_NO_CONTENT);
- assertIsReadOnly();
- }
-
- @Test
- public void enableReadOnlyByNonAdmins() throws Exception {
- assertNotReadOnly();
-
- when(capabilityControlMock.canAdministrateServer()).thenReturn(false);
- HttpServletResponse responseMock = mock(HttpServletResponse.class);
- servlet.doDelete(null, responseMock);
- verify(responseMock).sendError(SC_FORBIDDEN);
- assertNotReadOnly();
- }
-
- @Test
- public void errorDuringEnableReadOnly() throws Exception {
- // remove site dir to create an IOException
- tempFolder.delete();
-
- HttpServletResponse responseMock = mock(HttpServletResponse.class);
- servlet.doPost(null, responseMock);
- verify(responseMock).sendError(SC_INTERNAL_SERVER_ERROR);
- }
-
- @Test
- public void disableReadOnly() throws Exception {
- // first, mark as read-only
- servlet.doPost(null, mock(HttpServletResponse.class));
- assertIsReadOnly();
-
- // disable read-only
- HttpServletResponse responseMock = mock(HttpServletResponse.class);
- servlet.doDelete(null, responseMock);
- verify(responseMock).setStatus(SC_NO_CONTENT);
- assertNotReadOnly();
-
- // disabling read-only again should not change anything
- responseMock = mock(HttpServletResponse.class);
- servlet.doDelete(null, responseMock);
- verify(responseMock).setStatus(SC_NO_CONTENT);
- assertNotReadOnly();
- }
-
- @Test
- public void disableReadOnlyByNonAdmins() throws Exception {
- // first, mark as read-only
- servlet.doPost(null, mock(HttpServletResponse.class));
- assertIsReadOnly();
-
- when(capabilityControlMock.canAdministrateServer()).thenReturn(false);
- HttpServletResponse responseMock = mock(HttpServletResponse.class);
- servlet.doDelete(null, responseMock);
- verify(responseMock).sendError(SC_FORBIDDEN);
- assertIsReadOnly();
- }
-
- @Test
- public void errorDuringDisableReadOnly() throws Exception {
- // Create gerrit.readonly as a folder with content to create an IOException
- site.etc_dir.resolve("gerrit.readonly").resolve("child").toFile().mkdirs();
-
- HttpServletResponse responseMock = mock(HttpServletResponse.class);
- servlet.doDelete(null, responseMock);
- verify(responseMock).sendError(SC_INTERNAL_SERVER_ERROR);
- }
-
- @Test
- public void errorWhileSendingReadOnlyStatusResponse() throws IOException {
- HttpServletResponse responseMock = mock(HttpServletResponse.class);
- servlet.doPost(null, responseMock);
- verify(responseMock).setStatus(SC_NO_CONTENT);
-
- responseMock = mock(HttpServletResponse.class);
- doThrow(new IOException("someError")).when(responseMock).sendError(SC_SERVICE_UNAVAILABLE);
- servlet.doGet(null, responseMock);
- verify(responseMock).setStatus(SC_INTERNAL_SERVER_ERROR);
- }
-
- private void assertNotReadOnly() throws Exception {
- HttpServletResponse responseMock = mock(HttpServletResponse.class);
- servlet.doGet(null, responseMock);
- verify(responseMock).setStatus(SC_NO_CONTENT);
- }
-
- private void assertIsReadOnly() throws Exception {
- HttpServletResponse responseMock = mock(HttpServletResponse.class);
- servlet.doGet(null, responseMock);
- verify(responseMock).sendError(SC_SERVICE_UNAVAILABLE);
- }
-}
