git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH JGit] Adding update-server-info functionality try2
@ 2009-09-16  0:48 mr.gaffo
  2009-09-16  0:48 ` [PATCH JGit 1/5] adding tests for ObjectDirectory mr.gaffo
  0 siblings, 1 reply; 12+ messages in thread
From: mr.gaffo @ 2009-09-16  0:48 UTC (permalink / raw)
  To: git

This patch series implements update-server-info functionality
in JGit and integrates it with ReceivePack so that repositories
hosted by git-http can also be hosted by JGit.

It also incorporates suggesions from RobinRosenberg.

Please be gentle.

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

* [PATCH JGit 1/5] adding tests for ObjectDirectory
  2009-09-16  0:48 [PATCH JGit] Adding update-server-info functionality try2 mr.gaffo
@ 2009-09-16  0:48 ` mr.gaffo
  2009-09-16  0:48   ` [PATCH JGit 2/5] Create abstract method on ObjectDatabase for accessing the list of local pack files mr.gaffo
  2009-09-21 19:30   ` [PATCH JGit 1/5] adding tests for ObjectDirectory Shawn O. Pearce
  0 siblings, 2 replies; 12+ messages in thread
From: mr.gaffo @ 2009-09-16  0:48 UTC (permalink / raw)
  To: git; +Cc: mike.gaffney, Mike Gaffney

From: mike.gaffney <mike.gaffney@asolutions.com>

Signed-off-by: Mike Gaffney <mr.gaffo@gmail.com>
---
 .../org/spearce/jgit/lib/ObjectDirectoryTest.java  |  106 ++++++++++++++++++++
 .../org/spearce/jgit/lib/RepositoryTestCase.java   |   58 +----------
 .../tst/org/spearce/jgit/util/JGitTestUtil.java    |   49 +++++++++
 3 files changed, 161 insertions(+), 52 deletions(-)
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectDirectoryTest.java

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectDirectoryTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectDirectoryTest.java
new file mode 100644
index 0000000..5b1fc0f
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectDirectoryTest.java
@@ -0,0 +1,106 @@
+/*
+ * Copyright (C) 2009, Mike Gaffney.
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package org.spearce.jgit.lib;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.UUID;
+
+import org.spearce.jgit.util.JGitTestUtil;
+
+import junit.framework.TestCase;
+
+public class ObjectDirectoryTest extends TestCase {
+	
+	private File testDir;
+
+	@Override
+	protected void setUp() throws Exception {
+		testDir = new File(new File(System.getProperty("java.io.tmpdir")), UUID.randomUUID().toString());
+	}
+	
+	@Override
+	protected void tearDown() throws Exception {
+		if (testDir.exists()){
+			JGitTestUtil.recursiveDelete(testDir, false, getClass().getName() + "." + getName(), true);
+		}
+	}
+
+	public void testCanGetDirectory() throws Exception {
+		ObjectDirectory od = new ObjectDirectory(testDir);
+		assertEquals(testDir, od.getDirectory());
+	}
+	
+	public void testExistsWithExistingDirectory() throws Exception {
+		createTestDir();
+		ObjectDirectory od = new ObjectDirectory(testDir);
+		assertTrue(od.exists());
+	}
+	
+	public void testExistsWithNonExistantDirectory() throws Exception {
+		assertFalse(new ObjectDirectory(new File("/some/nonexistant/file")).exists());
+	}
+	
+	public void testCreateMakesCorrectDirectories() throws Exception {
+		assertFalse(testDir.exists());
+		new ObjectDirectory(testDir).create();
+		assertTrue(testDir.exists());
+		
+		File infoDir = new File(testDir, "info");
+		assertTrue(infoDir.exists());
+		assertTrue(infoDir.isDirectory());
+		
+		File packDir = new File(testDir, "pack");
+		assertTrue(packDir.exists());
+		assertTrue(packDir.isDirectory());
+	}
+	
+	public void testGettingObjectFile() throws Exception {
+		ObjectDirectory od = new ObjectDirectory(testDir);
+		assertEquals(new File(testDir, "02/829ae153935095e4223f30cfc98c835de71bee"), 
+					 od.fileFor(ObjectId.fromString("02829ae153935095e4223f30cfc98c835de71bee")));
+		assertEquals(new File(testDir, "b0/52a1272310d8df34de72f60204dee7e28a43d0"), 
+				 od.fileFor(ObjectId.fromString("b052a1272310d8df34de72f60204dee7e28a43d0")));
+	}
+	
+	private void createTestDir(){
+		if (!testDir.mkdir()){
+			fail("unable to create test directory");
+		}
+	}
+	
+}
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java
index d1aef78..cfd7d25 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java
@@ -106,53 +106,7 @@ protected void configure() {
 	 * @param dir
 	 */
 	protected void recursiveDelete(final File dir) {
-		recursiveDelete(dir, false, getClass().getName() + "." + getName(), true);
-	}
-
-	protected static boolean recursiveDelete(final File dir, boolean silent,
-			final String name, boolean failOnError) {
-		assert !(silent && failOnError);
-		if (!dir.exists())
-			return silent;
-		final File[] ls = dir.listFiles();
-		if (ls != null) {
-			for (int k = 0; k < ls.length; k++) {
-				final File e = ls[k];
-				if (e.isDirectory()) {
-					silent = recursiveDelete(e, silent, name, failOnError);
-				} else {
-					if (!e.delete()) {
-						if (!silent) {
-							reportDeleteFailure(name, failOnError, e);
-						}
-						silent = !failOnError;
-					}
-				}
-			}
-		}
-		if (!dir.delete()) {
-			if (!silent) {
-				reportDeleteFailure(name, failOnError, dir);
-			}
-			silent = !failOnError;
-		}
-		return silent;
-	}
-
-	private static void reportDeleteFailure(final String name,
-			boolean failOnError, final File e) {
-		String severity;
-		if (failOnError)
-			severity = "Error";
-		else
-			severity = "Warning";
-		String msg = severity + ": Failed to delete " + e;
-		if (name != null)
-			msg += " in " + name;
-		if (failOnError)
-			fail(msg);
-		else
-			System.out.println(msg);
+		JGitTestUtil.recursiveDelete(dir, false, getClass().getName() + "." + getName(), true);
 	}
 
 	protected static void copyFile(final File src, final File dst)
