kafka-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From j...@apache.org
Subject [kafka] branch trunk updated: KAFKA-6727; Fix broken Config hashCode() and equals() (#4796)
Date Mon, 16 Apr 2018 19:32:13 GMT
This is an automated email from the ASF dual-hosted git repository.

jgus pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/kafka.git


The following commit(s) were added to refs/heads/trunk by this push:
     new 432c82d  KAFKA-6727; Fix broken Config hashCode() and equals() (#4796)
432c82d is described below

commit 432c82d3bf6bbe679df1c2ac7cc11683e8f9c611
Author: Andy Coates <8012398+big-andy-coates@users.noreply.github.com>
AuthorDate: Mon Apr 16 20:32:07 2018 +0100

    KAFKA-6727; Fix broken Config hashCode() and equals() (#4796)
    
    Reviewers: Manikumar Reddy O <manikumar.reddy@gmail.com>, Guozhang Wang <wangguoz@gmail.com>,
Ismael Juma <ismael@juma.me.uk>, Jason Gustafson <jason@confluent.io>
---
 .../org/apache/kafka/clients/admin/Config.java     | 19 ++---
 .../org/apache/kafka/clients/admin/ConfigTest.java | 91 ++++++++++++++++++++++
 2 files changed, 101 insertions(+), 9 deletions(-)

diff --git a/clients/src/main/java/org/apache/kafka/clients/admin/Config.java b/clients/src/main/java/org/apache/kafka/clients/admin/Config.java
index 56f9c27..c81c0b6 100644
--- a/clients/src/main/java/org/apache/kafka/clients/admin/Config.java
+++ b/clients/src/main/java/org/apache/kafka/clients/admin/Config.java
@@ -21,39 +21,40 @@ import org.apache.kafka.common.annotation.InterfaceStability;
 
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
 
 /**
  * A configuration object containing the configuration entries for a resource.
- *
+ * <p>
  * The API of this class is evolving, see {@link AdminClient} for details.
  */
 @InterfaceStability.Evolving
 public class Config {
 
-    private final Collection<ConfigEntry> entries;
+    private final Map<String, ConfigEntry> entries = new HashMap<>();
 
     /**
      * Create a configuration instance with the provided entries.
      */
     public Config(Collection<ConfigEntry> entries) {
-        this.entries = Collections.unmodifiableCollection(entries);
+        for (ConfigEntry entry : entries) {
+            this.entries.put(entry.name(), entry);
+        }
     }
 
     /**
      * Configuration entries for a resource.
      */
     public Collection<ConfigEntry> entries() {
-        return entries;
+        return Collections.unmodifiableCollection(entries.values());
     }
 
     /**
      * Get the configuration entry with the provided name or null if there isn't one.
      */
     public ConfigEntry get(String name) {
-        for (ConfigEntry entry : entries)
-            if (entry.name().equals(name))
-                return entry;
-        return null;
+        return entries.get(name);
     }
 
     @Override
@@ -75,6 +76,6 @@ public class Config {
 
     @Override
     public String toString() {
-        return "Config(entries=" + entries + ")";
+        return "Config(entries=" + entries.values() + ")";
     }
 }
diff --git a/clients/src/test/java/org/apache/kafka/clients/admin/ConfigTest.java b/clients/src/test/java/org/apache/kafka/clients/admin/ConfigTest.java
new file mode 100644
index 0000000..45b174b
--- /dev/null
+++ b/clients/src/test/java/org/apache/kafka/clients/admin/ConfigTest.java
@@ -0,0 +1,91 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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 org.apache.kafka.clients.admin;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import java.util.ArrayList;
+import java.util.Collection;
+
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.CoreMatchers.hasItems;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.not;
+import static org.hamcrest.CoreMatchers.nullValue;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+public class ConfigTest {
+    private static final ConfigEntry E1 = new ConfigEntry("a", "b");
+    private static final ConfigEntry E2 = new ConfigEntry("c", "d");
+    private Config config;
+
+    @Before
+    public void setUp() {
+        final Collection<ConfigEntry> entries = new ArrayList<>();
+        entries.add(E1);
+        entries.add(E2);
+
+        config = new Config(entries);
+    }
+
+    @Test
+    public void shouldGetEntry() {
+        assertThat(config.get("a"), is(E1));
+        assertThat(config.get("c"), is(E2));
+    }
+
+    @Test
+    public void shouldReturnNullOnGetUnknownEntry() {
+        assertThat(config.get("unknown"), is(nullValue()));
+    }
+
+    @Test
+    public void shouldGetAllEntries() {
+        assertThat(config.entries().size(), is(2));
+        assertThat(config.entries(), hasItems(E1, E2));
+    }
+
+    @Test
+    public void shouldImplementEqualsProperly() {
+        final Collection<ConfigEntry> entries = new ArrayList<>();
+        entries.add(E1);
+
+        assertThat(config, is(equalTo(config)));
+        assertThat(config, is(equalTo(new Config(config.entries()))));
+        assertThat(config, is(not(equalTo(new Config(entries)))));
+        assertThat(config, is(not(equalTo((Object) "this"))));
+    }
+
+    @Test
+    public void shouldImplementHashCodeProperly() {
+        final Collection<ConfigEntry> entries = new ArrayList<>();
+        entries.add(E1);
+
+        assertThat(config.hashCode(), is(config.hashCode()));
+        assertThat(config.hashCode(), is(new Config(config.entries()).hashCode()));
+        assertThat(config.hashCode(), is(not(new Config(entries).hashCode())));
+    }
+
+    @Test
+    public void shouldImplementToStringProperly() {
+        assertThat(config.toString(), containsString(E1.toString()));
+        assertThat(config.toString(), containsString(E2.toString()));
+    }
+}

-- 
To stop receiving notification emails like this one, please contact
jgus@apache.org.

Mime
View raw message