Browse Source

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>
tags/v0.9.1
Shawn O. Pearce 13 years ago
parent
commit
d0043e5d31

+ 0
- 4
org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/LocalDiskRepositoryTestCase.java View File

@@ -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);
}


+ 1
- 1
org.eclipse.jgit.junit/src/org/eclipse/jgit/junit/TestRepository.java View File

@@ -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()));
}

/**

+ 1
- 1
org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/Log.java View File

@@ -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(); ) {

+ 2
- 2
org.eclipse.jgit.test/tst/org/eclipse/jgit/revwalk/RevObjectTest.java View File

@@ -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);

+ 1
- 1
org.eclipse.jgit.test/tst/org/eclipse/jgit/storage/file/RefUpdateTest.java View File

@@ -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());

+ 3
- 3
org.eclipse.jgit.test/tst/org/eclipse/jgit/transport/ReceivePackRefFilterTest.java View File

@@ -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 {

+ 2
- 2
org.eclipse.jgit/src/org/eclipse/jgit/transport/WalkFetchConnection.java View File

@@ -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()));

Loading…
Cancel
Save