Merge branch 'stable-3.9' * stable-3.9: Pull-replication plugin should warn about inconsistent timeouts Change-Id: Ifc22812bc9edc16ea757ef0a50669b5a6c0bf260
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/ReplicationQueue.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/ReplicationQueue.java index 2eb26e8..baeb330 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/ReplicationQueue.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/ReplicationQueue.java
@@ -774,7 +774,7 @@ private void fireBeforeStartupEvents() { Set<String> eventsReplayed = new HashSet<>(); ReferenceBatchUpdatedEvent event; - while ((event = beforeStartupEventsQueue.poll()) != null) { + while ((event = beforeStartupEventsQueue.peek()) != null) { String eventKey = String.format( "%s:%s", @@ -787,6 +787,7 @@ fire(event); eventsReplayed.add(eventKey); } + beforeStartupEventsQueue.remove(event); } }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ProjectInitializationAction.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ProjectInitializationAction.java index 6c24cbc..a69afa4 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ProjectInitializationAction.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/ProjectInitializationAction.java
@@ -27,12 +27,15 @@ import com.google.common.net.MediaType; import com.google.gerrit.entities.Project; import com.google.gerrit.entities.RefNames; +import com.google.gerrit.extensions.events.NewProjectCreatedListener; +import com.google.gerrit.extensions.registration.DynamicSet; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.Url; import com.google.gerrit.index.project.ProjectIndexer; import com.google.gerrit.server.CurrentUser; +import com.google.gerrit.server.extensions.events.AbstractNoNotifyEvent; import com.google.gerrit.server.permissions.GlobalPermission; import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.permissions.PermissionBackendException; @@ -70,6 +73,7 @@ private final ProjectIndexer projectIndexer; private final ApplyObjectCommand applyObjectCommand; private final ProjectCache projectCache; + private final DynamicSet<NewProjectCreatedListener> newProjectCreatedListeners; @Inject ProjectInitializationAction( @@ -78,13 +82,15 @@ PermissionBackend permissionBackend, ProjectIndexer projectIndexer, ApplyObjectCommand applyObjectCommand, - ProjectCache projectCache) { + ProjectCache projectCache, + DynamicSet<NewProjectCreatedListener> newProjectCreatedListeners) { this.gerritConfigOps = gerritConfigOps; this.userProvider = userProvider; this.permissionBackend = permissionBackend; this.projectIndexer = projectIndexer; this.applyObjectCommand = applyObjectCommand; this.projectCache = projectCache; + this.newProjectCreatedListeners = newProjectCreatedListeners; } @Override @@ -179,6 +185,10 @@ throw new BadRequestException("Configuration data cannot contain change meta refs", e); } projectCache.onCreateProject(Project.nameKey(projectName)); + // In case pull-replication is used in conjunction with multi-site, by convention the remote + // label is populated with the instanceId. That's why we are passing input.getLabel() + // to the Event to notify + notifyListenersOfNewProjectCreation(projectName, input.getLabel()); repLog.info( "Init project API from {} for {}:{} - {}", input.getLabel(), @@ -246,4 +256,38 @@ return false; } } + + private void notifyListenersOfNewProjectCreation(String projectName, String instanceId) { + NewProjectCreatedListener.Event newProjectCreatedEvent = + new Event(RefNames.REFS_CONFIG, projectName, instanceId); + newProjectCreatedListeners.forEach(l -> l.onNewProjectCreated(newProjectCreatedEvent)); + } + + private static class Event extends AbstractNoNotifyEvent + implements NewProjectCreatedListener.Event { + private final String headName; + private final String projectName; + private final String instanceId; + + public Event(String headName, String projectName, String maybeInstanceId) { + this.headName = headName; + this.projectName = projectName; + this.instanceId = maybeInstanceId; + } + + @Override + public String getHeadName() { + return headName; + } + + @Override + public String getProjectName() { + return projectName; + } + + @Override + public String getInstanceId() { + return instanceId; + } + } }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/UpdateHeadAction.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/UpdateHeadAction.java index 800f2b2..9a3b4fa 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/UpdateHeadAction.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/UpdateHeadAction.java
@@ -15,56 +15,31 @@ package com.googlesource.gerrit.plugins.replication.pull.api; import com.google.common.base.Strings; -import com.google.common.flogger.FluentLogger; -import com.google.gerrit.entities.Project; import com.google.gerrit.entities.RefNames; import com.google.gerrit.extensions.api.projects.HeadInput; -import com.google.gerrit.extensions.registration.DynamicItem; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.ResourceConflictException; -import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.Response; import com.google.gerrit.extensions.restapi.RestModifyView; -import com.google.gerrit.extensions.restapi.UnprocessableEntityException; -import com.google.gerrit.server.events.EventDispatcher; -import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.server.project.ProjectResource; import com.google.inject.Inject; import com.google.inject.Singleton; -import com.googlesource.gerrit.plugins.replication.LocalFS; -import com.googlesource.gerrit.plugins.replication.pull.Context; -import com.googlesource.gerrit.plugins.replication.pull.FetchRefReplicatedEvent; -import com.googlesource.gerrit.plugins.replication.pull.GerritConfigOps; -import com.googlesource.gerrit.plugins.replication.pull.ReplicationState; -import java.net.HttpURLConnection; -import java.util.Optional; -import org.eclipse.jgit.lib.RefUpdate; -import org.eclipse.jgit.transport.URIish; @Singleton public class UpdateHeadAction implements RestModifyView<ProjectResource, HeadInput> { - private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - private final GerritConfigOps gerritConfigOps; private final FetchPreconditions preconditions; - private final DynamicItem<EventDispatcher> eventDispatcher; - private static final URIish EMPTY_URI = new URIish(); + private final UpdateHeadCommand updateHeadCommand; @Inject - UpdateHeadAction( - GerritConfigOps gerritConfigOps, - FetchPreconditions preconditions, - DynamicItem<EventDispatcher> eventDispatcher) { - this.gerritConfigOps = gerritConfigOps; + UpdateHeadAction(FetchPreconditions preconditions, UpdateHeadCommand updateHeadCommand) { this.preconditions = preconditions; - this.eventDispatcher = eventDispatcher; + this.updateHeadCommand = updateHeadCommand; } @Override public Response<?> apply(ProjectResource projectResource, HeadInput input) throws AuthException, BadRequestException, ResourceConflictException, Exception { - Response<String> res = null; - if (input == null || Strings.isNullOrEmpty(input.ref)) { throw new BadRequestException("ref required"); } @@ -74,50 +49,8 @@ throw new AuthException("Update head not permitted"); } - // TODO: the .git suffix should not be added here, but rather it should be - // dealt with by the caller, honouring the naming style from the - // replication.config (Issue 15221) - Optional<URIish> maybeRepo = - gerritConfigOps.getGitRepositoryURI(String.format("%s.git", projectResource.getName())); + updateHeadCommand.doUpdate(projectResource.getNameKey(), ref); - try { - if (maybeRepo.isPresent()) { - if (new LocalFS(maybeRepo.get()).updateHead(projectResource.getNameKey(), ref)) { - return res = Response.ok(ref); - } - throw new UnprocessableEntityException( - String.format( - "Could not update HEAD of repo %s to ref %s", projectResource.getName(), ref)); - } - } finally { - fireEvent( - projectResource.getNameKey(), - res != null && res.statusCode() == HttpURLConnection.HTTP_OK); - } - throw new ResourceNotFoundException( - String.format("Could not compute URL for repo: %s", projectResource.getName())); - } - - private void fireEvent(Project.NameKey projectName, boolean succeeded) { - try { - Context.setLocalEvent(true); - eventDispatcher - .get() - .postEvent( - new FetchRefReplicatedEvent( - projectName.get(), - RefNames.HEAD, - EMPTY_URI, // TODO: the remote label is not passed as parameter, hence cannot be - // propagated to the event - succeeded - ? ReplicationState.RefFetchResult.SUCCEEDED - : ReplicationState.RefFetchResult.FAILED, - RefUpdate.Result.FORCED)); - } catch (PermissionBackendException e) { - logger.atSevere().withCause(e).log( - "Cannot post event for refs/meta/config on project %s", projectName); - } finally { - Context.unsetLocalEvent(); - } + return Response.ok(ref); } }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/UpdateHeadCommand.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/UpdateHeadCommand.java new file mode 100644 index 0000000..53bf5d9 --- /dev/null +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/api/UpdateHeadCommand.java
@@ -0,0 +1,99 @@ +// Copyright (C) 2023 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; + +import com.google.common.flogger.FluentLogger; +import com.google.gerrit.entities.Project; +import com.google.gerrit.entities.RefNames; +import com.google.gerrit.extensions.registration.DynamicItem; +import com.google.gerrit.extensions.restapi.ResourceNotFoundException; +import com.google.gerrit.extensions.restapi.UnprocessableEntityException; +import com.google.gerrit.server.events.EventDispatcher; +import com.google.gerrit.server.permissions.PermissionBackendException; +import com.google.inject.Singleton; +import com.googlesource.gerrit.plugins.replication.LocalFS; +import com.googlesource.gerrit.plugins.replication.pull.Context; +import com.googlesource.gerrit.plugins.replication.pull.FetchRefReplicatedEvent; +import com.googlesource.gerrit.plugins.replication.pull.GerritConfigOps; +import com.googlesource.gerrit.plugins.replication.pull.ReplicationState; +import java.util.Optional; +import javax.inject.Inject; +import org.eclipse.jgit.lib.RefUpdate; +import org.eclipse.jgit.transport.URIish; + +@Singleton +public class UpdateHeadCommand { + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + private static final URIish EMPTY_URI = new URIish(); + + private final GerritConfigOps gerritConfigOps; + private final DynamicItem<EventDispatcher> eventDispatcher; + + @Inject + UpdateHeadCommand(GerritConfigOps gerritConfigOps, DynamicItem<EventDispatcher> eventDispatcher) { + this.gerritConfigOps = gerritConfigOps; + this.eventDispatcher = eventDispatcher; + } + + public void doUpdate(Project.NameKey project, String ref) + throws UnprocessableEntityException, ResourceNotFoundException { + boolean succeeded = false; + // TODO: the .git suffix should not be added here, but rather it should be + // dealt with by the caller, honouring the naming style from the + // replication.config (Issue 15221) + Optional<URIish> maybeRepo = + gerritConfigOps.getGitRepositoryURI(String.format("%s.git", project.get())); + + logger.atInfo().log("do update: %s %s", project, ref); + try { + if (maybeRepo.isPresent()) { + if (new LocalFS(maybeRepo.get()).updateHead(project, ref)) { + succeeded = true; + return; + } + + throw new UnprocessableEntityException( + String.format("Could not update HEAD of repo %s to ref %s", project, ref)); + } + } finally { + fireEvent(project, succeeded); + } + throw new ResourceNotFoundException( + String.format("Could not compute URL for repo: %s", project.get())); + } + + private void fireEvent(Project.NameKey projectName, boolean succeeded) { + try { + Context.setLocalEvent(true); + eventDispatcher + .get() + .postEvent( + new FetchRefReplicatedEvent( + projectName.get(), + RefNames.HEAD, + EMPTY_URI, // TODO: the remote label is not passed as parameter, hence cannot be + // propagated to the event + succeeded + ? ReplicationState.RefFetchResult.SUCCEEDED + : ReplicationState.RefFetchResult.FAILED, + RefUpdate.Result.FORCED)); + } catch (PermissionBackendException e) { + logger.atSevere().withCause(e).log( + "Cannot post event for refs/meta/config on project %s", projectName); + } finally { + Context.unsetLocalEvent(); + } + } +}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/event/EventsBrokerMessageConsumer.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/event/EventsBrokerMessageConsumer.java index a9b4945..cf7abba 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/event/EventsBrokerMessageConsumer.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/event/EventsBrokerMessageConsumer.java
@@ -23,6 +23,8 @@ import com.google.gerrit.extensions.events.LifecycleListener; import com.google.gerrit.extensions.registration.DynamicItem; import com.google.gerrit.extensions.restapi.AuthException; +import com.google.gerrit.extensions.restapi.ResourceNotFoundException; +import com.google.gerrit.extensions.restapi.UnprocessableEntityException; import com.google.gerrit.server.events.Event; import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.inject.Inject; @@ -58,7 +60,11 @@ try { eventListener.fetchRefsForEvent(event); if (shutdownState.isShuttingDown()) stop(); - } catch (AuthException | PermissionBackendException | IOException e) { + } catch (AuthException + | PermissionBackendException + | IOException + | UnprocessableEntityException + | ResourceNotFoundException e) { throw new EventRejectedException(event, e); } }
diff --git a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/event/StreamEventListener.java b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/event/StreamEventListener.java index 4c621ac..2f992cd 100644 --- a/src/main/java/com/googlesource/gerrit/plugins/replication/pull/event/StreamEventListener.java +++ b/src/main/java/com/googlesource/gerrit/plugins/replication/pull/event/StreamEventListener.java
@@ -24,13 +24,16 @@ import com.google.gerrit.entities.Project.NameKey; import com.google.gerrit.entities.RefNames; import com.google.gerrit.extensions.restapi.AuthException; +import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.RestApiException; +import com.google.gerrit.extensions.restapi.UnprocessableEntityException; import com.google.gerrit.server.config.GerritInstanceId; import com.google.gerrit.server.data.RefUpdateAttribute; import com.google.gerrit.server.events.Event; import com.google.gerrit.server.events.EventListener; import com.google.gerrit.server.events.ProjectCreatedEvent; import com.google.gerrit.server.events.ProjectEvent; +import com.google.gerrit.server.events.ProjectHeadUpdatedEvent; import com.google.gerrit.server.events.RefUpdatedEvent; import com.google.gerrit.server.git.WorkQueue; import com.google.gerrit.server.permissions.PermissionBackendException; @@ -47,6 +50,7 @@ import com.googlesource.gerrit.plugins.replication.pull.api.FetchJob.Factory; import com.googlesource.gerrit.plugins.replication.pull.api.ProjectInitializationAction; import com.googlesource.gerrit.plugins.replication.pull.api.PullReplicationApiRequestMetrics; +import com.googlesource.gerrit.plugins.replication.pull.api.UpdateHeadCommand; import com.googlesource.gerrit.plugins.replication.pull.filter.ExcludedRefsFilter; import java.io.IOException; import java.util.Optional; @@ -59,6 +63,7 @@ private final DeleteRefCommand deleteCommand; private final ExcludedRefsFilter refsFilter; private final Factory fetchJobFactory; + private final UpdateHeadCommand updateHeadCommand; private final ProjectInitializationAction projectInitializationAction; private final Provider<PullReplicationApiRequestMetrics> metricsProvider; private final SourcesCollection sources; @@ -70,6 +75,7 @@ public StreamEventListener( @Nullable @GerritInstanceId String instanceId, DeleteRefCommand deleteCommand, + UpdateHeadCommand updateHeadCommand, ProjectInitializationAction projectInitializationAction, WorkQueue workQueue, FetchJob.Factory fetchJobFactory, @@ -79,6 +85,7 @@ @Named(APPLY_OBJECTS_CACHE) Cache<ApplyObjectsCacheKey, Long> refUpdatesSucceededCache) { this.instanceId = instanceId; this.deleteCommand = deleteCommand; + this.updateHeadCommand = updateHeadCommand; this.projectInitializationAction = projectInitializationAction; this.workQueue = workQueue; this.fetchJobFactory = fetchJobFactory; @@ -95,7 +102,11 @@ public void onEvent(Event event) { try { fetchRefsForEvent(event); - } catch (AuthException | PermissionBackendException | IOException e) { + } catch (AuthException + | PermissionBackendException + | IOException + | UnprocessableEntityException + | ResourceNotFoundException e) { logger.atSevere().withCause(e).log( "This is the event handler of Gerrit's event-bus. It isn't" + "supposed to throw any exception, otherwise the other handlers " @@ -104,7 +115,8 @@ } public void fetchRefsForEvent(Event event) - throws AuthException, PermissionBackendException, IOException { + throws AuthException, PermissionBackendException, IOException, UnprocessableEntityException, + ResourceNotFoundException { if (instanceId.equals(event.instanceId) || !shouldReplicateProject(event)) { return; } @@ -159,6 +171,15 @@ "Cannot initialise project:%s", projectCreatedEvent.projectName); throw e; } + } else if (event instanceof ProjectHeadUpdatedEvent) { + ProjectHeadUpdatedEvent headUpdatedEvent = (ProjectHeadUpdatedEvent) event; + try { + updateHeadCommand.doUpdate(headUpdatedEvent.getProjectNameKey(), headUpdatedEvent.newHead); + } catch (UnprocessableEntityException | ResourceNotFoundException e) { + logger.atSevere().withCause(e).log( + "Failed to update HEAD on project: %s", headUpdatedEvent.projectName); + throw e; + } } }
diff --git a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/event/StreamEventListenerTest.java b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/event/StreamEventListenerTest.java index 552853e..e91ce8d 100644 --- a/src/test/java/com/googlesource/gerrit/plugins/replication/pull/event/StreamEventListenerTest.java +++ b/src/test/java/com/googlesource/gerrit/plugins/replication/pull/event/StreamEventListenerTest.java
@@ -29,6 +29,7 @@ import com.google.gerrit.server.data.RefUpdateAttribute; import com.google.gerrit.server.events.Event; import com.google.gerrit.server.events.ProjectCreatedEvent; +import com.google.gerrit.server.events.ProjectHeadUpdatedEvent; import com.google.gerrit.server.events.RefUpdatedEvent; import com.google.gerrit.server.git.WorkQueue; import com.googlesource.gerrit.plugins.replication.pull.ApplyObjectsCacheKey; @@ -40,6 +41,7 @@ import com.googlesource.gerrit.plugins.replication.pull.api.FetchJob; import com.googlesource.gerrit.plugins.replication.pull.api.ProjectInitializationAction; import com.googlesource.gerrit.plugins.replication.pull.api.PullReplicationApiRequestMetrics; +import com.googlesource.gerrit.plugins.replication.pull.api.UpdateHeadCommand; import com.googlesource.gerrit.plugins.replication.pull.filter.ExcludedRefsFilter; import java.util.concurrent.ScheduledExecutorService; import org.eclipse.jgit.lib.ObjectId; @@ -66,6 +68,7 @@ @Mock private ScheduledExecutorService executor; @Mock private FetchJob fetchJob; @Mock private FetchJob.Factory fetchJobFactory; + @Mock private UpdateHeadCommand updateHeadCommand; @Mock private DeleteRefCommand deleteRefCommand; @Captor ArgumentCaptor<Input> inputCaptor; @Mock private PullReplicationApiRequestMetrics metrics; @@ -92,6 +95,7 @@ new StreamEventListener( INSTANCE_ID, deleteRefCommand, + updateHeadCommand, projectInitializationAction, workQueue, fetchJobFactory, @@ -311,4 +315,17 @@ verify(executor).submit(any(FetchJob.class)); } + + @Test + public void shouldUpdateProjectHeadOnProjectHeadUpdatedEvent() throws Exception { + ProjectHeadUpdatedEvent event = new ProjectHeadUpdatedEvent(); + event.projectName = TEST_PROJECT; + event.oldHead = "refs/heads/master"; + event.newHead = "refs/heads/main"; + event.instanceId = REMOTE_INSTANCE_ID; + + objectUnderTest.onEvent(event); + + verify(updateHeadCommand).doUpdate(event.getProjectNameKey(), event.newHead); + } }