HDDS-8707. Avoid linear search in DBDefinition implementations.

[szetszwo]

What changes were proposed in this pull request?

DBDefinition declares the following method returning an array.

public DBColumnFamilyDefinition[] getColumnFamilies();

It forces the callers to perform a linear search for looking up DBColumnFamilyDefinition by names. The method should return a Map.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-8707

How was this patch tested?

By existing unit tests.

[adoroszlai]

Thanks @szetszwo for the improvement. LGTM, except an edge case in the multi-map iterator, which does not affect its current usage with the DB definitions. I can post a follow-up PR for that if you prefer.

[adoroszlai]

If the current list has no more items, and the next list is empty, then hasNext() returns true, but next() throws NoSuchElementException.

I think a safe implementation would be:

      private Iterator<T> i = emptyIterator();

      private Iterator<T> nextIterator() {
        if (i.hasNext()) {
          return i;
        }
        while (listIterator.hasNext()) {
          i = listIterator.next().iterator();
          if (i.hasNext()) {
            return i;
          }
        }
        return emptyIterator();
      }

      @Override
      public boolean hasNext() {
        return nextIterator().hasNext();
      }

      @Override
      public T next() {
        if (hasNext()) {
          return i.next();
        }
        throw new NoSuchElementException();
      }

Test:

import org.junit.jupiter.api.Test;

import java.util.ArrayList;
import java.util.List;

import static java.util.Arrays.asList;
import static java.util.Collections.emptyList;
import static org.junit.jupiter.api.Assertions.assertEquals;

class TestCollectionUtils {

  @Test
  void someEmptyLists() {
    List<List<String>> listOfLists = new ArrayList<>();
    listOfLists.add(asList("a", "b"));
    listOfLists.add(emptyList());
    listOfLists.add(emptyList());
    listOfLists.add(asList("c", "d"));
    listOfLists.add(emptyList());

    assertIteration(asList("a", "b", "c", "d"), listOfLists);
  }

  @Test
  void allEmptyLists() {
    List<List<String>> listOfLists = new ArrayList<>();
    listOfLists.add(emptyList());
    listOfLists.add(emptyList());

    assertIteration(emptyList(), listOfLists);
  }

  @Test
  void empty() {
    assertIteration(emptyList(), emptyList());
  }

  private static <T> void assertIteration(List<T> expected,
      List<List<T>> listOfLists) {
    List<T> actual = new ArrayList<>();
    CollectionUtils.newIterator(listOfLists).forEachRemaining(actual::add);
    assertEquals(expected, actual);
  }

}

[szetszwo]

@adoroszlai , good catch! You are right that it won't work if there are empty lists. Thanks for providing the code. Let me try it.

[szetszwo]

When merging this, please add

Co-authored-by: Doroszlai, Attila <adoroszlai@apache.org>

[adoroszlai]

Thanks @szetszwo for updating the patch.