VersionedMetaData: Fix deletion of last file in branch

If the last file of a branch was deleted by VersionedMetaData.saveFile
we discarded the new tree and instead kept the tree from the parent
commit. Due to this the file was not deleted and its original content
was kept.

This issue was introduced by change I83f09e08f.

E.g. this bug can be hit if all project watches of a user are deleted
and the user branch only contained the watch.config file. In this case
the project watches were still there after the deletion.

Change-Id: I3047ea087fb9981eb484ec6689652a0ed44e1b4b
Signed-off-by: Edwin Kempin <ekempin@google.com>

2 files changed, 46 insertions, 8 deletions

diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java
index abd84ef29a..0b56f4303a 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java
@@ -16,6 +16,7 @@ package com.google.gerrit.acceptance.server.project;
 
 import static com.google.common.truth.Truth.assertThat;
 
+import com.google.common.collect.ImmutableSet;
 import com.google.gerrit.acceptance.AbstractDaemonTest;
 import com.google.gerrit.acceptance.NoHttpd;
 import com.google.gerrit.acceptance.PushOneCommit;
@@ -24,21 +25,32 @@ import com.google.gerrit.extensions.client.ProjectWatchInfo;
 import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.reviewdb.client.AccountProjectWatch.NotifyType;
 import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.common.TimeUtil;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.server.account.WatchConfig;
+import com.google.gerrit.server.account.WatchConfig.ProjectWatchKey;
 import com.google.gerrit.server.git.NotifyConfig;
 import com.google.gerrit.server.git.ProjectConfig;
 import com.google.gerrit.server.mail.Address;
 import com.google.gerrit.testutil.FakeEmailSender.Message;
 
+import com.google.inject.Inject;
+import java.util.Collections;
+import java.util.EnumSet;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
 import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
 import org.eclipse.jgit.junit.TestRepository;
 import org.junit.Test;
 
 import java.util.ArrayList;
-import java.util.EnumSet;
-import java.util.List;
 
 @NoHttpd
 public class ProjectWatchIT extends AbstractDaemonTest {
+  @Inject private WatchConfig.Accessor watchConfig;
+
   @Test
   public void newPatchSetsNotifyConfig() throws Exception {
     Address addr = new Address("Watcher", "watcher@example.com");
@@ -173,4 +185,34 @@ public class ProjectWatchIT extends AbstractDaemonTest {
     projectsToWatch.add(pwi);
     gApi.accounts().self().setWatchedProjects(projectsToWatch);
   }
+
+  @Test
+  public void deleteAllProjectWatches() throws Exception {
+    Map<ProjectWatchKey, Set<NotifyType>> watches = new HashMap<>();
+    watches.put(ProjectWatchKey.create(project, "*"), ImmutableSet.of(NotifyType.ALL));
+    watchConfig.upsertProjectWatches(admin.getId(), watches);
+    assertThat(watchConfig.getProjectWatches(admin.getId())).isNotEmpty();
+
+    watchConfig.deleteAllProjectWatches(admin.getId());
+    assertThat(watchConfig.getProjectWatches(admin.getId())).isEmpty();
+  }
+
+  @Test
+  public void deleteAllProjectWatchesIfWatchConfigIsTheOnlyFileInUserBranch() throws Exception {
+    // Create account that has no files in its refs/users/ branch.
+    Account.Id id = new Account.Id(db.nextAccountId());
+    Account a = new Account(id, TimeUtil.nowTs());
+    db.accounts().insert(Collections.singleton(a));
+
+    // Add a project watch so that a watch.config file in the refs/users/ branch is created.
+    Map<ProjectWatchKey, Set<NotifyType>> watches = new HashMap<>();
+    watches.put(ProjectWatchKey.create(project, "*"), ImmutableSet.of(NotifyType.ALL));
+    watchConfig.upsertProjectWatches(id, watches);
+    assertThat(watchConfig.getProjectWatches(id)).isNotEmpty();
+
+    // Delete all project watches so that the watch.config file in the refs/users/ branch is
+    // deleted.
+    watchConfig.deleteAllProjectWatches(id);
+    assertThat(watchConfig.getProjectWatches(id)).isEmpty();
+  }
 }
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java
index 6334cd2a8a..c95944374e 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java
@@ -287,12 +287,8 @@ public abstract class VersionedMetaData {
           return;
         }
 
-        // Reuse tree from parent commit unless there are contents in newTree or
-        // there is no tree for a parent commit.
-        ObjectId res = newTree.getEntryCount() != 0 || srcTree == null
-            ? newTree.writeTree(inserter) : srcTree.copy();
-        if (res.equals(srcTree) && !update.allowEmpty()
-            && (commit.getTreeId() == null)) {
+        ObjectId res = newTree.writeTree(inserter);
+        if (res.equals(srcTree) && !update.allowEmpty() && (commit.getTreeId() == null)) {
           // If there are no changes to the content, don't create the commit.
           return;
         }