Merge branch 'stable-2.16'

* stable-2.16:
  Add missing license/copyright headers
  Use '-' instead of spaces for owners label
  Removed redundant PLUGIN_DEPS from BUILD
  Format with GJF 1.7
  Format with GJF 1.7
  Add integration test for owners-autoassign
  Prevent infinite event processing with NoteDb
  Look for gerrit:/username: schemes for username
  Get rid of references to ReviewDb

Change-Id: Ie4e2420eb3cec37eaee83b2e8085d8ff25ffe4b7
diff --git a/owners-autoassign/BUILD b/owners-autoassign/BUILD
index 13aabbe..463afcb 100644
--- a/owners-autoassign/BUILD
+++ b/owners-autoassign/BUILD
@@ -1,4 +1,5 @@
-load("//tools/bzl:plugin.bzl", "gerrit_plugin")
+load("//tools/bzl:junit.bzl", "junit_tests")
+load("//tools/bzl:plugin.bzl", "PLUGIN_DEPS", "PLUGIN_DEPS_NEVERLINK", "PLUGIN_TEST_DEPS", "gerrit_plugin")
 
 gerrit_plugin(
     name = "owners-autoassign",
@@ -18,3 +19,24 @@
         "//owners-common",
     ],
 )
+
+java_library(
+    name = "owners-autoassign_deps",
+    srcs = glob([
+        "src/main/java/**/*.java",
+    ]),
+    visibility = ["//visibility:public"],
+    deps = PLUGIN_DEPS_NEVERLINK + [
+        "//owners-common",
+    ],
+)
+
+junit_tests(
+    name = "owners_autoassign_tests",
+    testonly = 1,
+    srcs = glob(["src/test/java/**/*.java"]),
+    deps = PLUGIN_DEPS + PLUGIN_TEST_DEPS + [
+        "//owners-common",
+        ":owners-autoassign_deps",
+    ],
+)
diff --git a/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/GitRefListener.java b/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/GitRefListener.java
index 09e087d..22e3db8 100644
--- a/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/GitRefListener.java
+++ b/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/GitRefListener.java
@@ -19,14 +19,18 @@
 import static com.google.gerrit.extensions.client.DiffPreferencesInfo.Whitespace.IGNORE_NONE;
 
 import com.google.common.collect.Sets;
-import com.google.gerrit.exceptions.StorageException;
 import com.google.gerrit.extensions.annotations.Listen;
+import com.google.gerrit.extensions.api.GerritApi;
+import com.google.gerrit.extensions.api.changes.ChangeApi;
+import com.google.gerrit.extensions.api.changes.Changes;
+import com.google.gerrit.extensions.common.ChangeInfo;
 import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
+import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.patch.PatchList;
 import com.google.gerrit.server.patch.PatchListCache;
 import com.google.gerrit.server.patch.PatchListKey;
@@ -43,24 +47,23 @@
 public class GitRefListener implements GitReferenceUpdatedListener {
   private static final Logger logger = LoggerFactory.getLogger(GitRefListener.class);
 
-  private static final String CHANGES_REF = "refs/changes/";
+  private final GerritApi api;
 
   private final PatchListCache patchListCache;
   private final GitRepositoryManager repositoryManager;
-  private final ChangeNotes.Factory notesFactory;
   private final Accounts accounts;
   private final ReviewerManager reviewerManager;
 
   @Inject
   public GitRefListener(
+      GerritApi api,
       PatchListCache patchListCache,
       GitRepositoryManager repositoryManager,
-      ChangeNotes.Factory notesFactory,
       Accounts accounts,
       ReviewerManager reviewerManager) {
+    this.api = api;
     this.patchListCache = patchListCache;
     this.repositoryManager = repositoryManager;
-    this.notesFactory = notesFactory;
     this.accounts = accounts;
     this.reviewerManager = reviewerManager;
   }
@@ -72,7 +75,10 @@
     try {
       repository = repositoryManager.openRepository(Project.NameKey.parse(projectName));
       try {
-        processEvent(repository, event);
+        String refName = event.getRefName();
+        if (refName.startsWith(RefNames.REFS_CHANGES) && !RefNames.isNoteDbMetaRef(refName)) {
+          processEvent(repository, event);
+        }
       } finally {
         repository.close();
       }
@@ -81,38 +87,36 @@
     }
   }
 
