From dac5fe1ea8375eaaa92be37fb2831fbc7cb2c9cc Mon Sep 17 00:00:00 2001 From: Renato Haeberli Date: Fri, 31 Jan 2025 21:59:27 +0100 Subject: [PATCH] SOLR-17623: SimpleOrderedMap implements Map (#3048) Also added NamedList.indexOf(name) convenience method. Does *not* adjust asShallowMap() yet. Coming soon. --------- Co-authored-by: David Smiley --- solr/CHANGES.txt | 4 +- .../component/PivotFacetProcessor.java | 8 +- .../apache/solr/common/util/NamedList.java | 19 +- .../solr/common/util/SimpleOrderedMap.java | 121 ++++++++++++- .../common/util/SimpleOrderedMapTest.java | 169 ++++++++++++++++++ 5 files changed, 305 insertions(+), 16 deletions(-) create mode 100644 solr/solrj/src/test/org/apache/solr/common/util/SimpleOrderedMapTest.java diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index a0b03278444..fd092411303 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -162,7 +162,7 @@ Improvements other v2 APIs. SolrJ now offers (experimental) SolrRequest implementations for all v2 configset APIs in `org.apache.solr.client.solrj.request.ConfigsetsApi`. (Jason Gerlowski) -Optimizations +Optimizations --------------------- * SOLR-17578: Remove ZkController internal core supplier, for slightly faster reconnection after Zookeeper session loss. (Pierre Salagnac) @@ -209,6 +209,8 @@ Other Changes * SOLR-17581: Introduce new test variant of waitForState(), that does not wait on live node changes when we're only interested in the collection state. (Pierre Salagnac) +* SOLR-17623: SimpleOrderedMap (a NamedList) now implements java.util.Map. (Renato Haeberli, David Smiley) + ================== 9.8.0 ================== New Features --------------------- diff --git a/solr/core/src/java/org/apache/solr/handler/component/PivotFacetProcessor.java b/solr/core/src/java/org/apache/solr/handler/component/PivotFacetProcessor.java index 77b275ea4c6..2a3ef5c69d5 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/PivotFacetProcessor.java +++ b/solr/core/src/java/org/apache/solr/handler/component/PivotFacetProcessor.java @@ -168,12 +168,14 @@ public SimpleOrderedMap>> process(String[] pivots) throws for (String refinements : refinementValuesByField) { pivotResponse.addAll( - processSingle( - pivotFields, refinements, statsFields, parsed, facetQueries, facetRanges)); + (Map>>) + processSingle( + pivotFields, refinements, statsFields, parsed, facetQueries, facetRanges)); } } else { pivotResponse.addAll( - processSingle(pivotFields, null, statsFields, parsed, facetQueries, facetRanges)); + (Map>>) + processSingle(pivotFields, null, statsFields, parsed, facetQueries, facetRanges)); } } return pivotResponse; diff --git a/solr/solrj/src/java/org/apache/solr/common/util/NamedList.java b/solr/solrj/src/java/org/apache/solr/common/util/NamedList.java index 123a5e81d91..98b4efd42bd 100644 --- a/solr/solrj/src/java/org/apache/solr/common/util/NamedList.java +++ b/solr/solrj/src/java/org/apache/solr/common/util/NamedList.java @@ -238,6 +238,15 @@ public int indexOf(String name, int start) { return -1; } + /*** + * Scans the names of the list sequentially beginning at index 0 and returns the index of the first + * pair with the specified name. + * @see #indexOf(String, int) + */ + public int indexOf(String name) { + return indexOf(name, 0); + } + /** * Gets the value for the first instance of the specified name found. * @@ -403,8 +412,12 @@ public Map asShallowMap() { return asShallowMap(false); } + /** + * @deprecated use {@link SimpleOrderedMap} instead of NamedList when a Map is required. + */ + @Deprecated public Map asShallowMap(boolean allowDps) { - return new Map() { + return new Map<>() { @Override public int size() { return NamedList.this.size(); @@ -799,7 +812,7 @@ public Collection removeConfigArgs(final String name) throws SolrExcepti throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, err + o.getClass()); } - if (collection.size() > 0) { + if (!collection.isEmpty()) { killAll(name); } @@ -845,7 +858,7 @@ public void forEachKey(Consumer fun) { } } - public void forEach(BiConsumer action) { + public void forEach(BiConsumer action) { int sz = size(); for (int i = 0; i < sz; i++) { action.accept(getName(i), getVal(i)); diff --git a/solr/solrj/src/java/org/apache/solr/common/util/SimpleOrderedMap.java b/solr/solrj/src/java/org/apache/solr/common/util/SimpleOrderedMap.java index 20178a252e3..cdd12b97749 100644 --- a/solr/solrj/src/java/org/apache/solr/common/util/SimpleOrderedMap.java +++ b/solr/solrj/src/java/org/apache/solr/common/util/SimpleOrderedMap.java @@ -16,9 +16,14 @@ */ package org.apache.solr.common.util; +import java.util.AbstractMap; +import java.util.AbstractSet; import java.util.ArrayList; +import java.util.Collection; +import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Set; /** * SimpleOrderedMap is a {@link NamedList} where access by key is more important than @@ -30,10 +35,12 @@ * {"foo":10,"bar":20} and may choose to write a NamedList as ["foo",10,"bar",20]. An XML response * writer may choose to render both the same way. * - *

This class does not provide efficient lookup by key, its main purpose is to hold data to be - * serialized. It aims to minimize overhead and to be efficient at adding new elements. + *

This class does not provide efficient lookup by key. The lookup performance is only O(N), and + * not O(1) or O(Log N) as it is for the most common Map-implementations. Its main purpose is to + * hold data to be serialized. It aims to minimize overhead and to be efficient at adding new + * elements. */ -public class SimpleOrderedMap extends NamedList { +public class SimpleOrderedMap extends NamedList implements Map { private static final SimpleOrderedMap EMPTY = new SimpleOrderedMap<>(List.of()); @@ -64,6 +71,8 @@ public SimpleOrderedMap(Map.Entry[] nameValuePairs) { super(nameValuePairs); } + // TODO override asShallowMap in Solr 10 + @Override public SimpleOrderedMap clone() { ArrayList newList = new ArrayList<>(nvPairs.size()); @@ -71,21 +80,115 @@ public SimpleOrderedMap clone() { return new SimpleOrderedMap<>(newList); } + @Override + public boolean isEmpty() { + return nvPairs.isEmpty(); + } + + @Override + public boolean containsKey(final Object key) { + return this.indexOf((String) key) >= 0; + } + + @Override + public boolean containsValue(final Object value) { + return values().contains(value); + } + /** - * Returns a shared, empty, and immutable instance of SimpleOrderedMap. + * {@inheritDoc} + * + *

Has linear lookup time O(N) * - * @return Empty SimpleOrderedMap (immutable) + * @see NamedList#get(String) */ - public static SimpleOrderedMap of() { - return EMPTY; + @Override + public T get(final Object key) { + return super.get((String) key); + } + + @Override + public T put(String key, T value) { + int idx = indexOf(key); + if (idx == -1) { + add(key, value); + return null; + } else { + T t = get(key); + setVal(idx, value); + return t; + } } /** - * Returns an immutable instance of SimpleOrderedMap with a single key-value pair. + * @see NamedList#remove(String) + */ + @Override + public T remove(final Object key) { + return super.remove((String) key); + } + + @Override + public void putAll(final Map m) { + if (isEmpty()) { + m.forEach(this::add); + } else { + m.forEach(this::put); + } + } + + @Override + public Set keySet() { + return new InnerMap().keySet(); + } + + @Override + public Collection values() { + return new InnerMap().values(); + } + + @Override + public Set> entrySet() { + + return new AbstractSet<>() { + @Override + public Iterator> iterator() { + return SimpleOrderedMap.this.iterator(); + } + + @Override + public int size() { + return SimpleOrderedMap.this.size(); + } + }; + } + + /** + * Returns an immutable instance of {@link SimpleOrderedMap} with a single key-value pair. * - * @return SimpleOrderedMap containing one key-value pair + * @return {@link SimpleOrderedMap} containing one key-value pair */ public static SimpleOrderedMap of(String name, T val) { return new SimpleOrderedMap<>(List.of(name, val)); } + + /** + * Returns a shared, empty, and immutable instance of {@link SimpleOrderedMap}. + * + * @return Empty {@link SimpleOrderedMap} (immutable) + */ + public static SimpleOrderedMap of() { + return EMPTY; + } + + /** + * {@link SimpleOrderedMap} extending {@link NamedList}, we are not able to extend {@link + * AbstractMap}. With the help of InnerMap we can still use {@link AbstractMap} methods. + */ + private class InnerMap extends AbstractMap { + @Override + public Set> entrySet() { + return SimpleOrderedMap.this.entrySet(); + } + } } diff --git a/solr/solrj/src/test/org/apache/solr/common/util/SimpleOrderedMapTest.java b/solr/solrj/src/test/org/apache/solr/common/util/SimpleOrderedMapTest.java new file mode 100644 index 00000000000..78038083b11 --- /dev/null +++ b/solr/solrj/src/test/org/apache/solr/common/util/SimpleOrderedMapTest.java @@ -0,0 +1,169 @@ +/* + * 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.solr.common.util; + +import java.util.Collection; +import java.util.Map; +import java.util.Set; +import org.apache.solr.SolrTestCase; +import org.junit.Test; + +public class SimpleOrderedMapTest extends SolrTestCase { + + private final SimpleOrderedMap map = new SimpleOrderedMap<>(); + + @Test + public void testPut() { + map.put("one", 1); + map.put("two", 2); + + assertEquals(4, map.nvPairs.size()); + assertEquals("one", map.nvPairs.get(0)); + assertEquals(1, map.nvPairs.get(1)); + assertEquals("two", map.nvPairs.get(2)); + assertEquals(2, map.nvPairs.get(3)); + } + + public void testPutReturnOldValue() { + map.put("one", 1); + int oldValue = map.put("one", 2); + + assertEquals(oldValue, oldValue); + } + + public void testPutReturnNullForNullValue() { + Integer oldValue = map.put("one", null); + + assertNull(oldValue); + } + + public void testPutReplaceExistingValue() { + map.put("one", 1); + map.put("one", 11); + + assertEquals(2, map.nvPairs.size()); + assertEquals("one", map.nvPairs.get(0)); + assertEquals(11, map.nvPairs.get(1)); + } + + @Test + public void testContains() { + setupData(); + + assertTrue(map.containsKey("one")); + assertTrue(map.containsKey("two")); + assertTrue(map.containsKey("three")); + assertFalse(map.containsKey("four")); + } + + @Test + public void testContainsNullAsKey() { + map.put(null, 1); + assertTrue(map.containsKey(null)); + } + + @Test + public void testContainsNullAsKeyValuePair() { + map.put(null, null); + assertTrue(map.containsKey(null)); + assertTrue(map.containsValue(null)); + } + + /*** + * if the map contains a entry with null as value, contains(null) should be true as it is with other maps e.g. HashMap + */ + @Test + public void testContainsValueWithNull() { + setupData(); + map.add("four", null); + assertTrue(map.containsValue(null)); + } + + @Test + public void testContainsValue() { + setupData(); + assertTrue(map.containsValue(1)); + assertTrue(map.containsValue(2)); + assertTrue(map.containsValue(3)); + assertFalse(map.containsValue(9)); + } + + @Test + public void testPutAll() { + setupData(); + map.putAll(Map.of("four", 4, "five", 5, " six", 6)); + assertEquals(12, map.nvPairs.size()); + + assertEquals("one", map.nvPairs.get(0)); + assertEquals(3, map.nvPairs.get(5)); + + // since putAll takes a Map (unordered), we do not know the order of the elements + assertTrue(map.nvPairs.contains("one")); + assertTrue(map.nvPairs.contains(1)); + assertTrue(map.nvPairs.contains("three")); + assertTrue(map.nvPairs.contains(3)); + assertTrue(map.nvPairs.contains(1)); + } + + @Test + public void testKeySet() { + setupData(); + Set keys = map.keySet(); + assertEquals(3, keys.size()); + assertTrue(keys.contains("one")); + assertTrue(keys.contains("three")); + assertFalse(keys.contains("four")); + } + + @Test + public void testValues() { + setupData(); + + Collection values = map.values(); + assertEquals(3, values.size()); + assertTrue(values.contains(1)); + assertTrue(values.contains(3)); + assertFalse(values.contains(4)); + } + + @Test + public void entrySet() { + setupData(); + + assertEquals(3, map.entrySet().size()); + assertTrue(map.nvPairs.contains("one")); + assertTrue(map.nvPairs.contains(1)); + assertTrue(map.nvPairs.contains("three")); + assertTrue(map.nvPairs.contains(3)); + } + + @Test + public void remove() { + setupData(); + Integer two = map.remove("two"); + + assertEquals(Integer.valueOf(2), two); + assertEquals(4, map.nvPairs.size()); + assertFalse(map.containsKey("two")); + } + + private void setupData() { + map.add("one", 1); + map.add("two", 2); + map.add("three", 3); + } +}