diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/EditPreferencesIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/EditPreferencesIT.java index 8770c3c47e..cf89c5a256 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/EditPreferencesIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/account/EditPreferencesIT.java @@ -69,6 +69,18 @@ public class EditPreferencesIT extends AbstractDaemonTest { assertThat(r.getStatusCode()).isEqualTo(HttpStatus.SC_OK); EditPreferencesInfo info = getEditPrefInfo(r); assertEditPreferences(info, out); + + // Partially filled input record + EditPreferencesInfo in = new EditPreferencesInfo(); + in.tabSize = 42; + r = adminSession.put(endPoint, in); + assertThat(r.getStatusCode()).isEqualTo(HttpStatus.SC_NO_CONTENT); + + r = adminSession.get(endPoint); + assertThat(r.getStatusCode()).isEqualTo(HttpStatus.SC_OK); + info = getEditPrefInfo(r); + out.tabSize = in.tabSize; + assertEditPreferences(info, out); } private EditPreferencesInfo getEditPrefInfo(RestResponse r) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GetEditPreferences.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GetEditPreferences.java index e6e76445f2..d99b68fa2a 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GetEditPreferences.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GetEditPreferences.java @@ -19,6 +19,7 @@ import static com.google.gerrit.server.config.ConfigUtil.loadSection; import com.google.gerrit.extensions.client.EditPreferencesInfo; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.RestReadView; +import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.config.AllUsersName; import com.google.gerrit.server.git.GitRepositoryManager; @@ -28,6 +29,7 @@ import com.google.inject.Provider; import com.google.inject.Singleton; import org.eclipse.jgit.errors.ConfigInvalidException; +import org.eclipse.jgit.errors.RepositoryNotFoundException; import org.eclipse.jgit.lib.Repository; import java.io.IOException; @@ -55,13 +57,21 @@ public class GetEditPreferences implements RestReadView { throw new AuthException("restricted to members of Modify Accounts"); } + return readFromGit( + rsrc.getUser().getAccountId(), gitMgr, allUsersName, null); + } + + static EditPreferencesInfo readFromGit(Account.Id id, + GitRepositoryManager gitMgr, AllUsersName allUsersName, + EditPreferencesInfo in) throws IOException, ConfigInvalidException, + RepositoryNotFoundException { try (Repository git = gitMgr.openRepository(allUsersName)) { VersionedAccountPreferences p = - VersionedAccountPreferences.forUser(rsrc.getUser().getAccountId()); + VersionedAccountPreferences.forUser(id); p.load(git); return loadSection(p.getConfig(), UserConfigSections.EDIT, null, - new EditPreferencesInfo(), EditPreferencesInfo.defaults()); + new EditPreferencesInfo(), EditPreferencesInfo.defaults(), in); } } } \ No newline at end of file diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/SetEditPreferences.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/SetEditPreferences.java index 2df1a77077..eabe31d59a 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/SetEditPreferences.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/SetEditPreferences.java @@ -14,6 +14,7 @@ package com.google.gerrit.server.account; +import static com.google.gerrit.server.account.GetEditPreferences.readFromGit; import static com.google.gerrit.server.config.ConfigUtil.storeSection; import com.google.gerrit.extensions.client.EditPreferencesInfo; @@ -24,6 +25,7 @@ import com.google.gerrit.extensions.restapi.RestModifyView; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.config.AllUsersName; +import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.MetaDataUpdate; import com.google.gerrit.server.git.UserConfigSections; import com.google.inject.Inject; @@ -41,14 +43,17 @@ public class SetEditPreferences implements private final Provider self; private final Provider metaDataUpdateFactory; + private final GitRepositoryManager gitMgr; private final AllUsersName allUsersName; @Inject SetEditPreferences(Provider self, Provider metaDataUpdateFactory, + GitRepositoryManager gitMgr, AllUsersName allUsersName) { this.self = self; this.metaDataUpdateFactory = metaDataUpdateFactory; + this.gitMgr = gitMgr; this.allUsersName = allUsersName; } @@ -72,7 +77,8 @@ public class SetEditPreferences implements try { prefs = VersionedAccountPreferences.forUser(accountId); prefs.load(md); - storeSection(prefs.getConfig(), UserConfigSections.EDIT, null, in, + storeSection(prefs.getConfig(), UserConfigSections.EDIT, null, + readFromGit(accountId, gitMgr, allUsersName, in), EditPreferencesInfo.defaults()); prefs.commit(md); } finally { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/ConfigUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/ConfigUtil.java index 2b6ee41f04..be9d984f54 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/ConfigUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/ConfigUtil.java @@ -314,19 +314,19 @@ public class ConfigUtil { * The loading is performed eagerly: all values are set. *

* Fields marked with final or transient modifiers are skipped. - *

- * Boolean fields are only set when their values are true. * * @param cfg config from which the values are loaded * @param section section * @param sub subsection * @param s instance of class in which the values are set * @param defaults instance of class with default values + * @param i instance to merge during the load. When present, the + * boolean fields are not nullified when their values are false * @return loaded instance * @throws ConfigInvalidException */ public static T loadSection(Config cfg, String section, String sub, - T s, T defaults) throws ConfigInvalidException { + T s, T defaults, T i) throws ConfigInvalidException { try { for (Field f : s.getClass().getDeclaredFields()) { if (skipField(f)) { @@ -345,7 +345,7 @@ public class ConfigUtil { f.set(s, cfg.getLong(section, sub, n, (Long) d)); } else if (isBoolean(t)) { boolean b = cfg.getBoolean(section, sub, n, (Boolean) d); - if (b) { + if (b || i != null) { f.set(s, b); } } else if (t.isEnum()) { @@ -353,6 +353,12 @@ public class ConfigUtil { } else { throw new ConfigInvalidException("type is unknown: " + t.getName()); } + if (i != null) { + Object o = f.get(i); + if (o != null) { + f.set(s, o); + } + } } } catch (SecurityException | IllegalArgumentException | IllegalAccessException e) { diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/config/ConfigUtilTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/config/ConfigUtilTest.java index ab00ba8ee3..4f409d1046 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/config/ConfigUtilTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/config/ConfigUtilTest.java @@ -98,7 +98,7 @@ public class ConfigUtilTest { assertThat(cfg.getString(SECT, SUB, "sd")).isNull(); SectionInfo out = new SectionInfo(); - ConfigUtil.loadSection(cfg, SECT, SUB, out, d); + ConfigUtil.loadSection(cfg, SECT, SUB, out, d, null); assertThat(out.i).isEqualTo(in.i); assertThat(out.ii).isEqualTo(in.ii); assertThat(out.id).isEqualTo(d.id); @@ -114,6 +114,25 @@ public class ConfigUtilTest { assertThat(out.td).isEqualTo(d.td); } + @Test + public void mergeSection() throws Exception { + SectionInfo d = SectionInfo.defaults(); + Config cfg = new Config(); + ConfigUtil.storeSection(cfg, SECT, SUB, d, d); + + SectionInfo in = new SectionInfo(); + in.i = 42; + + SectionInfo out = new SectionInfo(); + ConfigUtil.loadSection(cfg, SECT, SUB, out, d, in); + // Check original values preserved + assertThat(out.id).isEqualTo(d.id); + // Check merged values + assertThat(out.i).isEqualTo(in.i); + // Check that boolean attribute not nullified + assertThat(out.bb).isFalse(); + } + @Test public void testTimeUnit() { assertEquals(ms(0, MILLISECONDS), parse("0"));