opendev/gerrit
Code Issues Proposed changes

VersionedMetaData: Fix deletion of last file in branch

Browse Source 
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>
This commit is contained in:
Edwin Kempin
2017-03-17 13:03:38 +01:00
committed by David Pursehouse
parent decdaaad05
commit f1f980d3a8
2 changed files with 46 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

46
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/project/ProjectWatchIT.java
View File

@@ -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();
}
}

8
gerrit-server/src/main/java/com/google/gerrit/server/git/VersionedMetaData.java
View File

@@ -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;
}