git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [JGIT PATCH 1/2] Fix RepositoryConfig.setStringList to not corrupt internal state
@ 2008-07-27  2:11 Shawn O. Pearce
  2008-07-27  2:11 ` [JGIT PATCH 2/2] Extended test cases for RepositoryConfigTest Shawn O. Pearce
  0 siblings, 1 reply; 2+ messages in thread
From: Shawn O. Pearce @ 2008-07-27  2:11 UTC (permalink / raw)
  To: Robin Rosenberg, Marek Zawirski; +Cc: git

Calling setStringList was incorrectly storing the String objects
directly in byName.  The byName table should have either an Entry or
a List<Entry> stored within its value position.  Storing the String
(or List<String>) directly confused our get code as it did not find
the object type it expected.  This caused the get code to fallback
to the base configuration (e.g. ~/.gitconfig) or just return null
(claiming the value was never set).

A test case for this appears in the next commit.

Noticed-by: Marek Zawirski <marek.zawirski@gmail.com>
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../src/org/spearce/jgit/lib/RepositoryConfig.java |   23 ++++++++++++++++---
 1 files changed, 19 insertions(+), 4 deletions(-)

diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
index 1431f1f..d1cd5fc 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/RepositoryConfig.java
@@ -381,10 +381,25 @@ public class RepositoryConfig {
 		key += "." + name.toLowerCase();
 		if (values.size() == 0)
 			byName.remove(key);
-		else if (values.size() == 1)
-			byName.put(key, values.get(0));
-		else
-			byName.put(key, new ArrayList<String>(values));
+		else if (values.size() == 1) {
+			final Entry e = new Entry();
+			e.base = section;
+			e.extendedBase = subsection;
+			e.name = name;
+			e.value = values.get(0);
+			byName.put(key, e);
+		} else {
+			final ArrayList<Entry> eList = new ArrayList<Entry>(values.size());
+			for (final String v : values) {
+				final Entry e = new Entry();
+				e.base = section;
+				e.extendedBase = subsection;
+				e.name = name;
+				e.value = v;
+				eList.add(e);
+			}
+			byName.put(key, eList);
+		}
 
 		int entryIndex = 0;
 		int valueIndex = 0;
-- 
1.6.0.rc0.182.gb96c7

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* [JGIT PATCH 2/2] Extended test cases for RepositoryConfigTest
  2008-07-27  2:11 [JGIT PATCH 1/2] Fix RepositoryConfig.setStringList to not corrupt internal state Shawn O. Pearce
@ 2008-07-27  2:11 ` Shawn O. Pearce
  0 siblings, 0 replies; 2+ messages in thread
From: Shawn O. Pearce @ 2008-07-27  2:11 UTC (permalink / raw)
  To: Robin Rosenberg, Marek Zawirski; +Cc: git, Marek Zawirski

From: Marek Zawirski <marek.zawirski@gmail.com>

Add cases for getting values after setting them. It's something
different from reading config file from scratch or writing it out after
modifications. It's case for getting values after modifications in
object model.

Signed-off-by: Marek Zawirski <marek.zawirski@gmail.com>
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>
---
 .../org/spearce/jgit/lib/RepositoryConfigTest.java |   20 +++++++++++++++++++-
 1 files changed, 19 insertions(+), 1 deletions(-)

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java
index 8c55fe8..da7e704 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryConfigTest.java
@@ -2,6 +2,7 @@
  * Copyright (C) 2007, Dave Watson <dwatson@mimvista.com>
  * Copyright (C) 2008, Robin Rosenberg <robin.rosenberg@dewire.com>
  * Copyright (C) 2008, Shawn O. Pearce <spearce@spearce.org>
+ * Copyright (C) 2008, Marek Zawirski <marek.zawirski@gmail.com>
  *
  * All rights reserved.
  *
@@ -41,6 +42,8 @@ package org.spearce.jgit.lib;
 
 import java.io.File;
 import java.io.IOException;
+import java.util.Arrays;
+import java.util.LinkedList;
 
 /**
  * Test reading of git config
@@ -84,11 +87,26 @@ public class RepositoryConfigTest extends RepositoryTestCase {
 		checkFile(cfgFile, "[sec \"ext\"]\n\tname = value\n\tname2 = value2\n");
 	}
 
-	public void test004_PutSimple() throws IOException {
+	public void test004_PutGetSimple() throws IOException {
 		File cfgFile = writeTrashFile("config_004", "");
 		RepositoryConfig repositoryConfig = new RepositoryConfig(null, cfgFile);
 		repositoryConfig.setString("my", null, "somename", "false");
 		repositoryConfig.save();
 		checkFile(cfgFile, "[my]\n\tsomename = false\n");
+		assertEquals("false", repositoryConfig
+				.getString("my", null, "somename"));
+	}
+
+	public void test005_PutGetStringList() throws IOException {
+		File cfgFile = writeTrashFile("config_005", "");
+		RepositoryConfig repositoryConfig = new RepositoryConfig(null, cfgFile);
+		final LinkedList<String> values = new LinkedList<String>();
+		values.add("value1");
+		values.add("value2");
+		repositoryConfig.setStringList("my", null, "somename", values);
+		repositoryConfig.save();
+		assertTrue(Arrays.equals(values.toArray(), repositoryConfig
+				.getStringList("my", null, "somename")));
+		checkFile(cfgFile, "[my]\n\tsomename = value1\n\tsomename = value2\n");
 	}
 }
-- 
1.6.0.rc0.182.gb96c7

^ permalink raw reply related	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2008-07-27  2:13 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-07-27  2:11 [JGIT PATCH 1/2] Fix RepositoryConfig.setStringList to not corrupt internal state Shawn O. Pearce
2008-07-27  2:11 ` [JGIT PATCH 2/2] Extended test cases for RepositoryConfigTest Shawn O. Pearce

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).