-  private void processEvent(Repository repository, Event event) {
-    if (event.getRefName().startsWith(CHANGES_REF)) {
-      Change.Id id = Change.Id.fromRef(event.getRefName());
-      try {
-        Change change =
-            notesFactory
-                .createChecked(Project.NameKey.parse(event.getProjectName()), id)
-                .getChange();
-        if (change == null) {
-          return;
-        }
-        PatchList patchList = getPatchList(event, change);
-        if (patchList != null) {
-          PathOwners owners =
-              new PathOwners(accounts, repository, change.getDest().get(), patchList);
-          Set<Account.Id> allReviewers = Sets.newHashSet();
-          allReviewers.addAll(owners.get().values());
-          for (Matcher matcher : owners.getMatchers().values()) {
-            allReviewers.addAll(matcher.getOwners());
-          }
-          logger.debug("Autoassigned reviewers are: {}", allReviewers.toString());
-          reviewerManager.addReviewers(change, allReviewers);
-        }
-      } catch (StorageException e) {
-        logger.warn("Could not open change: {}", id, e);
-      } catch (ReviewerManagerException e) {
-        logger.warn("Could not add reviewers for change: {}", id, e);
+  public void processEvent(Repository repository, Event event) {
+    Change.Id cId = Change.Id.fromRef(event.getRefName());
+    Changes changes = api.changes();
+    // The provider injected by Gerrit is shared with other workers on the
+    // same local thread and thus cannot be closed in this event listener.
+    try {
+      ChangeApi cApi = changes.id(cId.id);
+      ChangeInfo change = cApi.get();
+      if (change == null) {
+        return;
       }
+      PatchList patchList = getPatchList(event, change);
+      if (patchList != null) {
+        PathOwners owners = new PathOwners(accounts, repository, change.branch, patchList);
+        Set<Account.Id> allReviewers = Sets.newHashSet();
+        allReviewers.addAll(owners.get().values());
+        for (Matcher matcher : owners.getMatchers().values()) {
+          allReviewers.addAll(matcher.getOwners());
+        }
+        logger.debug("Autoassigned reviewers are: {}", allReviewers.toString());
+        reviewerManager.addReviewers(cApi, allReviewers);
+      }
+    } catch (RestApiException e) {
+      logger.warn("Could not open change: {}", cId, e);
+    } catch (ReviewerManagerException e) {
+      logger.warn("Could not add reviewers for change: {}", cId, e);
     }
   }
 
-  private PatchList getPatchList(Event event, Change change) {
+  private PatchList getPatchList(Event event, ChangeInfo change) {
     ObjectId newId = null;
     if (event.getNewObjectId() != null) {
       newId = ObjectId.fromString(event.getNewObjectId());
@@ -120,7 +124,7 @@
 
     PatchListKey plKey = PatchListKey.againstCommit(null, newId, IGNORE_NONE);
     try {
-      return patchListCache.get(plKey, change.getProject());
+      return patchListCache.get(plKey, new Project.NameKey(change.project));
     } catch (PatchListNotAvailableException e) {
       logger.warn("Could not load patch list: {}", plKey, e);
     }
diff --git a/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/ReviewerManager.java b/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/ReviewerManager.java
index 28a5359..4a16316 100644
--- a/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/ReviewerManager.java
+++ b/owners-autoassign/src/main/java/com/googlesource/gerrit/owners/common/ReviewerManager.java
@@ -16,13 +16,13 @@
 
 package com.googlesource.gerrit.owners.common;
 
-import com.google.gerrit.exceptions.StorageException;
 import com.google.gerrit.extensions.api.GerritApi;
 import com.google.gerrit.extensions.api.changes.AddReviewerInput;
+import com.google.gerrit.extensions.api.changes.ChangeApi;
 import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.common.ChangeInfo;
 import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.server.util.ManualRequestContext;
 import com.google.gerrit.server.util.OneOffRequestContext;
 import com.google.inject.Inject;
@@ -36,29 +36,33 @@
 public class ReviewerManager {
   private static final Logger log = LoggerFactory.getLogger(ReviewerManager.class);
 
-  private final GerritApi gApi;
   private final OneOffRequestContext requestContext;
+  private final GerritApi gApi;
 
   @Inject
-  public ReviewerManager(GerritApi gApi, OneOffRequestContext requestContext) {
-    this.gApi = gApi;
+  public ReviewerManager(OneOffRequestContext requestContext, GerritApi gApi) {
     this.requestContext = requestContext;
+    this.gApi = gApi;
   }
 
-  public void addReviewers(Change change, Collection<Account.Id> reviewers)
+  public void addReviewers(ChangeApi cApi, Collection<Account.Id> reviewers)
       throws ReviewerManagerException {
-    try (ManualRequestContext ctx = requestContext.openAs(change.getOwner())) {
-      // TODO(davido): Switch back to using changes API again,
-      // when it supports batch mode for adding reviewers
-      ReviewInput in = new ReviewInput();
-      in.reviewers = new ArrayList<>(reviewers.size());
-      for (Account.Id account : reviewers) {
-        AddReviewerInput addReviewerInput = new AddReviewerInput();
-        addReviewerInput.reviewer = account.toString();
-        in.reviewers.add(addReviewerInput);
+    try {
+      ChangeInfo changeInfo = cApi.get();
+      try (ManualRequestContext ctx =
+          requestContext.openAs(new Account.Id(changeInfo.owner._accountId))) {
+        // TODO(davido): Switch back to using changes API again,
+        // when it supports batch mode for adding reviewers
+        ReviewInput in = new ReviewInput();
+        in.reviewers = new ArrayList<>(reviewers.size());
+        for (Account.Id account : reviewers) {
+          AddReviewerInput addReviewerInput = new AddReviewerInput();
+          addReviewerInput.reviewer = account.toString();
+          in.reviewers.add(addReviewerInput);
+        }
+        gApi.changes().id(changeInfo.id).current().review(in);
       }
-      gApi.changes().id(change.getId().get()).current().review(in);
-    } catch (RestApiException | StorageException e) {
+    } catch (RestApiException e) {
       log.error("Couldn't add reviewers to the change", e);
       throw new ReviewerManagerException(e);
     }
diff --git a/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/GitRefListenerIT.java b/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/GitRefListenerIT.java
new file mode 100644
index 0000000..aa6db4f
--- /dev/null
+++ b/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/GitRefListenerIT.java
@@ -0,0 +1,75 @@
+// Copyright (C) 2019 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.vmware.gerrit.owners.common;
+
+import static org.junit.Assert.assertEquals;
+
+import com.google.gerrit.acceptance.LightweightPluginDaemonTest;
+import com.google.gerrit.acceptance.TestPlugin;
+import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
+import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.inject.AbstractModule;
+import org.eclipse.jgit.transport.ReceiveCommand.Type;
+import org.junit.Test;
+
+@TestPlugin(
+    name = "owners-autoassign",
+    sysModule = "com.vmware.gerrit.owners.common.GitRefListenerIT$TestModule")
+public class GitRefListenerIT extends LightweightPluginDaemonTest {
+
+  public static class TestModule extends AbstractModule {
+    @Override
+    protected void configure() {
+      bind(GitReferenceUpdatedListener.class).to(GitRefListenerTest.class);
+    }
+  }
+
+  @Test
+  public void shouldNotProcessNoteDbOnlyRefs() {
+    GitRefListenerTest gitRefListener = getPluginInstance(GitRefListenerTest.class);
+
+    String aRefChange = RefNames.REFS_CHANGES + "01/01" + RefNames.META_SUFFIX;
+    String anOldObjectId = "anOldRef";
+    String aNewObjectId = "aNewRef";
+
+    ReferenceUpdatedEventTest refUpdatedEvent =
+        new ReferenceUpdatedEventTest(
+            project, aRefChange, anOldObjectId, aNewObjectId, Type.CREATE);
+
+    gitRefListener.onGitReferenceUpdated(refUpdatedEvent);
+    assertEquals(0, gitRefListener.getProcessedEvents());
+  }
+
+  @Test
+  public void shouldProcessRefChanges() {
+    GitRefListenerTest gitRefListener = getPluginInstance(GitRefListenerTest.class);
+
+    String aRefChange = RefNames.REFS_CHANGES + "01/01/01";
+    String anOldObjectId = "anOldRef";
+    String aNewObjectId = "aNewRef";
+
+    ReferenceUpdatedEventTest refUpdatedEvent =
+        new ReferenceUpdatedEventTest(
+            project, aRefChange, anOldObjectId, aNewObjectId, Type.CREATE);
+
+    gitRefListener.onGitReferenceUpdated(refUpdatedEvent);
+    assertEquals(1, gitRefListener.getProcessedEvents());
+  }
+
+  private <T> T getPluginInstance(Class<T> clazz) {
+    return plugin.getSysInjector().getInstance(clazz);
+  }
+}
diff --git a/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/GitRefListenerTest.java b/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/GitRefListenerTest.java
new file mode 100644
index 0000000..1052349
--- /dev/null
+++ b/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/GitRefListenerTest.java
@@ -0,0 +1,50 @@
+// Copyright (C) 2019 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.vmware.gerrit.owners.common;
+
+import com.google.gerrit.extensions.api.GerritApi;
+import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.patch.PatchListCache;
+import com.google.inject.Inject;
+import com.googlesource.gerrit.owners.common.Accounts;
+import com.googlesource.gerrit.owners.common.GitRefListener;
+import com.googlesource.gerrit.owners.common.ReviewerManager;
+import org.eclipse.jgit.lib.Repository;
+import org.junit.Ignore;
+
+@Ignore
+public class GitRefListenerTest extends GitRefListener {
+  int processedEvents = 0;
+
+  @Inject
+  public GitRefListenerTest(
+          GerritApi api,
+          PatchListCache patchListCache,
+          GitRepositoryManager repositoryManager,
+          Accounts accounts,
+          ReviewerManager reviewerManager) {
+    super(api, patchListCache, repositoryManager, accounts, reviewerManager);
+  }
+
+  @Override
+  public void processEvent(Repository repository, Event event) {
+    processedEvents++;
+  }
+
+  int getProcessedEvents() {
+    return processedEvents;
+  }
+}
diff --git a/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/ReferenceUpdatedEventTest.java b/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/ReferenceUpdatedEventTest.java
new file mode 100644
index 0000000..446a8bb
--- /dev/null
+++ b/owners-autoassign/src/test/java/com/vmware/gerrit/owners/common/ReferenceUpdatedEventTest.java
@@ -0,0 +1,91 @@
+// Copyright (C) 2019 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.vmware.gerrit.owners.common;
+
+import com.google.gerrit.extensions.api.changes.NotifyHandling;
+import com.google.gerrit.extensions.common.AccountInfo;
+import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
+import com.google.gerrit.reviewdb.client.Project;
+import org.eclipse.jgit.transport.ReceiveCommand;
+import org.junit.Ignore;
+
+@Ignore
+public class ReferenceUpdatedEventTest implements GitReferenceUpdatedListener.Event {
+
+  private final String projectName;
+  private final String ref;
+  private final String oldObjectId;
+  private final String newObjectId;
+  private final ReceiveCommand.Type type;
+
+  public ReferenceUpdatedEventTest(
+      Project.NameKey project,
+      String ref,
+      String oldObjectId,
+      String newObjectId,
+      ReceiveCommand.Type type) {
+    this.projectName = project.get();
+    this.ref = ref;
+    this.oldObjectId = oldObjectId;
+    this.newObjectId = newObjectId;
+    this.type = type;
+  }
+
+  @Override
+  public String getProjectName() {
+    return projectName;
+  }
+
+  @Override
+  public String getRefName() {
+    return ref;
+  }
+
+  @Override
+  public String getOldObjectId() {
+    return oldObjectId;
+  }
+
+  @Override
+  public String getNewObjectId() {
+    return newObjectId;
+  }
+
+  @Override
+  public boolean isCreate() {
+    return type == ReceiveCommand.Type.CREATE;
+  }
+
+  @Override
+  public boolean isDelete() {
+    return type == ReceiveCommand.Type.DELETE;
+  }
+
+  @Override
+  public boolean isNonFastForward() {
+    return type == ReceiveCommand.Type.UPDATE_NONFASTFORWARD;
+  }
+
+  @Override
+  public AccountInfo getUpdater() {
+    return null;
+  }
+
+  @Override
+  public NotifyHandling getNotify() {
+    return NotifyHandling.NONE;
+  }
+}
diff --git a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/AccountsImpl.java b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/AccountsImpl.java
index 49772f6..b5c1f90 100644
--- a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/AccountsImpl.java
+++ b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/AccountsImpl.java
@@ -16,11 +16,12 @@
 
 import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_GERRIT;
 import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_MAILTO;
+import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_USERNAME;
 
-import com.google.gerrit.exceptions.StorageException;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Account.Id;
 import com.google.gerrit.reviewdb.client.AccountGroup;
+import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.account.AccountCache;
 import com.google.gerrit.server.account.AccountResolver;
 import com.google.gerrit.server.account.AccountState;
@@ -48,6 +49,7 @@
   private final AccountCache byId;
   private final GroupCache groupCache;
   private final GroupMembers groupMembers;
+  private final IdentifiedUser adminUser;
   private final OneOffRequestContext oneOffRequestContext;
 
   @Inject
@@ -56,11 +58,13 @@
       AccountCache byId,
       GroupCache groupCache,
       GroupMembers groupMembers,
-      OneOffRequestContext oneOffRequestContext) {
+      OneOffRequestContext oneOffRequestContext,
+      IdentifiedUser.GenericFactory userFactory) {
     this.resolver = resolver;
     this.byId = byId;
     this.groupCache = groupCache;
     this.groupMembers = groupMembers;
+    this.adminUser = userFactory.create(new Account.Id(1000000));
     this.oneOffRequestContext = oneOffRequestContext;
   }
 
@@ -84,7 +88,8 @@
       return Collections.emptySet();
     }
 
-    try {
+    try (ManualRequestContext ctx = oneOffRequestContext.openAs(adminUser.getAccountId())) {
+
       return groupMembers.listAccounts(group.get().getGroupUUID(), null).stream()
           .map(Account::getId)
           .collect(Collectors.toSet());
@@ -127,7 +132,7 @@
       }
 
       return accountIds;
-    } catch (StorageException | IOException | ConfigInvalidException e) {
+    } catch (IOException | ConfigInvalidException e) {
       log.error("Error trying to resolve user " + nameOrEmail, e);
       return Collections.emptySet();
     }
@@ -153,7 +158,9 @@
   }
 
   private boolean isUsernameMatch(ExternalId externalId, String username) {
-    return keySchemeRest(SCHEME_GERRIT, externalId.key())
+    return OptionalUtils.combine(
+            keySchemeRest(SCHEME_GERRIT, externalId.key()),
+            keySchemeRest(SCHEME_USERNAME, externalId.key()))
         .filter(name -> name.equals(username))
         .isPresent();
   }
diff --git a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntry.java b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntry.java
index 9b629b6..3a7111d 100644
--- a/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntry.java
+++ b/owners-common/src/main/java/com/googlesource/gerrit/owners/common/PathOwnersEntry.java
@@ -40,9 +40,7 @@
       String path, OwnersConfig config, Accounts accounts, Set<Account.Id> inheritedOwners) {
     this.ownersPath = path;
     this.owners =
-        config
-            .getOwners()
-            .stream()
+        config.getOwners().stream()
             .flatMap(o -> accounts.find(o).stream())
             .collect(Collectors.toSet());
     if (config.isInherited()) {
diff --git a/owners/src/main/java/gerrit_owners/PRED_file_owners_2.java b/owners/src/main/java/gerrit_owners/PRED_file_owners_2.java
index a5effb4..4e7263a 100644
--- a/owners/src/main/java/gerrit_owners/PRED_file_owners_2.java
+++ b/owners/src/main/java/gerrit_owners/PRED_file_owners_2.java
@@ -76,7 +76,7 @@
     }
     Account account = user.asIdentifiedUser().state().getAccount();
     String userName = account.getFullName();
-    return userName;
+    return sanitizeAsSubmitLabel(userName);
   }
 
   public Term createFormattedList(Prolog engine, Term key) {
@@ -86,9 +86,13 @@
         iteratorStream(owners.getFileOwners().get(path).iterator())
             .map(id -> getFullNameFromId(engine, id))
             .collect(Collectors.toSet());
-    String ownVerb = ownersNames.size() > 1 ? " own " : " owns ";
-    String userNames = ownersNames.stream().collect(Collectors.joining(", "));
-    return SymbolTerm.create(userNames + ownVerb + (new File(path)).getName());
+    String ownVerb = ownersNames.size() > 1 ? "-own-" : "-owns-";
+    String userNames = ownersNames.stream().collect(Collectors.joining("-"));
+    return SymbolTerm.create(userNames + ownVerb + sanitizeAsSubmitLabel(new File(path).getName()));
+  }
+
+  private String sanitizeAsSubmitLabel(String anyLabelPart) {
+    return anyLabelPart.replaceAll("[\\s_\\.]+", "-");
   }
 
   private static IdentifiedUser.GenericFactory userFactory(Prolog engine) {
diff --git a/owners/src/main/prolog/gerrit_owners.pl b/owners/src/main/prolog/gerrit_owners.pl
index 157f0b6..1fe2a22 100644
--- a/owners/src/main/prolog/gerrit_owners.pl
+++ b/owners/src/main/prolog/gerrit_owners.pl
@@ -24,7 +24,7 @@
 add_owner_approval(In, Out) :-
   owner_path(Path),
   \+ owner_approved(Path),
-  Out = [label('Code-Review from owners', need(_)) | In],
+  Out = [label('Code-Review-from-owners', need(_)) | In],
   !.
 
 add_owner_approval(In, Out) :- In = Out.
@@ -32,7 +32,7 @@
 add_owner_approval(Users, In, Out) :-
   owner_path(Path),
   \+ owner_approved(Users, Path),
-  Out = [label('Code-Review from owners', need(_)) | In],
+  Out = [label('Code-Review-from-owners', need(_)) | In],
   !.
 
 add_owner_approval(_, In, Out) :- In = Out.
@@ -67,7 +67,7 @@
     findall(US,code_review_user(US),Approvers),
     matcher_needed(Approvers,P,W),
     \+ W == [],
-    Out = [label('Code-Review from owners', need(_)) | In], !.
+    Out = [label('Code-Review-from-owners', need(_)) | In], !.
 
 add_match_owner_approval(In,Out) :- Out = In.