From c3ffacf5c3e663da15c481f67576e3f5b6d65e3d Mon Sep 17 00:00:00 2001 From: Dave Borowitz Date: Tue, 27 Jan 2015 17:54:19 -0800 Subject: [PATCH] Remove LabelId#equals() and hashCode() Somehow the equals implementation got broken in Ibb162a6a. There shouldn't have even been an opportunity for a copy-paste error, since we should have just used the implementations from StringKey. Add a tiny test to verify key behavior. Change-Id: Idf91cc7142d160f6abeb5b1365490b73728fde0e --- gerrit-reviewdb/BUCK | 2 + .../gerrit/reviewdb/client/LabelId.java | 13 ---- .../reviewdb/client/PatchSetApprovalTest.java | 63 +++++++++++++++++++ 3 files changed, 65 insertions(+), 13 deletions(-) create mode 100644 gerrit-reviewdb/src/test/java/com/google/gerrit/reviewdb/client/PatchSetApprovalTest.java diff --git a/gerrit-reviewdb/BUCK b/gerrit-reviewdb/BUCK index 9b1991b8a0..ca2c18c33d 100644 --- a/gerrit-reviewdb/BUCK +++ b/gerrit-reviewdb/BUCK @@ -29,8 +29,10 @@ java_test( srcs = glob([TESTS + 'client/**/*.java']), deps = [ ':client', + '//lib:guava', '//lib:gwtorm', '//lib:junit', + '//lib:truth', ], source_under_test = [':client'], visibility = ['//tools/eclipse:classpath'], diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/LabelId.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/LabelId.java index 7532038c11..9a159664be 100644 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/LabelId.java +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/LabelId.java @@ -41,17 +41,4 @@ public class LabelId extends StringKey> { protected void set(String newValue) { id = newValue; } - - @Override - public int hashCode() { - return get().hashCode(); - } - - @Override - public boolean equals(Object b) { - if (this == b) { - return get().equals(((LabelId) b).get()); - } - return false; - } } diff --git a/gerrit-reviewdb/src/test/java/com/google/gerrit/reviewdb/client/PatchSetApprovalTest.java b/gerrit-reviewdb/src/test/java/com/google/gerrit/reviewdb/client/PatchSetApprovalTest.java new file mode 100644 index 0000000000..eba08c80bd --- /dev/null +++ b/gerrit-reviewdb/src/test/java/com/google/gerrit/reviewdb/client/PatchSetApprovalTest.java @@ -0,0 +1,63 @@ +// Copyright (C) 2015 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.reviewdb.client; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.gwtorm.client.KeyUtil; +import com.google.gwtorm.server.StandardKeyEncoder; + +import org.junit.Test; + +import java.util.HashMap; +import java.util.Map; + +public class PatchSetApprovalTest { + static { + KeyUtil.setEncoderImpl(new StandardKeyEncoder()); + } + + @Test + public void keyEquality() { + PatchSetApproval.Key k1 = new PatchSetApproval.Key( + new PatchSet.Id(new Change.Id(1), 2), + new Account.Id(3), + new LabelId("My-Label")); + PatchSetApproval.Key k2 = new PatchSetApproval.Key( + new PatchSet.Id(new Change.Id(1), 2), + new Account.Id(3), + new LabelId("My-Label")); + PatchSetApproval.Key k3 = new PatchSetApproval.Key( + new PatchSet.Id(new Change.Id(1), 2), + new Account.Id(3), + new LabelId("Other-Label")); + + assertThat(k2).isEqualTo(k1); + assertThat(k3).isNotEqualTo(k1); + assertThat(k2.hashCode()).isEqualTo(k1.hashCode()); + assertThat(k3.hashCode()).isNotEqualTo(k1.hashCode()); + + Map map = new HashMap<>(); + map.put(k1, "k1"); + map.put(k2, "k2"); + map.put(k3, "k3"); + assertThat(map).containsKey(k1); + assertThat(map).containsKey(k2); + assertThat(map).containsKey(k3); + assertThat(map).containsEntry(k1, "k2"); + assertThat(map).containsEntry(k2, "k2"); + assertThat(map).containsEntry(k3, "k3"); + } +}