@@ -215,7 +169,7 @@ public void setUp() throws Exception {
 		super.setUp();
 		configure();
 		final String name = getClass().getName() + "." + getName();
-		recursiveDelete(trashParent, true, name, false); // Cleanup old failed stuff
+		JGitTestUtil.recursiveDelete(trashParent, true, name, false); // Cleanup old failed stuff
 		trash = new File(trashParent,"trash"+System.currentTimeMillis()+"."+(testcount++));
 		trash_git = new File(trash, ".git").getCanonicalFile();
 		if (shutdownhook == null) {
@@ -230,7 +184,7 @@ public void run() {
 					System.gc();
 					for (Runnable r : shutDownCleanups)
 						r.run();
-					recursiveDelete(trashParent, false, null, false);
+					JGitTestUtil.recursiveDelete(trashParent, false, null, false);
 				}
 			};
 			Runtime.getRuntime().addShutdownHook(shutdownhook);
@@ -277,9 +231,9 @@ protected void tearDown() throws Exception {
 			System.gc();
 
 		final String name = getClass().getName() + "." + getName();
-		recursiveDelete(trash, false, name, true);
+		JGitTestUtil.recursiveDelete(trash, false, name, true);
 		for (Repository r : repositoriesToClose)
-			recursiveDelete(r.getWorkDir(), false, name, true);
+			JGitTestUtil.recursiveDelete(r.getWorkDir(), false, name, true);
 		repositoriesToClose.clear();
 
 		super.tearDown();
@@ -314,7 +268,7 @@ protected Repository createNewEmptyRepo(boolean bare) throws IOException {
 		final String name = getClass().getName() + "." + getName();
 		shutDownCleanups.add(new Runnable() {
 			public void run() {
-				recursiveDelete(newTestRepo, false, name, false);
+				JGitTestUtil.recursiveDelete(newTestRepo, false, name, false);
 			}
 		});
 		repositoriesToClose.add(newRepo);
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/util/JGitTestUtil.java b/org.spearce.jgit.test/tst/org/spearce/jgit/util/JGitTestUtil.java
index eee0c14..446c674 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/util/JGitTestUtil.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/util/JGitTestUtil.java
@@ -41,6 +41,9 @@
 import java.net.URISyntaxException;
 import java.net.URL;
 
+import junit.framework.AssertionFailedError;
+
+
 public abstract class JGitTestUtil {
 	public static final String CLASSPATH_TO_RESOURCES = "org/spearce/jgit/test/resources/";
 
@@ -68,4 +71,50 @@ public static File getTestResourceFile(final String fileName) {
 	private static ClassLoader cl() {
 		return JGitTestUtil.class.getClassLoader();
 	}
+
+	public static boolean recursiveDelete(final File dir, boolean silent,
+			final String name, boolean failOnError) {
+		assert !(silent && failOnError);
+		if (!dir.exists())
+			return silent;
+		final File[] ls = dir.listFiles();
+		if (ls != null) {
+			for (int k = 0; k < ls.length; k++) {
+				final File e = ls[k];
+				if (e.isDirectory()) {
+					silent = recursiveDelete(e, silent, name, failOnError);
+				} else {
+					if (!e.delete()) {
+						if (!silent) {
+							JGitTestUtil.reportDeleteFailure(name, failOnError, e);
+						}
+						silent = !failOnError;
+					}
+				}
+			}
+		}
+		if (!dir.delete()) {
+			if (!silent) {
+				JGitTestUtil.reportDeleteFailure(name, failOnError, dir);
+			}
+			silent = !failOnError;
+		}
+		return silent;
+	}
+
+	private static void reportDeleteFailure(final String name,
+			boolean failOnError, final File e) {
+		String severity;
+		if (failOnError)
+			severity = "Error";
+		else
+			severity = "Warning";
+		String msg = severity + ": Failed to delete " + e;
+		if (name != null)
+			msg += " in " + name;
+		if (failOnError)
+			throw new AssertionFailedError(msg);
+		else
+			System.out.println(msg);
+	}
 }
-- 
1.6.4.2

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

* [PATCH JGit 2/5] Create abstract method on ObjectDatabase for accessing the list of local pack files.
  2009-09-16  0:48 ` [PATCH JGit 1/5] adding tests for ObjectDirectory mr.gaffo
@ 2009-09-16  0:48   ` mr.gaffo
  2009-09-16  0:48     ` [PATCH JGit 3/5] Implemented directory based info cache for objects/info/packs mr.gaffo
  2009-09-21 19:40     ` [PATCH JGit 2/5] Create abstract method on ObjectDatabase for accessing the list of local pack files Shawn O. Pearce
  2009-09-21 19:30   ` [PATCH JGit 1/5] adding tests for ObjectDirectory Shawn O. Pearce
  1 sibling, 2 replies; 12+ messages in thread
From: mr.gaffo @ 2009-09-16  0:48 UTC (permalink / raw)
  To: git; +Cc: mike.gaffney, Mike Gaffney

From: mike.gaffney <mike.gaffney@asolutions.com>

Implemented the method for AlternateRepository database as a passthrough

Implemented the method for ObjectDirectory as a toList of the current
cached private PackList.

Hopefully this will allow easier reference to the list of packs for
others like the server side of fetch.

Signed-off-by: Mike Gaffney <mr.gaffo@gmail.com>
---
 .../org/spearce/jgit/lib/ObjectDirectoryTest.java  |   22 ++++++++++++++++++++
 .../tst/org/spearce/jgit/util/JGitTestUtil.java    |   21 ++++++++++++++++++-
 .../jgit/lib/AlternateRepositoryDatabase.java      |    6 +++++
 .../src/org/spearce/jgit/lib/ObjectDatabase.java   |   11 +++++++++-
 .../src/org/spearce/jgit/lib/ObjectDirectory.java  |    6 +++++
 5 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectDirectoryTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectDirectoryTest.java
index 5b1fc0f..c27580f 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectDirectoryTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectDirectoryTest.java
@@ -38,6 +38,7 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.util.List;
 import java.util.UUID;
 
 import org.spearce.jgit.util.JGitTestUtil;
@@ -45,6 +46,9 @@
 import junit.framework.TestCase;
 
 public class ObjectDirectoryTest extends TestCase {
+	private static final String PACK_NAME = "pack-34be9032ac282b11fa9babdc2b2a93ca996c9c2f";
+	private static final File TEST_PACK = JGitTestUtil.getTestResourceFile(PACK_NAME + ".pack");
+	private static final File TEST_IDX = JGitTestUtil.getTestResourceFile(PACK_NAME + ".idx");
 	
 	private File testDir;
 
@@ -97,6 +101,24 @@ public void testGettingObjectFile() throws Exception {
 				 od.fileFor(ObjectId.fromString("b052a1272310d8df34de72f60204dee7e28a43d0")));
 	}
 	
+	public void testListLocalPacksNotCreated() throws Exception {
+		assertEquals(0, new ObjectDirectory(testDir).listLocalPacks().size());
+	}
+	
+	public void testListLocalPacksWhenThereIsAPack() throws Exception {
+		createTestDir();
+		File packsDir = new File(testDir, "pack");
+		packsDir.mkdirs();
+		
+		JGitTestUtil.copyFile(TEST_PACK, new File(packsDir, TEST_PACK.getName()));
+		JGitTestUtil.copyFile(TEST_IDX, new File(packsDir, TEST_IDX.getName()));
+
+		ObjectDirectory od = new ObjectDirectory(testDir);
+		List<PackFile> localPacks = od.listLocalPacks();
+		assertEquals(1, localPacks.size());
+		assertEquals(TEST_PACK.getName(), localPacks.get(0).getPackFile().getName());
+	}
+
 	private void createTestDir(){
 		if (!testDir.mkdir()){
 			fail("unable to create test directory");
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/util/JGitTestUtil.java b/org.spearce.jgit.test/tst/org/spearce/jgit/util/JGitTestUtil.java
index 446c674..785922a 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/util/JGitTestUtil.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/util/JGitTestUtil.java
@@ -38,6 +38,12 @@
 package org.spearce.jgit.util;
 
 import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileNotFoundException;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
 import java.net.URISyntaxException;
 import java.net.URL;
 
@@ -63,11 +69,24 @@ public static File getTestResourceFile(final String fileName) {
 		}
 		try {
 			return new File(url.toURI());
-		} catch(URISyntaxException e) {
+		} catch (URISyntaxException e) {
 			return new File(url.getPath());
 		}
 	}
 
+	public static void copyFile(final File fromFile, final File toFile) throws IOException {
+		InputStream in = new FileInputStream(fromFile);
+		OutputStream out = new FileOutputStream(toFile);
+
+		byte[] buf = new byte[1024];
+		int len;
+		while ((len = in.read(buf)) > 0) {
+			out.write(buf, 0, len);
+		}
+		in.close();
+		out.close();
+	}
+
 	private static ClassLoader cl() {
 		return JGitTestUtil.class.getClassLoader();
 	}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/AlternateRepositoryDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/lib/AlternateRepositoryDatabase.java
index ee4c4cf..68ad488 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/AlternateRepositoryDatabase.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/AlternateRepositoryDatabase.java
@@ -39,6 +39,7 @@
 
 import java.io.IOException;
 import java.util.Collection;
+import java.util.List;
 
 /**
  * An ObjectDatabase of another {@link Repository}.
@@ -124,4 +125,9 @@ void openObjectInAllPacks1(final Collection<PackedObjectLoader> out,
 	protected void closeAlternates(final ObjectDatabase[] alt) {
 		// Do nothing; these belong to odb to close, not us.
 	}
+
+	@Override
+	public List<PackFile> listLocalPacks() {
+		return odb.listLocalPacks();
+	}
 }
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDatabase.java
index a547052..722c802 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDatabase.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDatabase.java
@@ -39,6 +39,7 @@
 
 import java.io.IOException;
 import java.util.Collection;
+import java.util.List;
 import java.util.concurrent.atomic.AtomicReference;
 
 /**
@@ -64,7 +65,15 @@
 	protected ObjectDatabase() {
 		alternates = new AtomicReference<ObjectDatabase[]>();
 	}
-
+	
+	/**
+	 * The list of Packs THIS repo contains
+	 * 
+	 * @return List<PackFile> of package names contained in this repo. 
+	 * 		   Should be an empty list if there are none.
+	 */
+	public abstract List<PackFile> listLocalPacks();
+	
 	/**
 	 * Does this database exist yet?
 	 *
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java
index 859824d..cbe132d 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java
@@ -508,4 +508,10 @@ boolean tryAgain(final long currLastModified) {
 			return true;
 		}
 	}
+
+	@Override
+	public List<PackFile> listLocalPacks() {
+		tryAgain1();
+		return new ArrayList<PackFile>(Arrays.asList(packList.get().packs));
+	}
 }
-- 
1.6.4.2

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

* [PATCH JGit 3/5] Implemented directory based info cache for objects/info/packs.
  2009-09-16  0:48   ` [PATCH JGit 2/5] Create abstract method on ObjectDatabase for accessing the list of local pack files mr.gaffo
@ 2009-09-16  0:48     ` mr.gaffo
  2009-09-16  0:48       ` [PATCH JGit 4/5] Adding in a InfoDatabase like ObjectDatabase and and implementation based upon a directory mr.gaffo
  2009-10-08 17:00       ` [PATCH JGit 3/5] Implemented directory based info cache for objects/info/packs Shawn O. Pearce
  2009-09-21 19:40     ` [PATCH JGit 2/5] Create abstract method on ObjectDatabase for accessing the list of local pack files Shawn O. Pearce
  1 sibling, 2 replies; 12+ messages in thread
From: mr.gaffo @ 2009-09-16  0:48 UTC (permalink / raw)
  To: git; +Cc: Mike Gaffney

From: Mike Gaffney <mr.gaffo@gmail.com>

Details:

Add abstract method for updating the object db's info cache to directory.

Implemented passthrough on Alternate for the update of infocache.

Added utility that generates the contents of the objects/info/packs
file as a string from a list of PackFiles.

Added implementation from ObjectDirectory on down
for creating the info cache.

Added test for creating the info cache

Signed-off-by: Mike Gaffney <mr.gaffo@gmail.com>
---
 .../CachedPacksInfoFileContentsGeneratorTest.java  |   74 ++++++++++++++++++++
 .../org/spearce/jgit/lib/ObjectDirectoryTest.java  |   36 +++++++---
 .../tst/org/spearce/jgit/util/JGitTestUtil.java    |   26 ++++++-
 .../jgit/lib/AlternateRepositoryDatabase.java      |    5 ++
 .../lib/CachedPacksInfoFileContentsGenerator.java  |   63 +++++++++++++++++
 .../src/org/spearce/jgit/lib/Constants.java        |    3 +
 .../src/org/spearce/jgit/lib/ObjectDatabase.java   |    8 ++
 .../src/org/spearce/jgit/lib/ObjectDirectory.java  |    5 ++
 .../lib/UpdateDirectoryBasedPacksInfoCache.java    |   62 ++++++++++++++++
 .../spearce/jgit/lib/UpdateDirectoryInfoCache.java |   26 +++++++
 .../org/spearce/jgit/transport/ReceivePack.java    |   10 +++
 11 files changed, 307 insertions(+), 11 deletions(-)
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/lib/CachedPacksInfoFileContentsGeneratorTest.java
 create mode 100644 org.spearce.jgit/src/org/spearce/jgit/lib/CachedPacksInfoFileContentsGenerator.java
 create mode 100644 org.spearce.jgit/src/org/spearce/jgit/lib/UpdateDirectoryBasedPacksInfoCache.java
 create mode 100644 org.spearce.jgit/src/org/spearce/jgit/lib/UpdateDirectoryInfoCache.java

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/CachedPacksInfoFileContentsGeneratorTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/CachedPacksInfoFileContentsGeneratorTest.java
new file mode 100644
index 0000000..bea0b70
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/CachedPacksInfoFileContentsGeneratorTest.java
@@ -0,0 +1,74 @@
+/*
+ * Copyright (C) 2009, Mike Gaffney.
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package org.spearce.jgit.lib;
+
+import java.io.File;
+import java.util.ArrayList;
+import java.util.List;
+
+import org.spearce.jgit.util.JGitTestUtil;
+
+import junit.framework.TestCase;
+
+public class CachedPacksInfoFileContentsGeneratorTest extends TestCase {
+	private static final String PACK_NAME = "pack-34be9032ac282b11fa9babdc2b2a93ca996c9c2f";
+	private static final File TEST_PACK = JGitTestUtil.getTestResourceFile(PACK_NAME + ".pack");
+	private static final File TEST_IDX = JGitTestUtil.getTestResourceFile(PACK_NAME + ".idx");
+
+	public void testGettingPacksContentsSinglePack() throws Exception {
+		List<PackFile> packs = new ArrayList<PackFile>();
+		packs.add(new PackFile(TEST_IDX, TEST_PACK));
+		
+		assertEquals("P " + TEST_PACK.getName() + "\n\n", new CachedPacksInfoFileContentsGenerator(packs).generateContents());
+	}
+	
+	public void testGettingPacksContentsMultiplePacks() throws Exception {
+		List<PackFile> packs = new ArrayList<PackFile>();
+		packs.add(new PackFile(TEST_IDX, TEST_PACK));
+		packs.add(new PackFile(TEST_IDX, TEST_PACK));
+		packs.add(new PackFile(TEST_IDX, TEST_PACK));
+		
+		StringBuilder expected = new StringBuilder();
+		expected.append("P ").append(TEST_PACK.getName()).append("\n");
+		expected.append("P ").append(TEST_PACK.getName()).append("\n");
+		expected.append("P ").append(TEST_PACK.getName()).append("\n");
+		expected.append("\n");
+		
+		assertEquals(expected.toString(), new CachedPacksInfoFileContentsGenerator(packs).generateContents());
+	}
+	
+}
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectDirectoryTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectDirectoryTest.java
index c27580f..204fb7c 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectDirectoryTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectDirectoryTest.java
@@ -38,8 +38,8 @@
 
 import java.io.File;
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.List;
-import java.util.UUID;
 
 import org.spearce.jgit.util.JGitTestUtil;
 
@@ -54,9 +54,9 @@
 
 	@Override
 	protected void setUp() throws Exception {
-		testDir = new File(new File(System.getProperty("java.io.tmpdir")), UUID.randomUUID().toString());
+		testDir = JGitTestUtil.generateTempDirectoryFileObject();
 	}
-	
+
 	@Override
 	protected void tearDown() throws Exception {
 		if (testDir.exists()){
@@ -106,23 +106,41 @@ public void testListLocalPacksNotCreated() throws Exception {
 	}
 	
 	public void testListLocalPacksWhenThereIsAPack() throws Exception {
-		createTestDir();
-		File packsDir = new File(testDir, "pack");
-		packsDir.mkdirs();
-		
-		JGitTestUtil.copyFile(TEST_PACK, new File(packsDir, TEST_PACK.getName()));
-		JGitTestUtil.copyFile(TEST_IDX, new File(packsDir, TEST_IDX.getName()));
+		createSamplePacksDir();
 
 		ObjectDirectory od = new ObjectDirectory(testDir);
 		List<PackFile> localPacks = od.listLocalPacks();
 		assertEquals(1, localPacks.size());
 		assertEquals(TEST_PACK.getName(), localPacks.get(0).getPackFile().getName());
 	}
+	
+	public void testUpdateInfoCacheCreatesPacksAndRefsFile() throws Exception {
+		createSamplePacksDir();
+
+		ObjectDirectory od = new ObjectDirectory(testDir);
+		od.create();
+		od.updateInfoCache();
+		
+		String expectedContents = new CachedPacksInfoFileContentsGenerator(od.listLocalPacks()).generateContents();
+		File packsFile = new File(od.getDirectory(), Constants.CACHED_PACKS_FILE);
+
+		assertTrue(packsFile.exists());
+		assertEquals(expectedContents, JGitTestUtil.readFileAsString(packsFile));
+	}
 
 	private void createTestDir(){
 		if (!testDir.mkdir()){
 			fail("unable to create test directory");
 		}
 	}
+
+	private void createSamplePacksDir() throws IOException {
+		createTestDir();
+		File packsDir = new File(testDir, "pack");
+		packsDir.mkdirs();
+		
+		JGitTestUtil.copyFile(TEST_PACK, new File(packsDir, TEST_PACK.getName()));
+		JGitTestUtil.copyFile(TEST_IDX, new File(packsDir, TEST_IDX.getName()));
+	}
 	
 }
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/util/JGitTestUtil.java b/org.spearce.jgit.test/tst/org/spearce/jgit/util/JGitTestUtil.java
index 785922a..44630fd 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/util/JGitTestUtil.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/util/JGitTestUtil.java
@@ -37,15 +37,17 @@
 
 package org.spearce.jgit.util;
 
+import java.io.BufferedReader;
 import java.io.File;
 import java.io.FileInputStream;
-import java.io.FileNotFoundException;
 import java.io.FileOutputStream;
+import java.io.FileReader;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.net.URISyntaxException;
 import java.net.URL;
+import java.util.UUID;
 
 import junit.framework.AssertionFailedError;
 
@@ -74,7 +76,8 @@ public static File getTestResourceFile(final String fileName) {
 		}
 	}
 
-	public static void copyFile(final File fromFile, final File toFile) throws IOException {
+	public static void copyFile(final File fromFile, final File toFile)
+			throws IOException {
 		InputStream in = new FileInputStream(fromFile);
 		OutputStream out = new FileOutputStream(toFile);
 
@@ -87,6 +90,21 @@ public static void copyFile(final File fromFile, final File toFile) throws IOExc
 		out.close();
 	}
 
+	public static String readFileAsString(final File file)
+			throws java.io.IOException {
+		StringBuilder fileData = new StringBuilder(1000);
+		BufferedReader reader = new BufferedReader(new FileReader(file));
+		char[] buf = new char[1024];
+		int numRead = 0;
+		while ((numRead = reader.read(buf)) != -1) {
+			String readData = String.valueOf(buf, 0, numRead);
+			fileData.append(readData);
+			buf = new char[1024];
+		}
+		reader.close();
+		return fileData.toString();
+	}
+
 	private static ClassLoader cl() {
 		return JGitTestUtil.class.getClassLoader();
 	}
@@ -136,4 +154,8 @@ private static void reportDeleteFailure(final String name,
 		else
 			System.out.println(msg);
 	}
+
+	public static File generateTempDirectoryFileObject() {
+		return new File(new File(System.getProperty("java.io.tmpdir")), UUID.randomUUID().toString());
+	}
 }
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/AlternateRepositoryDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/lib/AlternateRepositoryDatabase.java
index 68ad488..70ce505 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/AlternateRepositoryDatabase.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/AlternateRepositoryDatabase.java
@@ -130,4 +130,9 @@ protected void closeAlternates(final ObjectDatabase[] alt) {
 	public List<PackFile> listLocalPacks() {
 		return odb.listLocalPacks();
 	}
+
+	@Override
+	public void updateInfoCache() throws IOException {
+		odb.updateInfoCache();
+	}
 }
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/CachedPacksInfoFileContentsGenerator.java b/org.spearce.jgit/src/org/spearce/jgit/lib/CachedPacksInfoFileContentsGenerator.java
new file mode 100644
index 0000000..6046c94
--- /dev/null
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/CachedPacksInfoFileContentsGenerator.java
@@ -0,0 +1,63 @@
+/*
+ * Copyright (C) 2009, Mike Gaffney.
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package org.spearce.jgit.lib;
+
+import java.util.List;
+
+/**
+ * This file is used to generate the contents of the file system
+ * based pack file cache used by the dumb git-http client protocol.
+ * @author mike
+ */
+public class CachedPacksInfoFileContentsGenerator {
+
+	private List<PackFile> packs;
+
+	public CachedPacksInfoFileContentsGenerator(List<PackFile> packs) {
+		this.packs = packs;
+	}
+	
+	public String generateContents(){
+		StringBuilder builder = new StringBuilder();
+		for (PackFile packFile : packs) {
+			builder.append("P ").append(packFile.getPackFile().getName()).append('\n');
+		}
+		builder.append('\n');
+		return builder.toString();
+	}
+	
+}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Constants.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Constants.java
index 9afea67..2d78dda 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Constants.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Constants.java
@@ -224,6 +224,9 @@
 
 	/** Info refs folder */
 	public static final String INFO_REFS = "info/refs";
+	
+	/** cached packs file */
+	public static final String CACHED_PACKS_FILE = "info/packs"; 
 
 	/** Packed refs file */
 	public static final String PACKED_REFS = "packed-refs";
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDatabase.java
index 722c802..5ded7bb 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDatabase.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDatabase.java
@@ -75,6 +75,14 @@ protected ObjectDatabase() {
 	public abstract List<PackFile> listLocalPacks();
 	
 	/**
+	 * Creates the caches that are typically done by 
+	 * update-server-info, namely objects/info/packs and 
+	 * info/refs
+	 * @throws IOException 
+	 */
+	public abstract void updateInfoCache() throws IOException;
+	
+	/**
 	 * Does this database exist yet?
 	 *
 	 * @return true if this database is already created; false if the caller
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java
index cbe132d..f4251c1 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java
@@ -514,4 +514,9 @@ boolean tryAgain(final long currLastModified) {
 		tryAgain1();
 		return new ArrayList<PackFile>(Arrays.asList(packList.get().packs));
 	}
+
+	@Override
+	public void updateInfoCache() throws IOException {
+		new UpdateDirectoryBasedPacksInfoCache(this.listLocalPacks(), new File(this.getDirectory(), Constants.CACHED_PACKS_FILE)).execute();
+	}
 }
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/UpdateDirectoryBasedPacksInfoCache.java b/org.spearce.jgit/src/org/spearce/jgit/lib/UpdateDirectoryBasedPacksInfoCache.java
new file mode 100644
index 0000000..327bb34
--- /dev/null
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/UpdateDirectoryBasedPacksInfoCache.java
@@ -0,0 +1,62 @@
+/*
+ * Copyright (C) 2009, Mike Gaffney.
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package org.spearce.jgit.lib;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.util.List;
+
+public class UpdateDirectoryBasedPacksInfoCache {
+
+	private List<PackFile> packsList;
+	private File infoPacksFile;
+
+	public UpdateDirectoryBasedPacksInfoCache(List<PackFile> packsList,
+									File infoPacksFile) {
+		this.packsList = packsList;
+		this.infoPacksFile = infoPacksFile;
+	}
+
+	public void execute() throws IOException {
+		String packsContents = new CachedPacksInfoFileContentsGenerator(packsList).generateContents();
+		FileOutputStream fos = new FileOutputStream(infoPacksFile);
+		fos.write(packsContents.getBytes());
+		fos.close();
+	}
+
+}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/UpdateDirectoryInfoCache.java b/org.spearce.jgit/src/org/spearce/jgit/lib/UpdateDirectoryInfoCache.java
new file mode 100644
index 0000000..b6947ce
--- /dev/null
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/UpdateDirectoryInfoCache.java
@@ -0,0 +1,26 @@
+package org.spearce.jgit.lib;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.util.List;
+
+public class UpdateDirectoryInfoCache {
+
+	private List<PackFile> packsList;
+	private File infoPacksFile;
+
+	public UpdateDirectoryInfoCache(List<PackFile> packsList,
+									File infoPacksFile) {
+		this.packsList = packsList;
+		this.infoPacksFile = infoPacksFile;
+	}
+
+	public void execute() throws IOException {
+		String packsContents = new CachedPacksInfoFileContentsGenerator(packsList).generateContents();
+		FileOutputStream fos = new FileOutputStream(infoPacksFile);
+		fos.write(packsContents.getBytes());
+		fos.close();
+	}
+
+}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/ReceivePack.java b/org.spearce.jgit/src/org/spearce/jgit/transport/ReceivePack.java
index eb21254..5865736 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/ReceivePack.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/ReceivePack.java
@@ -521,6 +521,16 @@ void sendString(final String s) throws IOException {
 			}
 
 			postReceive.onPostReceive(this, filterCommands(Result.OK));
+			updateObjectInfoCache();
+		}
+	}
+
+	private void updateObjectInfoCache() {
+		try{
+			getRepository().getObjectDatabase().updateInfoCache();
+		} 
+		catch (IOException e){
+			sendMessage("error updating server info: " + e.getMessage());
 		}
 	}
 
-- 
1.6.4.2

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

* [PATCH JGit 4/5] Adding in a InfoDatabase like ObjectDatabase and and implementation based upon a directory.
  2009-09-16  0:48     ` [PATCH JGit 3/5] Implemented directory based info cache for objects/info/packs mr.gaffo
@ 2009-09-16  0:48       ` mr.gaffo
  2009-09-16  0:48         ` [PATCH JGit 5/5] added tests for the file based info cache update and made pass mr.gaffo
  2009-10-08 17:05         ` [PATCH JGit 4/5] Adding in a InfoDatabase like ObjectDatabase and and implementation based upon a directory Shawn O. Pearce
  2009-10-08 17:00       ` [PATCH JGit 3/5] Implemented directory based info cache for objects/info/packs Shawn O. Pearce
  1 sibling, 2 replies; 12+ messages in thread
From: mr.gaffo @ 2009-09-16  0:48 UTC (permalink / raw)
  To: git; +Cc: Mike Gaffney

From: Mike Gaffney <mr.gaffo@gmail.com>

Signed-off-by: Mike Gaffney <mr.gaffo@gmail.com>
---
 .../jgit/lib/InfoDirectoryDatabaseTest.java        |   66 ++++++++++++++++++++
 .../org/spearce/jgit/lib/ObjectDirectoryTest.java  |    1 -
 .../src/org/spearce/jgit/lib/InfoDatabase.java     |   44 +++++++++++++
 .../spearce/jgit/lib/InfoDirectoryDatabase.java    |   54 ++++++++++++++++
 .../src/org/spearce/jgit/lib/Repository.java       |   11 +++
 5 files changed, 175 insertions(+), 1 deletions(-)
 create mode 100644 org.spearce.jgit.test/tst/org/spearce/jgit/lib/InfoDirectoryDatabaseTest.java
 create mode 100644 org.spearce.jgit/src/org/spearce/jgit/lib/InfoDatabase.java
 create mode 100644 org.spearce.jgit/src/org/spearce/jgit/lib/InfoDirectoryDatabase.java

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/InfoDirectoryDatabaseTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/InfoDirectoryDatabaseTest.java
new file mode 100644
index 0000000..066473d
--- /dev/null
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/InfoDirectoryDatabaseTest.java
@@ -0,0 +1,66 @@
+/*
+ * Copyright (C) 2009, Mike Gaffney.
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package org.spearce.jgit.lib;
+
+import java.io.File;
+
+import org.spearce.jgit.util.JGitTestUtil;
+
+import junit.framework.TestCase;
+
+public class InfoDirectoryDatabaseTest extends TestCase {
+
+	private File testDir;
+
+	@Override
+	protected void setUp() throws Exception {
+		testDir = JGitTestUtil.generateTempDirectoryFileObject();
+	}
+
+	@Override
+	protected void tearDown() throws Exception {
+		if (testDir.exists()){
+			JGitTestUtil.recursiveDelete(testDir, false, getClass().getName() + "." + getName(), true);
+		}
+	}
+	
+	public void testCreateCreatesDirectory() throws Exception {
+		assertFalse(testDir.exists());
+		new InfoDirectoryDatabase(testDir).create();
+		assertTrue(testDir.exists());
+	}
+}
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectDirectoryTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectDirectoryTest.java
index 204fb7c..8c1d32d 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectDirectoryTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectDirectoryTest.java
@@ -38,7 +38,6 @@
 
 import java.io.File;
 import java.io.IOException;
-import java.util.ArrayList;
 import java.util.List;
 
 import org.spearce.jgit.util.JGitTestUtil;
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/InfoDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/lib/InfoDatabase.java
new file mode 100644
index 0000000..2a8d88d
--- /dev/null
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/InfoDatabase.java
@@ -0,0 +1,44 @@
+/*
+ * Copyright (C) 2009, Mike Gaffney.
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package org.spearce.jgit.lib;
+
+public abstract class InfoDatabase {
+
+	public void create() {
+	}
+
+}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/InfoDirectoryDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/lib/InfoDirectoryDatabase.java
new file mode 100644
index 0000000..90655e8
--- /dev/null
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/InfoDirectoryDatabase.java
@@ -0,0 +1,54 @@
+/*
+ * Copyright (C) 2009, Mike Gaffney.
+ *
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or
+ * without modification, are permitted provided that the following
+ * conditions are met:
+ *
+ * - Redistributions of source code must retain the above copyright
+ *   notice, this list of conditions and the following disclaimer.
+ *
+ * - Redistributions in binary form must reproduce the above
+ *   copyright notice, this list of conditions and the following
+ *   disclaimer in the documentation and/or other materials provided
+ *   with the distribution.
+ *
+ * - Neither the name of the Git Development Community nor the
+ *   names of its contributors may be used to endorse or promote
+ *   products derived from this software without specific prior
+ *   written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
+ * CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES,
+ * INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR
+ * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
+ * NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
+ * STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
+ * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package org.spearce.jgit.lib;
+
+import java.io.File;
+
+public class InfoDirectoryDatabase extends InfoDatabase {
+
+	private File info;
+
+	public InfoDirectoryDatabase(final File directory) {
+		info = directory;
+	}
+	
+	@Override
+	public void create() {
+		info.mkdirs();
+	}
+
+}
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
index 46b7804..f658b5c 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Repository.java
@@ -97,6 +97,8 @@
 	private final RefDatabase refs;
 
 	private final ObjectDirectory objectDatabase;
+	
+	private final InfoDatabase infoDatabase;
 
 	private GitIndex index;
 
@@ -116,6 +118,7 @@ public Repository(final File d) throws IOException {
 		gitDir = d.getAbsoluteFile();
 		refs = new RefDatabase(this);
 		objectDatabase = new ObjectDirectory(FS.resolve(gitDir, "objects"));
+		infoDatabase = new InfoDirectoryDatabase(FS.resolve(gitDir, "info"));
 
 		final FileBasedConfig userConfig;
 		userConfig = SystemReader.getInstance().openUserConfig();
@@ -177,6 +180,7 @@ public void create(boolean bare) throws IOException {
 		gitDir.mkdirs();
 		refs.create();
 		objectDatabase.create();
+		infoDatabase.create();
 
 		new File(gitDir, "branches").mkdir();
 		new File(gitDir, "remotes").mkdir();
@@ -210,6 +214,13 @@ public File getObjectsDirectory() {
 	public ObjectDatabase getObjectDatabase() {
 		return objectDatabase;
 	}
+	
+	/**
+	 * @return the info database which stores this repository's info
+	 */
+	public InfoDatabase getInfoDatabase() {
+		return infoDatabase;
+	}
 
 	/**
 	 * @return the configuration of this repository
-- 
1.6.4.2

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

* [PATCH JGit 5/5] added tests for the file based info cache update and made pass
  2009-09-16  0:48       ` [PATCH JGit 4/5] Adding in a InfoDatabase like ObjectDatabase and and implementation based upon a directory mr.gaffo
@ 2009-09-16  0:48         ` mr.gaffo
  2009-10-08 17:12           ` Shawn O. Pearce
  2009-10-08 17:05         ` [PATCH JGit 4/5] Adding in a InfoDatabase like ObjectDatabase and and implementation based upon a directory Shawn O. Pearce
  1 sibling, 1 reply; 12+ messages in thread
From: mr.gaffo @ 2009-09-16  0:48 UTC (permalink / raw)
  To: git; +Cc: mike.gaffney, Mike Gaffney

From: mike.gaffney <mike.gaffney@asolutions.com>

Signed-off-by: Mike Gaffney <mr.gaffo@gmail.com>
---
 .../CachedPacksInfoFileContentsGeneratorTest.java  |    8 ++--
 .../jgit/lib/InfoDirectoryDatabaseTest.java        |   30 ++++++++++++++++++++
 .../org/spearce/jgit/lib/ObjectDirectoryTest.java  |    4 +-
 .../src/org/spearce/jgit/lib/InfoDatabase.java     |   15 ++++++++++
 .../spearce/jgit/lib/InfoDirectoryDatabase.java    |   15 ++++++++++
 .../org/spearce/jgit/transport/ReceivePack.java    |   10 ++++++
 6 files changed, 76 insertions(+), 6 deletions(-)

diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/CachedPacksInfoFileContentsGeneratorTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/CachedPacksInfoFileContentsGeneratorTest.java
index bea0b70..10ce9e3 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/CachedPacksInfoFileContentsGeneratorTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/CachedPacksInfoFileContentsGeneratorTest.java
@@ -63,10 +63,10 @@ public void testGettingPacksContentsMultiplePacks() throws Exception {
 		packs.add(new PackFile(TEST_IDX, TEST_PACK));
 		
 		StringBuilder expected = new StringBuilder();
-		expected.append("P ").append(TEST_PACK.getName()).append("\n");
-		expected.append("P ").append(TEST_PACK.getName()).append("\n");
-		expected.append("P ").append(TEST_PACK.getName()).append("\n");
-		expected.append("\n");
+		expected.append("P ").append(TEST_PACK.getName()).append('\n');
+		expected.append("P ").append(TEST_PACK.getName()).append('\n');
+		expected.append("P ").append(TEST_PACK.getName()).append('\n');
+		expected.append('\n');
 		
 		assertEquals(expected.toString(), new CachedPacksInfoFileContentsGenerator(packs).generateContents());
 	}
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/InfoDirectoryDatabaseTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/InfoDirectoryDatabaseTest.java
index 066473d..e31b883 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/InfoDirectoryDatabaseTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/InfoDirectoryDatabaseTest.java
@@ -37,6 +37,10 @@
 package org.spearce.jgit.lib;
 
 import java.io.File;
+import java.io.IOException;
+import java.io.StringWriter;
+import java.util.ArrayList;
+import java.util.Collection;
 
 import org.spearce.jgit.util.JGitTestUtil;
 
@@ -63,4 +67,30 @@ public void testCreateCreatesDirectory() throws Exception {
 		new InfoDirectoryDatabase(testDir).create();
 		assertTrue(testDir.exists());
 	}
+	
+	public void testUpdateInfoCache() throws Exception {
+		Collection<Ref> refs = new ArrayList<Ref>();
+		refs.add(new Ref(Ref.Storage.LOOSE, "refs/heads/master", ObjectId.fromString("32aae7aef7a412d62192f710f2130302997ec883")));
+		refs.add(new Ref(Ref.Storage.LOOSE, "refs/heads/development", ObjectId.fromString("184063c9b594f8968d61a686b2f6052779551613")));
+
+		File expectedFile = new File(testDir, "refs");
+		assertFalse(expectedFile.exists());
+		
+		
+		final StringWriter expectedString = new StringWriter();
+		new RefWriter(refs) {
+			@Override
+			protected void writeFile(String file, byte[] content) throws IOException {
+				expectedString.write(new String(content));
+			}
+		}.writeInfoRefs();
+		
+		InfoDirectoryDatabase out = new InfoDirectoryDatabase(testDir);
+		out.create();
+		out.updateInfoCache(refs);
+		assertTrue(expectedFile.exists());
+		
+		String actual = JGitTestUtil.readFileAsString(expectedFile);
+		assertEquals(expectedString.toString(), actual);
+	}
 }
diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectDirectoryTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectDirectoryTest.java
index 8c1d32d..a3f5278 100644
--- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectDirectoryTest.java
+++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectDirectoryTest.java
@@ -40,10 +40,10 @@
 import java.io.IOException;
 import java.util.List;
 
-import org.spearce.jgit.util.JGitTestUtil;
-
 import junit.framework.TestCase;
 
+import org.spearce.jgit.util.JGitTestUtil;
+
 public class ObjectDirectoryTest extends TestCase {
 	private static final String PACK_NAME = "pack-34be9032ac282b11fa9babdc2b2a93ca996c9c2f";
 	private static final File TEST_PACK = JGitTestUtil.getTestResourceFile(PACK_NAME + ".pack");
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/InfoDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/lib/InfoDatabase.java
index 2a8d88d..96a39fc 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/InfoDatabase.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/InfoDatabase.java
@@ -36,9 +36,24 @@
  */
 package org.spearce.jgit.lib;
 
+import java.io.IOException;
+import java.util.Collection;
+
 public abstract class InfoDatabase {
 
+	/**
+	 * Create the info database
+	 */
 	public void create() {
 	}
 
+	/**
+	 * Updates the info cache typically done by update-server-info command.
+	 * This writes THIS repository's refs out to the info/refs file.
+	 * @param collection the collections of refs to update the info cache with
+	 * @throws IOException for any type of failure on the local or remote 
+	 * 					   data store
+	 */
+	public abstract void updateInfoCache(Collection<Ref> collection) throws IOException;
+
 }
diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/InfoDirectoryDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/lib/InfoDirectoryDatabase.java
index 90655e8..48f60d1 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/lib/InfoDirectoryDatabase.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/lib/InfoDirectoryDatabase.java
@@ -37,6 +37,9 @@
 package org.spearce.jgit.lib;
 
 import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.util.Collection;
 
 public class InfoDirectoryDatabase extends InfoDatabase {
 
@@ -51,4 +54,16 @@ public void create() {
 		info.mkdirs();
 	}
 
+	@Override
+	public void updateInfoCache(Collection<Ref> refs) throws IOException {
+		new RefWriter(refs) {
+			@Override
+			protected void writeFile(String file, byte[] content) throws IOException {
+				FileOutputStream fos = new FileOutputStream(new File(info, "refs"));
+				fos.write(content);
+				fos.close();
+			}
+		}.writeInfoRefs();
+	}
+
 }
diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/ReceivePack.java b/org.spearce.jgit/src/org/spearce/jgit/transport/ReceivePack.java
index 5865736..23277c9 100644
--- a/org.spearce.jgit/src/org/spearce/jgit/transport/ReceivePack.java
+++ b/org.spearce.jgit/src/org/spearce/jgit/transport/ReceivePack.java
@@ -522,6 +522,16 @@ void sendString(final String s) throws IOException {
 
 			postReceive.onPostReceive(this, filterCommands(Result.OK));
 			updateObjectInfoCache();
+			updateInfoRefsCache();
+		}
+	}
+
+	private void updateInfoRefsCache() {
+		try{
+			getRepository().getInfoDatabase().updateInfoCache(getRepository().getAllRefs().values());
+		}
+		catch (IOException e){
+			sendMessage("error updating info/refs: " + e.getMessage());
 		}
 	}
 
-- 
1.6.4.2

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

* Re: [PATCH JGit 1/5] adding tests for ObjectDirectory
  2009-09-16  0:48 ` [PATCH JGit 1/5] adding tests for ObjectDirectory mr.gaffo
  2009-09-16  0:48   ` [PATCH JGit 2/5] Create abstract method on ObjectDatabase for accessing the list of local pack files mr.gaffo
@ 2009-09-21 19:30   ` Shawn O. Pearce
  1 sibling, 0 replies; 12+ messages in thread
From: Shawn O. Pearce @ 2009-09-21 19:30 UTC (permalink / raw)
  To: mr.gaffo; +Cc: git, mike.gaffney

mr.gaffo@gmail.com wrote:
> diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectDirectoryTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectDirectoryTest.java
> +	private File testDir;
> +
> +	@Override
> +	protected void setUp() throws Exception {
> +		testDir = new File(new File(System.getProperty("java.io.tmpdir")), UUID.randomUUID().toString());
> +	}

Can't we use the same logic we use in RepositoryTestCase to create
the temporary directory for this test?  I would rather keep the
temporary space under target/ when testing under Maven, as it
makes it far easier to clean up the directory.  Plus we know we
have sufficient write space there.

> +	@Override
> +	protected void tearDown() throws Exception {
> +		if (testDir.exists()){

Style nit: Space between ) and {

> +	public void testExistsWithNonExistantDirectory() throws Exception {
> +		assertFalse(new ObjectDirectory(new File("/some/nonexistant/file")).exists());

Please create a path name below your testDir which you know won't
exist.  I don't want this test to rely upon the fact that some
absolute path doesn't exist that is outside of our namespace control.

> +	private void createTestDir(){

You use this method once, inline it inside
testExistsWithExistingDirectory().

Otherwise, the test case is OK, but is still quite sparse with
regards to functionality of the class being tested.  Was it your
intention to only cover the most basic parts at this time?  Its more
coverage than we have now, so I'm happy, but just wanted to point
out it certainly isn't complete (e.g. no pack support).

-- 
Shawn.

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

* Re: [PATCH JGit 2/5] Create abstract method on ObjectDatabase for accessing the list of local pack files.
  2009-09-16  0:48   ` [PATCH JGit 2/5] Create abstract method on ObjectDatabase for accessing the list of local pack files mr.gaffo
  2009-09-16  0:48     ` [PATCH JGit 3/5] Implemented directory based info cache for objects/info/packs mr.gaffo
@ 2009-09-21 19:40     ` Shawn O. Pearce
  2009-09-21 19:51       ` Michael Gaffney
  1 sibling, 1 reply; 12+ messages in thread
From: Shawn O. Pearce @ 2009-09-21 19:40 UTC (permalink / raw)
  To: mr.gaffo; +Cc: git, mike.gaffney

mr.gaffo@gmail.com wrote:
> diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectDirectoryTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectDirectoryTest.java
> +	public void testListLocalPacksWhenThereIsAPack() throws Exception {
> +		createTestDir();
> +		File packsDir = new File(testDir, "pack");
> +		packsDir.mkdirs();

Why not allow the ObjectDirectory code to create the directory
before copying the pack into it?

> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDatabase.java
> +	/**
> +	 * The list of Packs THIS repo contains

Don't you mean the list of packs this object database contains?
An object database may not be a git repository.  Though yes, the
common case is that it is a repository.

> +	 * @return List<PackFile> of package names contained in this repo. 
> +	 * 		   Should be an empty list if there are none.
> +	 */
> +	public abstract List<PackFile> listLocalPacks();

I think you should define this to be an unmodifiable list, not just
any list.  Its sad that the Java type system didn't support this
idea back when they added the new collections APIs.

> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java
> +	@Override
> +	public List<PackFile> listLocalPacks() {
> +		tryAgain1();
> +		return new ArrayList<PackFile>(Arrays.asList(packList.get().packs));

Instead of copying, why not return an unmodifiableList wrapped
around the array?  PackList will never modify its internal array.

-- 
Shawn.

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

* Re: [PATCH JGit 2/5] Create abstract method on ObjectDatabase for accessing the list of local pack files.
  2009-09-21 19:40     ` [PATCH JGit 2/5] Create abstract method on ObjectDatabase for accessing the list of local pack files Shawn O. Pearce
@ 2009-09-21 19:51       ` Michael Gaffney
  0 siblings, 0 replies; 12+ messages in thread
From: Michael Gaffney @ 2009-09-21 19:51 UTC (permalink / raw)
  To: Shawn O. Pearce; +Cc: git

Shawn O. Pearce wrote:
> Why not allow the ObjectDirectory code to create the directory
> before copying the pack into it?

Good point, this was one of the first tests I did before I got to that 
part of ObjectDirectory. Will fix.

> Don't you mean the list of packs this object database contains?
> An object database may not be a git repository.  Though yes, the
> common case is that it is a repository.

What's the difference in terminology? Not aruging, just wanting to know 
what we're calling a repo and what we're not so that I use it correctly. 
Will fix.

>> +	public abstract List<PackFile> listLocalPacks();
> 
> I think you should define this to be an unmodifiable list, not just
> any list.  Its sad that the Java type system didn't support this
> idea back when they added the new collections APIs.

Should it be a collection as well instead of a list; what would you 
specifically suggest?

> Instead of copying, why not return an unmodifiableList wrapped
> around the array?  PackList will never modify its internal array.

Same as above


-Mike

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

* Re: [PATCH JGit 3/5] Implemented directory based info cache for objects/info/packs.
  2009-09-16  0:48     ` [PATCH JGit 3/5] Implemented directory based info cache for objects/info/packs mr.gaffo
  2009-09-16  0:48       ` [PATCH JGit 4/5] Adding in a InfoDatabase like ObjectDatabase and and implementation based upon a directory mr.gaffo
@ 2009-10-08 17:00       ` Shawn O. Pearce
  1 sibling, 0 replies; 12+ messages in thread
From: Shawn O. Pearce @ 2009-10-08 17:00 UTC (permalink / raw)
  To: mr.gaffo; +Cc: git

mr.gaffo@gmail.com wrote:
> Add abstract method for updating the object db's info cache to directory.
> 
> Implemented passthrough on Alternate for the update of infocache.
> 
> Added utility that generates the contents of the objects/info/packs
> file as a string from a list of PackFiles.
> 
> Added implementation from ObjectDirectory on down
> for creating the info cache.
> 
> Added test for creating the info cache

Reading this message gave me the funny feeling that I'm going to
see a lot of unrelated code mashed into one patch.  There doesn't
seem to be a general theme to the commit, at least as far as the
message is concerned.

[a bit later...] After reading the code itself, I agree with the
original guess on the commit message, there is too much happening
in this one patch that is unrelated, and there are several problems
lurking that are harder to spot because its a mash of changes.
Please try to break it down to more focused commits.
 
> diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/CachedPacksInfoFileContentsGeneratorTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/CachedPacksInfoFileContentsGeneratorTest.java
> new file mode 100644
> index 0000000..bea0b70
> --- /dev/null
> +++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/CachedPacksInfoFileContentsGeneratorTest.java
> @@ -0,0 +1,74 @@

FYI, our new package space is org.eclipse.jgit and any new changes
need to take that into account.

> + * - Neither the name of the Git Development Community nor the

Also FYI, since our move to the Eclipse Foundation the generic term
"Git Development Community" has been replaced in the license header
by "Eclipse Foundation, Inc.", otherwise the license header remains
as-is and copyright is still attributed to the contributor.

> +public class CachedPacksInfoFileContentsGeneratorTest extends TestCase {
...
> +	public void testGettingPacksContentsMultiplePacks() throws Exception {
> +		List<PackFile> packs = new ArrayList<PackFile>();
> +		packs.add(new PackFile(TEST_IDX, TEST_PACK));
> +		packs.add(new PackFile(TEST_IDX, TEST_PACK));
> +		packs.add(new PackFile(TEST_IDX, TEST_PACK));

I think we should be testing multiple names here, to ensure the
generator didn't do something stupid like reuse the same array index
for pulling the name while looping for the size of the input list.

> diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectDirectoryTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectDirectoryTest.java
> @@ -54,9 +54,9 @@
>  
>  	@Override
>  	protected void setUp() throws Exception {
> -		testDir = new File(new File(System.getProperty("java.io.tmpdir")), UUID.randomUUID().toString());
> +		testDir = JGitTestUtil.generateTempDirectoryFileObject();

Yea, I'm confused about this hunk.  It isn't strictly necessary
for the new feature this commit is adding.  Pull this into its own
commit, before the new feature, so you can take advantage of the
refactoring in your new tests, but you also don't muddle the new
feature addition with old code refactoring.

> diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/util/JGitTestUtil.java b/org.spearce.jgit.test/tst/org/spearce/jgit/util/JGitTestUtil.java
> @@ -74,7 +76,8 @@ public static File getTestResourceFile(final String fileName) {
>  		}
>  	}
>  
> -	public static void copyFile(final File fromFile, final File toFile) throws IOException {
> +	public static void copyFile(final File fromFile, final File toFile)
> +			throws IOException {
>  		InputStream in = new FileInputStream(fromFile);
>  		OutputStream out = new FileOutputStream(toFile);

Unnecessary reformatting hunk; we try to avoid these when possible.

> @@ -87,6 +90,21 @@ public static void copyFile(final File fromFile, final File toFile) throws IOExc
>  		out.close();
>  	}
>  
> +	public static String readFileAsString(final File file)
> +			throws java.io.IOException {

This method already exists in some form in the RepositoryTest class.
Can we instead make a commit to refactor it out of there here?

> +		StringBuilder fileData = new StringBuilder(1000);
> +		BufferedReader reader = new BufferedReader(new FileReader(file));

We should be more specific about our encoding and not rely on the
platform default.

> +		char[] buf = new char[1024];
> +		int numRead = 0;
> +		while ((numRead = reader.read(buf)) != -1) {
> +			String readData = String.valueOf(buf, 0, numRead);
> +			fileData.append(readData);
> +			buf = new char[1024];

There is no need to reallocate the buffer, String.valueOf is required
to copy the array to ensure the returned String is immutable.
Worse, you don't need to convert the char array to string, there is
a method on StringBuilder to append a char[] taking char[],int,int.

> @@ -136,4 +154,8 @@ private static void reportDeleteFailure(final String name,
> +
> +	public static File generateTempDirectoryFileObject() {
> +		return new File(new File(System.getProperty("java.io.tmpdir")), UUID.randomUUID().toString());
> +	}

Please generate temporary directories under the same area that
RepositoryTest produces them, which makes it easier to cleanup,
and ensures we aren't treading out of our build area.

> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/AlternateRepositoryDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/lib/AlternateRepositoryDatabase.java
> index 68ad488..70ce505 100644
> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/AlternateRepositoryDatabase.java
> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/AlternateRepositoryDatabase.java
> @@ -130,4 +130,9 @@ protected void closeAlternates(final ObjectDatabase[] alt) {
>  	public List<PackFile> listLocalPacks() {
>  		return odb.listLocalPacks();
>  	}
> +
> +	@Override
> +	public void updateInfoCache() throws IOException {
> +		odb.updateInfoCache();
> +	}
>  }
> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/CachedPacksInfoFileContentsGenerator.java b/org.spearce.jgit/src/org/spearce/jgit/lib/CachedPacksInfoFileContentsGenerator.java
> + * This file is used to generate the contents of the file system
> + * based pack file cache used by the dumb git-http client protocol.
> + * @author mike

We don't record @author annotations.

> +public class CachedPacksInfoFileContentsGenerator {
> +
> +	private List<PackFile> packs;
> +
> +	public CachedPacksInfoFileContentsGenerator(List<PackFile> packs) {
> +		this.packs = packs;
> +	}
> +	
> +	public String generateContents(){

Style nit: space between () and {.

I also wonder about the value of an instance for this class, if
all it does is produce one return value as a string, why not just
use a static method?

> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/Constants.java b/org.spearce.jgit/src/org/spearce/jgit/lib/Constants.java
> index 9afea67..2d78dda 100644
> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/Constants.java
> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/Constants.java
> @@ -224,6 +224,9 @@
>  
>  	/** Info refs folder */
>  	public static final String INFO_REFS = "info/refs";
> +	
> +	/** cached packs file */
> +	public static final String CACHED_PACKS_FILE = "info/packs"; 

I think you should denote in the comment that this is relative to
the objects directory, and not the repository.  The INFO_REFS file
right above you is relative to the repository and something looks
wrong seeing these together in context.
  
> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDatabase.java
> index 722c802..5ded7bb 100644
> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDatabase.java
> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDatabase.java
> @@ -75,6 +75,14 @@ protected ObjectDatabase() {
>  	public abstract List<PackFile> listLocalPacks();
>  	
>  	/**
> +	 * Creates the caches that are typically done by 
> +	 * update-server-info, namely objects/info/packs and 
> +	 * info/refs

Why is the object database updating info/refs?  The info/refs file
has nothing to do with the raw object storage.

> +	 * @throws IOException 
> +	 */
> +	public abstract void updateInfoCache() throws IOException;

I'm not sure I'm happy with this being on ObjectDatabase but there
may not be any other choice.  We'd like to eventually have other
types of ObjectDatabase that don't even store packs, in such a
database updating the info cache makes no sense.  Asking them to
implement the operation is silly.  But... we have no other way to
easily signal the database that it should do this update.

Hmmph.  I wonder if it might be better to configure the
ObjectDirectory at creation time to automatically maintain
the info/packs file during openPack, and put this method on
ObjectDirectory for explicit invocation in case someone has made
edits outside of JGit and wants to force the cache to be current
again.

I'm not sold either way yet.  I'm still open on the approach.

> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java
> index cbe132d..f4251c1 100644
> --- a/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java
> +++ b/org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java
> @@ -514,4 +514,9 @@ boolean tryAgain(final long currLastModified) {
>  		tryAgain1();
>  		return new ArrayList<PackFile>(Arrays.asList(packList.get().packs));
>  	}
> +
> +	@Override
> +	public void updateInfoCache() throws IOException {
> +		new UpdateDirectoryBasedPacksInfoCache(this.listLocalPacks(), new File(this.getDirectory(), Constants.CACHED_PACKS_FILE)).execute();

Style-nit: Line is far too long, we try to wrap at around 80
characters but sometimes allow a line to go longer if its only
longer by 1 or 2 characters and the wrapped result would be harder
to read than the unwrapped result.  This is out at 140, too many
over the limit.

> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/UpdateDirectoryBasedPacksInfoCache.java b/org.spearce.jgit/src/org/spearce/jgit/lib/UpdateDirectoryBasedPacksInfoCache.java
> +public class UpdateDirectoryBasedPacksInfoCache {
> +	public void execute() throws IOException {
> +		String packsContents = new CachedPacksInfoFileContentsGenerator(packsList).generateContents();
> +		FileOutputStream fos = new FileOutputStream(infoPacksFile);
> +		fos.write(packsContents.getBytes());
> +		fos.close();

I'm not sure why this 4 line method requires its own top level
class and being able to create an instance.  Code bloat?

You need to specify the character encoding here and not rely on
the platform default.

I'm not sure how C Git handles this update, but we probably should be
safer about it and use the LockFile class so the update is going to
be transactional.  Here you are truncating a live file, which means
readers could see an empty content if they happen to request the file
from the web server at the wrong moment.  LockFile does the writing
to a temporary file and then renames it over, which on a POSIX file
system means there is no way someone can see a partial update.

> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/UpdateDirectoryInfoCache.java b/org.spearce.jgit/src/org/spearce/jgit/lib/UpdateDirectoryInfoCache.java
> +public class UpdateDirectoryInfoCache {
> +	public void execute() throws IOException {
> +		String packsContents = new CachedPacksInfoFileContentsGenerator(packsList).generateContents();
> +		FileOutputStream fos = new FileOutputStream(infoPacksFile);
> +		fos.write(packsContents.getBytes());
> +		fos.close();

Uh, isn't this the same as UpdateDirectoryBasedPacksInfoCache?
Two top level classes for a 4 line method?

> diff --git a/org.spearce.jgit/src/org/spearce/jgit/transport/ReceivePack.java b/org.spearce.jgit/src/org/spearce/jgit/transport/ReceivePack.java
> +	private void updateObjectInfoCache() {
> +		try{

Style-nit: space after try

> +			getRepository().getObjectDatabase().updateInfoCache();
> +		} 
> +		catch (IOException e){

Style-nit: space between ) and {

> +			sendMessage("error updating server info: " + e.getMessage());

-- 
Shawn.

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

* Re: [PATCH JGit 4/5] Adding in a InfoDatabase like ObjectDatabase and and implementation based upon a directory.
  2009-09-16  0:48       ` [PATCH JGit 4/5] Adding in a InfoDatabase like ObjectDatabase and and implementation based upon a directory mr.gaffo
  2009-09-16  0:48         ` [PATCH JGit 5/5] added tests for the file based info cache update and made pass mr.gaffo
@ 2009-10-08 17:05         ` Shawn O. Pearce
  1 sibling, 0 replies; 12+ messages in thread
From: Shawn O. Pearce @ 2009-10-08 17:05 UTC (permalink / raw)
  To: mr.gaffo; +Cc: git

mr.gaffo@gmail.com wrote:
> From: Mike Gaffney <mr.gaffo@gmail.com>
> Subject: Re: [PATCH JGit 4/5] Adding in a InfoDatabase like ObjectDatabase
> and and implementation based upon a directory.

Typo on "and and".

We should have a bit more justification for this change, the subject
sounds aggressive, but there's no rationle for 175 insertions.
You and I both can make a reaosnable guess about why, but not
everyone knows the code or what you are trying to accomplish.

> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/InfoDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/lib/InfoDatabase.java
> +public abstract class InfoDatabase {
> +
> +	public void create() {
> +	}

New public code should have Javadoc to document its purpose and
usage, especially for an abstract class that needs to be implemented.

But, that said, I think this direction is of dubious value.  What we
really care about is having the contents of the current RefDatabase
(that is, packed-refs and files under refs/) written into info/refs.

There really isn't anything else of value under GIT_DIR/info, other
than GIT_DIR/info/exclude, but that is related to ignore processing
for a repository with a working directory and isn't something that
a bare repository on a server ever cares about.

IMHO, updating GIT_DIR/info/refs should be part of RefDatabase,
not some new InfoDirectoryDatabase class.

-- 
Shawn.

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

* Re: [PATCH JGit 5/5] added tests for the file based info cache update and made pass
  2009-09-16  0:48         ` [PATCH JGit 5/5] added tests for the file based info cache update and made pass mr.gaffo
@ 2009-10-08 17:12           ` Shawn O. Pearce
  0 siblings, 0 replies; 12+ messages in thread
From: Shawn O. Pearce @ 2009-10-08 17:12 UTC (permalink / raw)
  To: mr.gaffo; +Cc: git, mike.gaffney

mr.gaffo@gmail.com wrote:
> From: mike.gaffney <mike.gaffney@asolutions.com>
> Subject: Re: [PATCH JGit 5/5] added tests for the file based info cache
>	update and made pass

"and made pass" is the sneaky way of saying "and I actually
implemented what I should have implemented in the prior commit,
but didn't because ..." ?
 
> diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/CachedPacksInfoFileContentsGeneratorTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/CachedPacksInfoFileContentsGeneratorTest.java
> index bea0b70..10ce9e3 100644
> --- a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/CachedPacksInfoFileContentsGeneratorTest.java
> +++ b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/CachedPacksInfoFileContentsGeneratorTest.java
> @@ -63,10 +63,10 @@ public void testGettingPacksContentsMultiplePacks() throws Exception {
>  		packs.add(new PackFile(TEST_IDX, TEST_PACK));
>  		
>  		StringBuilder expected = new StringBuilder();
> -		expected.append("P ").append(TEST_PACK.getName()).append("\n");
> -		expected.append("P ").append(TEST_PACK.getName()).append("\n");
> -		expected.append("P ").append(TEST_PACK.getName()).append("\n");
> -		expected.append("\n");
> +		expected.append("P ").append(TEST_PACK.getName()).append('\n');
> +		expected.append("P ").append(TEST_PACK.getName()).append('\n');
> +		expected.append("P ").append(TEST_PACK.getName()).append('\n');
> +		expected.append('\n');

This should be squashed to the patch that introduced the code,
not be twiddled in something that is completely unrelated to it.
  		
> diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/InfoDirectoryDatabaseTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/InfoDirectoryDatabaseTest.java
> +	public void testUpdateInfoCache() throws Exception {
> +		Collection<Ref> refs = new ArrayList<Ref>();
> +		refs.add(new Ref(Ref.Storage.LOOSE, "refs/heads/master", ObjectId.fromString("32aae7aef7a412d62192f710f2130302997ec883")));
> +		refs.add(new Ref(Ref.Storage.LOOSE, "refs/heads/development", ObjectId.fromString("184063c9b594f8968d61a686b2f6052779551613")));
> +
> +		File expectedFile = new File(testDir, "refs");
> +		assertFalse(expectedFile.exists());
> +		
> +		
> +		final StringWriter expectedString = new StringWriter();
> +		new RefWriter(refs) {
> +			@Override
> +			protected void writeFile(String file, byte[] content) throws IOException {
> +				expectedString.write(new String(content));
> +			}
> +		}.writeInfoRefs();

This feels a bit too much like testing the formatting code by
relying on the formatting code to produce the correct output.

Its a 2 line file with a very well known format that cannot change
without breaking every Git HTTP client out in the wild.  We will
not break those clients anytime in the next few years.  You have
the data hardcoded above *anyway*, hardcode the expected result
here to ensure we formatted it right.

Oh, and IIRC order doesn't matter in the file but I think almost
everyone assumes the order is as per git ls-remote, which matches the
order produced by RefComparator.  Which means you want to assert that
development comes before master, and that tags come before heads.

Also, we need to assert that the peeled information for a tag appears
in the file.  So you need a tag ref with a peeled ObjectId available.

> diff --git a/org.spearce.jgit/src/org/spearce/jgit/lib/InfoDirectoryDatabase.java b/org.spearce.jgit/src/org/spearce/jgit/lib/InfoDirectoryDatabase.java
> @@ -51,4 +54,16 @@ public void create() {
>  		info.mkdirs();
>  	}
>  
> +	@Override
> +	public void updateInfoCache(Collection<Ref> refs) throws IOException {
> +		new RefWriter(refs) {
> +			@Override
> +			protected void writeFile(String file, byte[] content) throws IOException {
> +				FileOutputStream fos = new FileOutputStream(new File(info, "refs"));
> +				fos.write(content);
> +				fos.close();

I think you need to use a LockFile to avoid races between readers
and writers.

-- 
Shawn.

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

end of thread, other threads:[~2009-10-08 17:21 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-09-16  0:48 [PATCH JGit] Adding update-server-info functionality try2 mr.gaffo
2009-09-16  0:48 ` [PATCH JGit 1/5] adding tests for ObjectDirectory mr.gaffo
2009-09-16  0:48   ` [PATCH JGit 2/5] Create abstract method on ObjectDatabase for accessing the list of local pack files mr.gaffo
2009-09-16  0:48     ` [PATCH JGit 3/5] Implemented directory based info cache for objects/info/packs mr.gaffo
2009-09-16  0:48       ` [PATCH JGit 4/5] Adding in a InfoDatabase like ObjectDatabase and and implementation based upon a directory mr.gaffo
2009-09-16  0:48         ` [PATCH JGit 5/5] added tests for the file based info cache update and made pass mr.gaffo
2009-10-08 17:12           ` Shawn O. Pearce
2009-10-08 17:05         ` [PATCH JGit 4/5] Adding in a InfoDatabase like ObjectDatabase and and implementation based upon a directory Shawn O. Pearce
2009-10-08 17:00       ` [PATCH JGit 3/5] Implemented directory based info cache for objects/info/packs Shawn O. Pearce
2009-09-21 19:40     ` [PATCH JGit 2/5] Create abstract method on ObjectDatabase for accessing the list of local pack files Shawn O. Pearce
2009-09-21 19:51       ` Michael Gaffney
2009-09-21 19:30   ` [PATCH JGit 1/5] adding tests for ObjectDirectory 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).