threads / patch / 20959
Subject: [PATCH JGit 1/5] adding tests for ObjectDirectory
mr.gaffo@gmail.com[PATCH JGit] Adding update-server-info functionality try2
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.
mr.gaffo@gmail.comFrom: 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
org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectDirectoryTest.java, org.spearce.jgit.test/tst/org/spearce/jgit/lib/RepositoryTestCase.java, org.spearce.jgit.test/tst/org/spearce/jgit/util/JGitTestUtil.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
mr.gaffo@gmail.com[PATCH JGit 2/5] Create abstract method on ObjectDatabase for accessing the list of local pack files.
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(-)
org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectDirectoryTest.java, org.spearce.jgit.test/tst/org/spearce/jgit/util/JGitTestUtil.java, org.spearce.jgit/src/org/spearce/jgit/lib/AlternateRepositoryDatabase.java, org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDatabase.java, org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.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 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
mr.gaffo@gmail.com[PATCH JGit 3/5] Implemented directory based info cache for objects/info/packs.
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
org.spearce.jgit.test/tst/org/spearce/jgit/lib/CachedPacksInfoFileContentsGeneratorTest.java, org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectDirectoryTest.java, org.spearce.jgit.test/tst/org/spearce/jgit/util/JGitTestUtil.java, org.spearce.jgit/src/org/spearce/jgit/lib/AlternateRepositoryDatabase.java, org.spearce.jgit/src/org/spearce/jgit/lib/CachedPacksInfoFileContentsGenerator.java, org.spearce.jgit/src/org/spearce/jgit/lib/Constants.java, org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDatabase.java, org.spearce.jgit/src/org/spearce/jgit/lib/ObjectDirectory.java, org.spearce.jgit/src/org/spearce/jgit/lib/UpdateDirectoryBasedPacksInfoCache.java, org.spearce.jgit/src/org/spearce/jgit/lib/UpdateDirectoryInfoCache.java, org.spearce.jgit/src/org/spearce/jgit/transport/ReceivePack.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
mr.gaffo@gmail.com[PATCH JGit 4/5] Adding in a InfoDatabase like ObjectDatabase and and implementation based upon a directory.
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
org.spearce.jgit.test/tst/org/spearce/jgit/lib/InfoDirectoryDatabaseTest.java, org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectDirectoryTest.java, org.spearce.jgit/src/org/spearce/jgit/lib/InfoDatabase.java, org.spearce.jgit/src/org/spearce/jgit/lib/InfoDirectoryDatabase.java, org.spearce.jgit/src/org/spearce/jgit/lib/Repository.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
mr.gaffo@gmail.com[PATCH JGit 5/5] added tests for the file based info cache update and made pass
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(-)
org.spearce.jgit.test/tst/org/spearce/jgit/lib/CachedPacksInfoFileContentsGeneratorTest.java, org.spearce.jgit.test/tst/org/spearce/jgit/lib/InfoDirectoryDatabaseTest.java, org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectDirectoryTest.java, org.spearce.jgit/src/org/spearce/jgit/lib/InfoDatabase.java, org.spearce.jgit/src/org/spearce/jgit/lib/InfoDirectoryDatabase.java, org.spearce.jgit/src/org/spearce/jgit/transport/ReceivePack.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 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
Shawn O. PearceRe: [PATCH JGit 5/5] added tests for the file based info cache update and made pass
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.
Shawn O. PearceRe: [PATCH JGit 4/5] Adding in a InfoDatabase like ObjectDatabase and and implementation based upon a directory.
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.
Shawn O. PearceRe: [PATCH JGit 3/5] Implemented directory based info cache for objects/info/packs.
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 mikeWe 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/refsWhy 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.
Shawn O. PearceRe: [PATCH JGit 2/5] Create abstract method on ObjectDatabase for accessing the list of local pack files.
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.
Michael GaffneyRe: [PATCH JGit 2/5] Create abstract method on ObjectDatabase for accessing the list of local pack files.
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
Shawn O. PearceRe: [PATCH JGit 1/5] adding tests for ObjectDirectory
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.