Remove unnecessary ObjectId.copy() calls

When RevObject overrode equals() to provide only reference equality
we used to need to convert a RevObject into an ObjectId by copy()
just to use standard Java tools like JUnit assertEquals(), or to
use contains() or get() on standard java.util collection types.

Now that we have removed this override and made ObjectId's equals()
final (preventing any of this mess in the future), some copy()
calls are unnecessary.  Anytime the value is being used as an input
to a lookup routine, or to an equals, we can avoid the copy().

However we still want to use copy() anytime we are given an ObjectId
that may exist long-term, where we don't want the high cost of the
additional storage from a RevCommit extension.  So we can't remove
all uses of copy(), just some of them.

Change-Id: Ief275dace435c0ddfa362ac8e5d93558bc7e9fc3
Signed-off-by: Shawn O. Pearce <spearce@spearce.org>

diff --git a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/LocalDiskRepositoryTestCase.java b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/LocalDiskRepositoryTestCase.java
index 47956e5127486afad8d187b4309b3aab9b1ccb96..d28c7edb506f5c2440aae0486dd3d23ee2ff02e8 100644 (file)
--- a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/LocalDiskRepositoryTestCase.java
+++ b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/LocalDiskRepositoryTestCase.java
@@ -409,10 +409,6 @@ public abstract class LocalDiskRepositoryTestCase extends TestCase {
        }
 
        protected static void assertEquals(AnyObjectId exp, AnyObjectId act) {
-               if (exp != null)
-                       exp = exp.copy();
-               if (act != null)
-                       act = act.copy();
                Assert.assertEquals(exp, act);
        }
 

diff --git a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java
index afe1c0bc63e1aa7e1e578e7e4eb0c7fa1b6cbf34..858fe64a6d768e6d6bf466a8e0febb0288ba2a24 100644 (file)
--- a/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java
+++ b/org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java
@@ -586,7 +586,7 @@ public class TestRepository<R extends Repository> {
                md.update(Constants.encodeASCII(bin.length));
                md.update((byte) 0);
                md.update(bin);
-               Assert.assertEquals(id.copy(), ObjectId.fromRaw(md.digest()));
+               Assert.assertEquals(id, ObjectId.fromRaw(md.digest()));
        }
 
        /**

diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Log.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Log.java
index fbc019fa246cd7c75a9bd84060d353dba0ed94f5..d0ae22a2164d0b3b847a20473b9a579772afc757 100644 (file)
--- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Log.java
+++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Log.java
@@ -161,7 +161,7 @@ class Log extends RevWalkTextBuiltin {
                out.print(" ");
                c.getId().copyTo(outbuffer, out);
                if (decorate) {
-                       Collection<Ref> list = allRefsByPeeledObjectId.get(c.copy());
+                       Collection<Ref> list = allRefsByPeeledObjectId.get(c);
                        if (list != null) {
                                out.print(" (");
                                for (Iterator<Ref> i = list.iterator(); i.hasNext(); ) {

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevObjectTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevObjectTest.java
index 2b24aa49adadf754ea3bd973ec1926c302e5f08b..052d390033f351a913ef4aa0c46f534016120bba 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevObjectTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevObjectTest.java
@@ -60,8 +60,8 @@ public class RevObjectTest extends RevWalkTestCase {
                assertTrue(a1.equals((Object) a1));
                assertFalse(a1.equals(b1));
 
-               assertTrue(a1.equals(a1.copy()));
-               assertTrue(a1.equals((Object) a1.copy()));
+               assertTrue(a1.equals(a1));
+               assertTrue(a1.equals((Object) a1));
                assertFalse(a1.equals(""));
 
                final RevWalk rw2 = new RevWalk(db);

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/RefUpdateTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/RefUpdateTest.java
index 871700898feba01eeb82129bc99e5503391be8d2..a8a6d9d8234706f142e7f18524511183762ca26f 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/RefUpdateTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/RefUpdateTest.java
@@ -115,7 +115,7 @@ public class RefUpdateTest extends SampleDataRepositoryTestCase {
                assertNotNull(r.getObjectId());
                assertNotSame(newid, r.getObjectId());
                assertSame(ObjectId.class, r.getObjectId().getClass());
-               assertEquals(newid.copy(), r.getObjectId());
+               assertEquals(newid, r.getObjectId());
                List<org.eclipse.jgit.storage.file.ReflogReader.Entry> reverseEntries1 = db.getReflogReader("refs/heads/abc").getReverseEntries();
                org.eclipse.jgit.storage.file.ReflogReader.Entry entry1 = reverseEntries1.get(0);
                assertEquals(1, reverseEntries1.size());

diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackRefFilterTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackRefFilterTest.java
index b331f9cf54a5201b5a367e9c9e41e0456930a369..f89959c68c2f4c83e4ed99256893d750aae3c1e7 100644 (file)
--- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackRefFilterTest.java
+++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackRefFilterTest.java
@@ -102,7 +102,7 @@ public class ReceivePackRefFilterTest extends LocalDiskRepositoryTestCase {
                Transport t = Transport.open(src, uriOf(dst));
                try {
                        t.fetch(PM, Collections.singleton(new RefSpec("+refs/*:refs/*")));
-                       assertEquals(B.copy(), src.resolve(R_MASTER));
+                       assertEquals(B, src.resolve(R_MASTER));
                } finally {
                        t.close();
                }
@@ -154,7 +154,7 @@ public class ReceivePackRefFilterTest extends LocalDiskRepositoryTestCase {
 
                Ref master = refs.get(R_MASTER);
                assertNotNull("has master", master);
-               assertEquals(B.copy(), master.getObjectId());
+               assertEquals(B, master.getObjectId());
        }
 
        public void testSuccess() throws Exception {
@@ -219,7 +219,7 @@ public class ReceivePackRefFilterTest extends LocalDiskRepositoryTestCase {
                assertNotNull("have result", r);
                assertNull("private not advertised", r.getAdvertisedRef(R_PRIVATE));
                assertSame("master updated", RemoteRefUpdate.Status.OK, u.getStatus());
-               assertEquals(N.copy(), dst.resolve(R_MASTER));
+               assertEquals(N, dst.resolve(R_MASTER));
        }
 
        public void testCreateBranchAtHiddenCommitFails() throws Exception {

diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/transport/WalkFetchConnection.java b/org.eclipse.jgit/src/org/eclipse/jgit/transport/WalkFetchConnection.java
index 237b4314c40c388767ef29f03c860f6bf4785412..a669ac5cec7b553c596a84ddcbc00a91de8a47ab 100644 (file)
--- a/org.eclipse.jgit/src/org/eclipse/jgit/transport/WalkFetchConnection.java
+++ b/org.eclipse.jgit/src/org/eclipse/jgit/transport/WalkFetchConnection.java
@@ -317,7 +317,7 @@ class WalkFetchConnection extends BaseFetchConnection {
                // If we had any prior errors fetching this object they are
                // now resolved, as the object was parsed successfully.
                //
-               fetchErrors.remove(id.copy());
+               fetchErrors.remove(id);
        }
 
        private void processBlob(final RevObject obj) throws TransportException {
@@ -461,7 +461,7 @@ class WalkFetchConnection extends BaseFetchConnection {
 
                        // We could not obtain the object. There may be reasons why.
                        //
-                       List<Throwable> failures = fetchErrors.get(id.copy());
+                       List<Throwable> failures = fetchErrors.get(id);
                        final TransportException te;
 
                        te = new TransportException(MessageFormat.format(JGitText.get().cannotGet, id.name()));