From 83252baa584e4660ea938fcb12e336063b7c0202 Mon Sep 17 00:00:00 2001 From: Zhangmei Li Date: Fri, 27 Dec 2019 01:02:36 +0800 Subject: [PATCH 1/6] fix iterators may not be closed Change-Id: I3709d96fd2114fa782d6f28f8853b32d65fbd22b --- .../com/baidu/hugegraph/iterator/CIter.java | 25 ++++ .../iterator/ExtendableIterator.java | 11 +- .../iterator/FlatMapperFilterIterator.java | 2 +- .../iterator/FlatMapperIterator.java | 16 +- .../hugegraph/iterator/ToListIterator.java | 58 ++++++++ .../hugegraph/iterator/WrappedIterator.java | 13 +- .../baidu/hugegraph/unit/UnitTestSuite.java | 2 + .../unit/iterator/ToListIteratorTest.java | 137 ++++++++++++++++++ 8 files changed, 250 insertions(+), 14 deletions(-) create mode 100644 src/main/java/com/baidu/hugegraph/iterator/CIter.java create mode 100644 src/main/java/com/baidu/hugegraph/iterator/ToListIterator.java create mode 100644 src/test/java/com/baidu/hugegraph/unit/iterator/ToListIteratorTest.java diff --git a/src/main/java/com/baidu/hugegraph/iterator/CIter.java b/src/main/java/com/baidu/hugegraph/iterator/CIter.java new file mode 100644 index 000000000..35eda4527 --- /dev/null +++ b/src/main/java/com/baidu/hugegraph/iterator/CIter.java @@ -0,0 +1,25 @@ +/* + * Copyright 2017 HugeGraph Authors + * + * 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 com.baidu.hugegraph.iterator; + +import java.util.Iterator; + +public interface CIter extends Iterator, AutoCloseable, Metadatable { +} diff --git a/src/main/java/com/baidu/hugegraph/iterator/ExtendableIterator.java b/src/main/java/com/baidu/hugegraph/iterator/ExtendableIterator.java index 7d036111a..c3df9ea23 100644 --- a/src/main/java/com/baidu/hugegraph/iterator/ExtendableIterator.java +++ b/src/main/java/com/baidu/hugegraph/iterator/ExtendableIterator.java @@ -19,10 +19,8 @@ package com.baidu.hugegraph.iterator; -import java.util.ArrayList; import java.util.Deque; import java.util.Iterator; -import java.util.List; import java.util.concurrent.ConcurrentLinkedDeque; import com.baidu.hugegraph.util.E; @@ -30,13 +28,11 @@ public class ExtendableIterator extends WrappedIterator { private final Deque> itors; - private final List> removedItors; private Iterator currentIterator; public ExtendableIterator() { this.itors = new ConcurrentLinkedDeque<>(); - this.removedItors = new ArrayList<>(); this.currentIterator = null; } @@ -62,11 +58,6 @@ public ExtendableIterator extend(Iterator iter) { @Override public void close() throws Exception { - for (Iterator iter : this.removedItors) { - if (iter instanceof AutoCloseable) { - ((AutoCloseable) iter).close(); - } - } for (Iterator iter : this.itors) { if (iter instanceof AutoCloseable) { ((AutoCloseable) iter).close(); @@ -98,7 +89,7 @@ protected boolean fetch() { // The last one return false; } - this.removedItors.add(this.itors.removeFirst()); + close(this.itors.removeFirst()); } assert first != null && first.hasNext(); diff --git a/src/main/java/com/baidu/hugegraph/iterator/FlatMapperFilterIterator.java b/src/main/java/com/baidu/hugegraph/iterator/FlatMapperFilterIterator.java index 6046dd25a..8eb7a5325 100644 --- a/src/main/java/com/baidu/hugegraph/iterator/FlatMapperFilterIterator.java +++ b/src/main/java/com/baidu/hugegraph/iterator/FlatMapperFilterIterator.java @@ -46,7 +46,7 @@ protected final boolean fetchMapped() { return true; } } - this.results = null; + this.resetResults(); return false; } } diff --git a/src/main/java/com/baidu/hugegraph/iterator/FlatMapperIterator.java b/src/main/java/com/baidu/hugegraph/iterator/FlatMapperIterator.java index 9d50d79c5..630ebef5a 100644 --- a/src/main/java/com/baidu/hugegraph/iterator/FlatMapperIterator.java +++ b/src/main/java/com/baidu/hugegraph/iterator/FlatMapperIterator.java @@ -38,6 +38,12 @@ public FlatMapperIterator(Iterator origin, this.results = null; } + @Override + public void close() throws Exception { + this.resetResults(); + super.close(); + } + @Override protected Iterator originIterator() { return this.originIterator; @@ -70,7 +76,15 @@ protected boolean fetchMapped() { return true; } } - this.results = null; + this.resetResults(); return false; } + + protected final void resetResults() { + if (this.results == null) { + return; + } + close(this.results); + this.results = null; + } } diff --git a/src/main/java/com/baidu/hugegraph/iterator/ToListIterator.java b/src/main/java/com/baidu/hugegraph/iterator/ToListIterator.java new file mode 100644 index 000000000..6ac1d5928 --- /dev/null +++ b/src/main/java/com/baidu/hugegraph/iterator/ToListIterator.java @@ -0,0 +1,58 @@ +/* + * Copyright 2017 HugeGraph Authors + * + * 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 com.baidu.hugegraph.iterator; + +import java.util.Iterator; +import java.util.List; + +import org.apache.commons.collections.IteratorUtils; + +public class ToListIterator extends WrappedIterator { + + private final Iterator origin; + private final Iterator iterator; + + public ToListIterator(Iterator origin) { + this.origin = origin; + @SuppressWarnings("unchecked") + List results = IteratorUtils.toList(origin); + this.iterator = results.iterator(); + } + + @Override + public void remove() { + this.iterator.remove(); + } + + @Override + protected boolean fetch() { + assert this.current == none(); + if (!this.iterator.hasNext()) { + return false; + } + this.current = this.iterator.next(); + return true; + } + + @Override + protected Iterator originIterator() { + return this.origin; + } +} diff --git a/src/main/java/com/baidu/hugegraph/iterator/WrappedIterator.java b/src/main/java/com/baidu/hugegraph/iterator/WrappedIterator.java index 30cf0725e..c6e9ae7bc 100644 --- a/src/main/java/com/baidu/hugegraph/iterator/WrappedIterator.java +++ b/src/main/java/com/baidu/hugegraph/iterator/WrappedIterator.java @@ -22,8 +22,7 @@ import java.util.Iterator; import java.util.NoSuchElementException; -public abstract class WrappedIterator - implements Iterator, AutoCloseable, Metadatable { +public abstract class WrappedIterator implements CIter { private static final Object NONE = new Object(); @@ -86,6 +85,16 @@ protected static final R none() { return (R) NONE; } + public static void close(Iterator iterator) { + if (iterator instanceof AutoCloseable) { + try { + ((AutoCloseable) iterator).close(); + } catch (Exception e) { + throw new IllegalStateException("Failed to close iterator"); + } + } + } + protected abstract Iterator originIterator(); protected abstract boolean fetch(); diff --git a/src/test/java/com/baidu/hugegraph/unit/UnitTestSuite.java b/src/test/java/com/baidu/hugegraph/unit/UnitTestSuite.java index 4d292d626..48b1536ba 100644 --- a/src/test/java/com/baidu/hugegraph/unit/UnitTestSuite.java +++ b/src/test/java/com/baidu/hugegraph/unit/UnitTestSuite.java @@ -35,6 +35,7 @@ import com.baidu.hugegraph.unit.iterator.FlatMapperFilterIteratorTest; import com.baidu.hugegraph.unit.iterator.FlatMapperIteratorTest; import com.baidu.hugegraph.unit.iterator.MapperIteratorTest; +import com.baidu.hugegraph.unit.iterator.ToListIteratorTest; import com.baidu.hugegraph.unit.license.ExtraParamTest; import com.baidu.hugegraph.unit.license.LicenseCreateParamTest; import com.baidu.hugegraph.unit.license.LicenseManagerTest; @@ -74,6 +75,7 @@ MapperIteratorTest.class, FlatMapperIteratorTest.class, FlatMapperFilterIteratorTest.class, + ToListIteratorTest.class, BytesTest.class, CollectionUtilTest.class, diff --git a/src/test/java/com/baidu/hugegraph/unit/iterator/ToListIteratorTest.java b/src/test/java/com/baidu/hugegraph/unit/iterator/ToListIteratorTest.java new file mode 100644 index 000000000..7c8d18fae --- /dev/null +++ b/src/test/java/com/baidu/hugegraph/unit/iterator/ToListIteratorTest.java @@ -0,0 +1,137 @@ +/* + * Copyright 2017 HugeGraph Authors + * + * 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 com.baidu.hugegraph.unit.iterator; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.Iterator; +import java.util.List; +import java.util.NoSuchElementException; + +import org.junit.Test; + +import com.baidu.hugegraph.iterator.ToListIterator; +import com.baidu.hugegraph.testutil.Assert; +import com.baidu.hugegraph.unit.BaseUnitTest; +import com.baidu.hugegraph.unit.iterator.ExtendableIteratorTest.CloseableItor; +import com.google.common.collect.ImmutableList; + +@SuppressWarnings("resource") +public class ToListIteratorTest extends BaseUnitTest { + + private static final List DATA1 = ImmutableList.of(1); + private static final List DATA2 = ImmutableList.of(2, 3); + private static final List DATA3 = ImmutableList.of(4, 5, 6); + + @Test + public void testHasNext() { + Iterator origin = DATA1.iterator(); + Assert.assertTrue(origin.hasNext()); + + Iterator results = new ToListIterator<>(origin); + Assert.assertTrue(results.hasNext()); + Assert.assertTrue(results.hasNext()); + Assert.assertFalse(origin.hasNext()); + } + + @Test + public void testNext() { + Iterator results = new ToListIterator<>(DATA1.iterator()); + Assert.assertEquals(1, (int) results.next()); + Assert.assertThrows(NoSuchElementException.class, () -> { + results.next(); + }); + } + + @Test + public void testNextWithMultiTimes() { + Iterator results = new ToListIterator<>(DATA2.iterator()); + Assert.assertEquals(2, (int) results.next()); + Assert.assertEquals(3, (int) results.next()); + Assert.assertThrows(NoSuchElementException.class, () -> { + results.next(); + }); + } + + @Test + public void testHasNextAndNext() { + Iterator results = new ToListIterator<>(DATA1.iterator()); + Assert.assertTrue(results.hasNext()); + Assert.assertTrue(results.hasNext()); + Assert.assertEquals(1, (int) results.next()); + Assert.assertFalse(results.hasNext()); + Assert.assertFalse(results.hasNext()); + Assert.assertThrows(NoSuchElementException.class, () -> { + results.next(); + }); + } + + @Test + public void testRemove() { + List list3 = new ArrayList<>(DATA3); + Iterator results = new ToListIterator<>(list3.iterator()); + + Assert.assertEquals(ImmutableList.of(4, 5, 6), list3); + + Assert.assertEquals(4, (int) results.next()); + Assert.assertEquals(5, (int) results.next()); + results.remove(); + Assert.assertEquals(6, (int) results.next()); + + Assert.assertEquals(ImmutableList.of(4, 5, 6), list3); + } + + @Test + public void testRemoveWithoutResult() { + Iterator empty = Collections.emptyIterator(); + Iterator results = new ToListIterator<>(empty); + Assert.assertThrows(IllegalStateException.class, () -> { + results.remove(); + }); + + List list0 = new ArrayList<>(); + Iterator results2 = new ToListIterator<>(list0.iterator()); + Assert.assertThrows(IllegalStateException.class, () -> { + results2.remove(); + }); + + List list1 = new ArrayList<>(DATA1); + Iterator results3 = new ToListIterator<>(list1.iterator()); + results3.next(); + Assert.assertThrows(NoSuchElementException.class, () -> { + results3.next(); + }); + results3.remove(); // OK + Assert.assertEquals(ImmutableList.of(1), list1); + } + + @Test + public void testClose() throws Exception { + CloseableItor c1 = new CloseableItor<>(DATA1.iterator()); + + ToListIterator results = new ToListIterator<>(c1); + + Assert.assertFalse(c1.closed()); + + results.close(); + + Assert.assertTrue(c1.closed()); + } +} From 6246876035dffb6222944eb59efc2e669ff04f8a Mon Sep 17 00:00:00 2001 From: Zhangmei Li Date: Sat, 28 Dec 2019 20:59:41 +0800 Subject: [PATCH 2/6] add BatchMapperIterator and ListIterator Change-Id: If59870fb4f281f40af5bfbe3858c443108d4180e --- .../iterator/BatchMapperIterator.java | 103 +++++++ .../iterator/ExtendableIterator.java | 2 +- .../iterator/FlatMapperFilterIterator.java | 8 +- .../iterator/FlatMapperIterator.java | 29 +- ...{ToListIterator.java => ListIterator.java} | 40 ++- .../hugegraph/iterator/MapperIterator.java | 2 +- .../baidu/hugegraph/unit/UnitTestSuite.java | 6 +- .../iterator/BatchMapperIteratorTest.java | 260 ++++++++++++++++++ ...teratorTest.java => ListIteratorTest.java} | 79 +++++- 9 files changed, 481 insertions(+), 48 deletions(-) create mode 100644 src/main/java/com/baidu/hugegraph/iterator/BatchMapperIterator.java rename src/main/java/com/baidu/hugegraph/iterator/{ToListIterator.java => ListIterator.java} (51%) create mode 100644 src/test/java/com/baidu/hugegraph/unit/iterator/BatchMapperIteratorTest.java rename src/test/java/com/baidu/hugegraph/unit/iterator/{ToListIteratorTest.java => ListIteratorTest.java} (59%) diff --git a/src/main/java/com/baidu/hugegraph/iterator/BatchMapperIterator.java b/src/main/java/com/baidu/hugegraph/iterator/BatchMapperIterator.java new file mode 100644 index 000000000..343c7b5f1 --- /dev/null +++ b/src/main/java/com/baidu/hugegraph/iterator/BatchMapperIterator.java @@ -0,0 +1,103 @@ +/* + * Copyright 2017 HugeGraph Authors + * + * 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 com.baidu.hugegraph.iterator; + +import java.util.Iterator; +import java.util.List; +import java.util.function.Function; + +import com.baidu.hugegraph.util.E; +import com.baidu.hugegraph.util.InsertionOrderUtil; +import com.google.common.collect.ImmutableList; + +public class BatchMapperIterator extends WrappedIterator { + + private final int batch; + private final Iterator originIterator; + private final Function, Iterator> mapperCallback; + + private Iterator batchIterator; + + public BatchMapperIterator(int batch, Iterator origin, + Function, Iterator> mapper) { + E.checkArgument(batch > 0, "Expect batch > 0, but got %s", batch); + this.batch = batch; + this.originIterator = origin; + this.mapperCallback = mapper; + this.batchIterator = null; + } + + @Override + protected Iterator originIterator() { + return this.originIterator; + } + + @Override + protected final boolean fetch() { + if (this.batchIterator != null && this.fetchFromBatch()) { + return true; + } + + List list = this.fetchBatch(); + if (!list.isEmpty()) { + assert this.batchIterator == null; + // Do fetch + this.batchIterator = this.mapperCallback.apply(list); + if (this.batchIterator != null && this.fetchFromBatch()) { + return true; + } + } + return false; + } + + protected List fetchBatch() { + if (!this.originIterator.hasNext()) { + return ImmutableList.of(); + } + List list = InsertionOrderUtil.newList(); + for (int i = 0; i < this.batch && this.originIterator.hasNext(); i++) { + T next = this.originIterator.next(); + list.add(next); + } + return list; + } + + protected boolean fetchFromBatch() { + E.checkNotNull(this.batchIterator, "mapper results"); + while (this.batchIterator.hasNext()) { + R result = this.batchIterator.next(); + if (result != null) { + assert this.current == none(); + this.current = result; + return true; + } + } + this.resetResults(); + return false; + } + + protected final void resetResults() { + if (this.batchIterator == null) { + return; + } + close(this.batchIterator); + this.batchIterator = null; + } +} diff --git a/src/main/java/com/baidu/hugegraph/iterator/ExtendableIterator.java b/src/main/java/com/baidu/hugegraph/iterator/ExtendableIterator.java index c3df9ea23..d9985edc5 100644 --- a/src/main/java/com/baidu/hugegraph/iterator/ExtendableIterator.java +++ b/src/main/java/com/baidu/hugegraph/iterator/ExtendableIterator.java @@ -66,7 +66,7 @@ public void close() throws Exception { } @Override - protected Iterator originIterator() { + protected Iterator originIterator() { return this.currentIterator; } diff --git a/src/main/java/com/baidu/hugegraph/iterator/FlatMapperFilterIterator.java b/src/main/java/com/baidu/hugegraph/iterator/FlatMapperFilterIterator.java index 8eb7a5325..577c3da7f 100644 --- a/src/main/java/com/baidu/hugegraph/iterator/FlatMapperFilterIterator.java +++ b/src/main/java/com/baidu/hugegraph/iterator/FlatMapperFilterIterator.java @@ -36,10 +36,10 @@ public FlatMapperFilterIterator(Iterator origin, } @Override - protected final boolean fetchMapped() { - E.checkNotNull(this.results, "mapper results"); - while (this.results.hasNext()) { - R result = this.results.next(); + protected final boolean fetchFromMap() { + E.checkNotNull(this.batchIterator, "mapper results"); + while (this.batchIterator.hasNext()) { + R result = this.batchIterator.next(); if (result != null && this.filterCallback.apply(result)) { assert this.current == none(); this.current = result; diff --git a/src/main/java/com/baidu/hugegraph/iterator/FlatMapperIterator.java b/src/main/java/com/baidu/hugegraph/iterator/FlatMapperIterator.java index 630ebef5a..098ba8888 100644 --- a/src/main/java/com/baidu/hugegraph/iterator/FlatMapperIterator.java +++ b/src/main/java/com/baidu/hugegraph/iterator/FlatMapperIterator.java @@ -29,13 +29,13 @@ public class FlatMapperIterator extends WrappedIterator { private final Iterator originIterator; private final Function> mapperCallback; - protected Iterator results; + protected Iterator batchIterator; public FlatMapperIterator(Iterator origin, Function> mapper) { this.originIterator = origin; this.mapperCallback = mapper; - this.results = null; + this.batchIterator = null; } @Override @@ -45,31 +45,32 @@ public void close() throws Exception { } @Override - protected Iterator originIterator() { + protected Iterator originIterator() { return this.originIterator; } @Override protected final boolean fetch() { - if (this.results != null && this.fetchMapped()) { + if (this.batchIterator != null && this.fetchFromMap()) { return true; } while (this.originIterator.hasNext()) { T next = this.originIterator.next(); - assert this.results == null; - this.results = this.mapperCallback.apply(next); - if (this.results != null && this.fetchMapped()) { + assert this.batchIterator == null; + // Do fetch + this.batchIterator = this.mapperCallback.apply(next); + if (this.batchIterator != null && this.fetchFromMap()) { return true; } } return false; } - protected boolean fetchMapped() { - E.checkNotNull(this.results, "mapper results"); - while (this.results.hasNext()) { - R result = this.results.next(); + protected boolean fetchFromMap() { + E.checkNotNull(this.batchIterator, "mapper results"); + while (this.batchIterator.hasNext()) { + R result = this.batchIterator.next(); if (result != null) { assert this.current == none(); this.current = result; @@ -81,10 +82,10 @@ protected boolean fetchMapped() { } protected final void resetResults() { - if (this.results == null) { + if (this.batchIterator == null) { return; } - close(this.results); - this.results = null; + close(this.batchIterator); + this.batchIterator = null; } } diff --git a/src/main/java/com/baidu/hugegraph/iterator/ToListIterator.java b/src/main/java/com/baidu/hugegraph/iterator/ListIterator.java similarity index 51% rename from src/main/java/com/baidu/hugegraph/iterator/ToListIterator.java rename to src/main/java/com/baidu/hugegraph/iterator/ListIterator.java index 6ac1d5928..a46f73857 100644 --- a/src/main/java/com/baidu/hugegraph/iterator/ToListIterator.java +++ b/src/main/java/com/baidu/hugegraph/iterator/ListIterator.java @@ -19,40 +19,52 @@ package com.baidu.hugegraph.iterator; +import java.util.Collections; import java.util.Iterator; import java.util.List; -import org.apache.commons.collections.IteratorUtils; +import com.baidu.hugegraph.util.InsertionOrderUtil; -public class ToListIterator extends WrappedIterator { +public class ListIterator extends WrappedIterator { - private final Iterator origin; - private final Iterator iterator; + private final Iterator originIterator; + private final Iterator resultsIterator; + private final List results; - public ToListIterator(Iterator origin) { - this.origin = origin; - @SuppressWarnings("unchecked") - List results = IteratorUtils.toList(origin); - this.iterator = results.iterator(); + public ListIterator(long capacity, Iterator origin) { + this.originIterator = origin; + this.results = InsertionOrderUtil.newList(); + while (origin.hasNext()) { + if (capacity >= 0L && this.results.size() >= capacity) { + throw new IllegalArgumentException( + "The iterator exceeded capacity " + capacity); + } + this.results.add(origin.next()); + } + this.resultsIterator = this.results.iterator(); } @Override public void remove() { - this.iterator.remove(); + this.resultsIterator.remove(); + } + + public List list() { + return Collections.unmodifiableList(this.results); } @Override protected boolean fetch() { assert this.current == none(); - if (!this.iterator.hasNext()) { + if (!this.resultsIterator.hasNext()) { return false; } - this.current = this.iterator.next(); + this.current = this.resultsIterator.next(); return true; } @Override - protected Iterator originIterator() { - return this.origin; + protected Iterator originIterator() { + return this.originIterator; } } diff --git a/src/main/java/com/baidu/hugegraph/iterator/MapperIterator.java b/src/main/java/com/baidu/hugegraph/iterator/MapperIterator.java index f9930f512..c02d17c27 100644 --- a/src/main/java/com/baidu/hugegraph/iterator/MapperIterator.java +++ b/src/main/java/com/baidu/hugegraph/iterator/MapperIterator.java @@ -33,7 +33,7 @@ public MapperIterator(Iterator origin, Function mapper) { } @Override - protected Iterator originIterator() { + protected Iterator originIterator() { return this.originIterator; } diff --git a/src/test/java/com/baidu/hugegraph/unit/UnitTestSuite.java b/src/test/java/com/baidu/hugegraph/unit/UnitTestSuite.java index 48b1536ba..dc90b35bf 100644 --- a/src/test/java/com/baidu/hugegraph/unit/UnitTestSuite.java +++ b/src/test/java/com/baidu/hugegraph/unit/UnitTestSuite.java @@ -30,12 +30,13 @@ import com.baidu.hugegraph.unit.config.OptionSpaceTest; import com.baidu.hugegraph.unit.date.SafeDateFormatTest; import com.baidu.hugegraph.unit.event.EventHubTest; +import com.baidu.hugegraph.unit.iterator.BatchMapperIteratorTest; import com.baidu.hugegraph.unit.iterator.ExtendableIteratorTest; import com.baidu.hugegraph.unit.iterator.FilterIteratorTest; import com.baidu.hugegraph.unit.iterator.FlatMapperFilterIteratorTest; import com.baidu.hugegraph.unit.iterator.FlatMapperIteratorTest; +import com.baidu.hugegraph.unit.iterator.ListIteratorTest; import com.baidu.hugegraph.unit.iterator.MapperIteratorTest; -import com.baidu.hugegraph.unit.iterator.ToListIteratorTest; import com.baidu.hugegraph.unit.license.ExtraParamTest; import com.baidu.hugegraph.unit.license.LicenseCreateParamTest; import com.baidu.hugegraph.unit.license.LicenseManagerTest; @@ -75,7 +76,8 @@ MapperIteratorTest.class, FlatMapperIteratorTest.class, FlatMapperFilterIteratorTest.class, - ToListIteratorTest.class, + ListIteratorTest.class, + BatchMapperIteratorTest.class, BytesTest.class, CollectionUtilTest.class, diff --git a/src/test/java/com/baidu/hugegraph/unit/iterator/BatchMapperIteratorTest.java b/src/test/java/com/baidu/hugegraph/unit/iterator/BatchMapperIteratorTest.java new file mode 100644 index 000000000..5aa196de9 --- /dev/null +++ b/src/test/java/com/baidu/hugegraph/unit/iterator/BatchMapperIteratorTest.java @@ -0,0 +1,260 @@ +/* + * Copyright 2017 HugeGraph Authors + * + * 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 com.baidu.hugegraph.unit.iterator; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.Iterator; +import java.util.List; +import java.util.NoSuchElementException; +import java.util.function.Function; + +import org.junit.Test; + +import com.baidu.hugegraph.iterator.BatchMapperIterator; +import com.baidu.hugegraph.testutil.Assert; +import com.baidu.hugegraph.unit.BaseUnitTest; +import com.baidu.hugegraph.unit.iterator.ExtendableIteratorTest.CloseableItor; +import com.google.common.collect.ImmutableList; + +@SuppressWarnings("resource") +public class BatchMapperIteratorTest extends BaseUnitTest { + + private static final Iterator EMPTY = Collections.emptyIterator(); + + private static final List DATA1 = ImmutableList.of(1); + private static final List DATA2 = ImmutableList.of(2, 3); + private static final List DATA3 = ImmutableList.of(4, 5, 6); + + private static final Function, Iterator> MAPPER = + batch -> batch.iterator(); + + @Test + public void testBatchMapper() { + Iterator results; + + results = new BatchMapperIterator<>(1, DATA1.iterator(), MAPPER); + Assert.assertEquals(ImmutableList.of(1), ImmutableList.copyOf(results)); + + results = new BatchMapperIterator<>(1, DATA2.iterator(), MAPPER); + Assert.assertEquals(ImmutableList.of(2, 3), + ImmutableList.copyOf(results)); + + results = new BatchMapperIterator<>(1, DATA3.iterator(), MAPPER); + Assert.assertEquals(ImmutableList.of(4, 5, 6), + ImmutableList.copyOf(results)); + } + + @Test + public void testBatch() { + Iterator results; + + results = new BatchMapperIterator<>(2, DATA2.iterator(), MAPPER); + Assert.assertEquals(ImmutableList.of(2, 3), + ImmutableList.copyOf(results)); + + results = new BatchMapperIterator<>(2, DATA3.iterator(), MAPPER); + Assert.assertEquals(ImmutableList.of(4, 5, 6), + ImmutableList.copyOf(results)); + + results = new BatchMapperIterator<>(3, DATA3.iterator(), MAPPER); + Assert.assertEquals(ImmutableList.of(4, 5, 6), + ImmutableList.copyOf(results)); + + results = new BatchMapperIterator<>(4, DATA3.iterator(), MAPPER); + Assert.assertEquals(ImmutableList.of(4, 5, 6), + ImmutableList.copyOf(results)); + } + + @Test + public void testInvalidBatch() { + Assert.assertThrows(IllegalArgumentException.class, () -> { + new BatchMapperIterator<>(0, DATA1.iterator(), MAPPER); + }); + + Assert.assertThrows(IllegalArgumentException.class, () -> { + new BatchMapperIterator<>(-1, DATA1.iterator(), MAPPER); + }); + } + + @Test + public void testHasNext() { + Iterator results; + + results = new BatchMapperIterator<>(1, EMPTY, MAPPER); + Assert.assertFalse(results.hasNext()); + + results = new BatchMapperIterator<>(1, DATA1.iterator(), MAPPER); + Assert.assertTrue(results.hasNext()); + } + + @Test + public void testHasNextAndNextWithMultiTimes() { + Iterator results; + + results = new BatchMapperIterator<>(1, DATA3.iterator(), MAPPER); + + for (int i = 0; i < 5; i++) { + Assert.assertTrue(results.hasNext()); + } + + for (int i = 0; i < 3; i++) { + results.next(); + } + + Assert.assertFalse(results.hasNext()); + Assert.assertFalse(results.hasNext()); + + Assert.assertThrows(NoSuchElementException.class, () -> { + results.next(); + }); + Assert.assertThrows(NoSuchElementException.class, () -> { + results.next(); + }); + } + + @Test + public void testNext() { + Iterator results; + + results = new BatchMapperIterator<>(1, DATA1.iterator(), MAPPER); + // Call next() without hasNext() + Assert.assertEquals(1, results.next()); + + results = new BatchMapperIterator<>(1, DATA3.iterator(), MAPPER); + Assert.assertEquals(4, results.next()); + Assert.assertEquals(5, results.next()); + Assert.assertEquals(6, results.next()); + } + + @Test + public void testNextWithMultiTimes() { + Iterator results; + + results = new BatchMapperIterator<>(1, DATA2.iterator(), MAPPER); + + for (int i = 0; i < 2; i++) { + results.next(); + } + Assert.assertThrows(NoSuchElementException.class, () -> { + results.next(); + }); + } + + @Test + public void testMapperWithBatch() { + Iterator results; + + results = new BatchMapperIterator<>(1, DATA3.iterator(), batch -> { + Assert.assertEquals(1, batch.size()); + return batch.iterator(); + }); + Assert.assertEquals(4, results.next()); + Assert.assertEquals(5, results.next()); + Assert.assertEquals(6, results.next()); + Assert.assertFalse(results.hasNext()); + + results = new BatchMapperIterator<>(2, DATA3.iterator(), batch -> { + if (batch.size() == 1) { + Assert.assertEquals(6, batch.get(0)); + } else { + Assert.assertEquals(2, batch.size()); + } + return batch.iterator(); + }); + Assert.assertEquals(4, results.next()); + Assert.assertEquals(5, results.next()); + Assert.assertEquals(6, results.next()); + Assert.assertFalse(results.hasNext()); + + results = new BatchMapperIterator<>(3, DATA3.iterator(), batch -> { + Assert.assertEquals(3, batch.size()); + return batch.iterator(); + }); + Assert.assertEquals(4, results.next()); + Assert.assertEquals(5, results.next()); + Assert.assertEquals(6, results.next()); + Assert.assertFalse(results.hasNext()); + + results = new BatchMapperIterator<>(4, DATA3.iterator(), batch -> { + Assert.assertEquals(3, batch.size()); + return batch.iterator(); + }); + Assert.assertEquals(4, results.next()); + Assert.assertEquals(5, results.next()); + Assert.assertEquals(6, results.next()); + Assert.assertFalse(results.hasNext()); + } + + @Test + public void testMapperTHenReturn2X() { + Iterator results; + + results = new BatchMapperIterator<>(1, DATA3.iterator(), batch -> { + List list = new ArrayList<>(); + for (int i : batch) { + list.add(2 * i); + } + return list.iterator(); + }); + + Assert.assertEquals(8, results.next()); + Assert.assertEquals(10, results.next()); + Assert.assertEquals(12, results.next()); + Assert.assertFalse(results.hasNext()); + } + + @Test + public void testMapperReturnNullThenHasNext() { + Iterator results; + + results = new BatchMapperIterator<>(1, DATA3.iterator(), batch -> { + return null; + }); + Assert.assertFalse(results.hasNext()); + Assert.assertFalse(results.hasNext()); + } + + @Test + public void testMapperReturnNullThenNext() { + Iterator results; + + results = new BatchMapperIterator<>(1, DATA3.iterator(), batch -> { + return null; + }); + Assert.assertThrows(NoSuchElementException.class, () -> { + results.next(); + }); + Assert.assertThrows(NoSuchElementException.class, () -> { + results.next(); + }); + } + + @Test + public void testClose() throws Exception { + CloseableItor vals = new CloseableItor<>(DATA1.iterator()); + + Iterator results = new BatchMapperIterator<>(1, vals, MAPPER); + + Assert.assertFalse(vals.closed()); + ((BatchMapperIterator) results).close(); + Assert.assertTrue(vals.closed()); + } +} diff --git a/src/test/java/com/baidu/hugegraph/unit/iterator/ToListIteratorTest.java b/src/test/java/com/baidu/hugegraph/unit/iterator/ListIteratorTest.java similarity index 59% rename from src/test/java/com/baidu/hugegraph/unit/iterator/ToListIteratorTest.java rename to src/test/java/com/baidu/hugegraph/unit/iterator/ListIteratorTest.java index 7c8d18fae..8fff02ff5 100644 --- a/src/test/java/com/baidu/hugegraph/unit/iterator/ToListIteratorTest.java +++ b/src/test/java/com/baidu/hugegraph/unit/iterator/ListIteratorTest.java @@ -27,25 +27,70 @@ import org.junit.Test; -import com.baidu.hugegraph.iterator.ToListIterator; +import com.baidu.hugegraph.iterator.ListIterator; import com.baidu.hugegraph.testutil.Assert; import com.baidu.hugegraph.unit.BaseUnitTest; import com.baidu.hugegraph.unit.iterator.ExtendableIteratorTest.CloseableItor; import com.google.common.collect.ImmutableList; +import com.google.common.collect.Iterators; @SuppressWarnings("resource") -public class ToListIteratorTest extends BaseUnitTest { +public class ListIteratorTest extends BaseUnitTest { + + private static final Iterator EMPTY = Collections.emptyIterator(); private static final List DATA1 = ImmutableList.of(1); private static final List DATA2 = ImmutableList.of(2, 3); private static final List DATA3 = ImmutableList.of(4, 5, 6); + @Test + public void testCapacity() { + Iterator results; + + results = new ListIterator<>(0, EMPTY); + Assert.assertFalse(results.hasNext()); + Assert.assertEquals(0, Iterators.size(results)); + + results = new ListIterator<>(2, DATA1.iterator()); + Assert.assertTrue(results.hasNext()); + Assert.assertEquals(1, Iterators.size(results)); + + results = new ListIterator<>(2, DATA2.iterator()); + Assert.assertTrue(results.hasNext()); + Assert.assertEquals(2, Iterators.size(results)); + + Assert.assertThrows(IllegalArgumentException.class, () -> { + new ListIterator<>(0, DATA1.iterator()); + }); + + Assert.assertThrows(IllegalArgumentException.class, () -> { + new ListIterator<>(2, DATA3.iterator()); + }); + } + + @Test + public void testList() { + ListIterator results; + + results = new ListIterator<>(-1, EMPTY); + Assert.assertEquals(ImmutableList.of(), results.list()); + + results = new ListIterator<>(-1, DATA1.iterator()); + Assert.assertEquals(ImmutableList.of(1), results.list()); + + results = new ListIterator<>(-1, DATA2.iterator()); + Assert.assertEquals(ImmutableList.of(2, 3), results.list()); + + results = new ListIterator<>(-1, DATA3.iterator()); + Assert.assertEquals(ImmutableList.of(4, 5, 6), results.list()); + } + @Test public void testHasNext() { Iterator origin = DATA1.iterator(); Assert.assertTrue(origin.hasNext()); - Iterator results = new ToListIterator<>(origin); + Iterator results = new ListIterator<>(-1, origin); Assert.assertTrue(results.hasNext()); Assert.assertTrue(results.hasNext()); Assert.assertFalse(origin.hasNext()); @@ -53,7 +98,18 @@ public void testHasNext() { @Test public void testNext() { - Iterator results = new ToListIterator<>(DATA1.iterator()); + Iterator results = new ListIterator<>(-1, DATA1.iterator()); + Assert.assertEquals(1, (int) results.next()); + Assert.assertThrows(NoSuchElementException.class, () -> { + results.next(); + }); + } + + @Test + public void testNextAfterList() { + Iterator results = new ListIterator<>(-1, DATA1.iterator()); + Assert.assertEquals(ImmutableList.of(1), + ((ListIterator) results).list()); Assert.assertEquals(1, (int) results.next()); Assert.assertThrows(NoSuchElementException.class, () -> { results.next(); @@ -62,7 +118,7 @@ public void testNext() { @Test public void testNextWithMultiTimes() { - Iterator results = new ToListIterator<>(DATA2.iterator()); + Iterator results = new ListIterator<>(-1, DATA2.iterator()); Assert.assertEquals(2, (int) results.next()); Assert.assertEquals(3, (int) results.next()); Assert.assertThrows(NoSuchElementException.class, () -> { @@ -72,7 +128,7 @@ public void testNextWithMultiTimes() { @Test public void testHasNextAndNext() { - Iterator results = new ToListIterator<>(DATA1.iterator()); + Iterator results = new ListIterator<>(-1, DATA1.iterator()); Assert.assertTrue(results.hasNext()); Assert.assertTrue(results.hasNext()); Assert.assertEquals(1, (int) results.next()); @@ -86,7 +142,7 @@ public void testHasNextAndNext() { @Test public void testRemove() { List list3 = new ArrayList<>(DATA3); - Iterator results = new ToListIterator<>(list3.iterator()); + Iterator results = new ListIterator<>(-1, list3.iterator()); Assert.assertEquals(ImmutableList.of(4, 5, 6), list3); @@ -100,20 +156,19 @@ public void testRemove() { @Test public void testRemoveWithoutResult() { - Iterator empty = Collections.emptyIterator(); - Iterator results = new ToListIterator<>(empty); + Iterator results = new ListIterator<>(-1, EMPTY); Assert.assertThrows(IllegalStateException.class, () -> { results.remove(); }); List list0 = new ArrayList<>(); - Iterator results2 = new ToListIterator<>(list0.iterator()); + Iterator results2 = new ListIterator<>(-1, list0.iterator()); Assert.assertThrows(IllegalStateException.class, () -> { results2.remove(); }); List list1 = new ArrayList<>(DATA1); - Iterator results3 = new ToListIterator<>(list1.iterator()); + Iterator results3 = new ListIterator<>(-1, list1.iterator()); results3.next(); Assert.assertThrows(NoSuchElementException.class, () -> { results3.next(); @@ -126,7 +181,7 @@ public void testRemoveWithoutResult() { public void testClose() throws Exception { CloseableItor c1 = new CloseableItor<>(DATA1.iterator()); - ToListIterator results = new ToListIterator<>(c1); + ListIterator results = new ListIterator<>(-1, c1); Assert.assertFalse(c1.closed()); From ab2d2219d1bff13a4200fbfb8122a89219abda46 Mon Sep 17 00:00:00 2001 From: Zhangmei Li Date: Sun, 29 Dec 2019 23:42:41 +0800 Subject: [PATCH 3/6] fix ExtendableIteratorTest.testCloseAfterNext() Change-Id: I1add942c011cfc1b6faf998d94901fc2d8723169 --- .../unit/iterator/ExtendableIteratorTest.java | 29 +++++++++++++++++-- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/src/test/java/com/baidu/hugegraph/unit/iterator/ExtendableIteratorTest.java b/src/test/java/com/baidu/hugegraph/unit/iterator/ExtendableIteratorTest.java index 1a9a635b5..4c7ba31b8 100644 --- a/src/test/java/com/baidu/hugegraph/unit/iterator/ExtendableIteratorTest.java +++ b/src/test/java/com/baidu/hugegraph/unit/iterator/ExtendableIteratorTest.java @@ -175,7 +175,30 @@ public void testClose() throws Exception { } @Test - public void testCloseAfterNext() throws Exception { + public void testCloseAfterNext1() throws Exception { + CloseableItor c1 = new CloseableItor<>(DATA1.iterator()); + CloseableItor c2 = new CloseableItor<>(DATA2.iterator()); + CloseableItor c3 = new CloseableItor<>(DATA3.iterator()); + + ExtendableIterator results = new ExtendableIterator<>(); + results.extend(c1).extend(c2).extend(c3); + + results.next(); + results.hasNext(); + + Assert.assertTrue(c1.closed()); // close after iterated + Assert.assertFalse(c2.closed()); + Assert.assertFalse(c3.closed()); + + results.close(); + + Assert.assertTrue(c1.closed()); + Assert.assertTrue(c2.closed()); + Assert.assertTrue(c3.closed()); + } + + @Test + public void testCloseAfterNext3() throws Exception { CloseableItor c1 = new CloseableItor<>(DATA1.iterator()); CloseableItor c2 = new CloseableItor<>(DATA2.iterator()); CloseableItor c3 = new CloseableItor<>(DATA3.iterator()); @@ -191,8 +214,8 @@ public void testCloseAfterNext() throws Exception { results.next(); } - Assert.assertFalse(c1.closed()); - Assert.assertFalse(c2.closed()); + Assert.assertTrue(c1.closed()); + Assert.assertTrue(c2.closed()); Assert.assertFalse(c3.closed()); results.close(); From 684d8414f92c7398a7de377420128c19e721c302 Mon Sep 17 00:00:00 2001 From: Zhangmei Li Date: Mon, 30 Dec 2019 13:24:30 +0800 Subject: [PATCH 4/6] support ListIterator construct from list Change-Id: I3ed1e3431f3d9b4ba6f66479fd557808c4c28ddc --- .../hugegraph/iterator/IterableIterator.java | 52 --------- .../hugegraph/iterator/ListIterator.java | 24 ++-- .../unit/iterator/ListIteratorTest.java | 108 +++++++++++++++--- 3 files changed, 106 insertions(+), 78 deletions(-) delete mode 100644 src/main/java/com/baidu/hugegraph/iterator/IterableIterator.java diff --git a/src/main/java/com/baidu/hugegraph/iterator/IterableIterator.java b/src/main/java/com/baidu/hugegraph/iterator/IterableIterator.java deleted file mode 100644 index eb2a96637..000000000 --- a/src/main/java/com/baidu/hugegraph/iterator/IterableIterator.java +++ /dev/null @@ -1,52 +0,0 @@ -/* - * Copyright 2017 HugeGraph Authors - * - * 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 com.baidu.hugegraph.iterator; - -import java.util.Iterator; - -public class IterableIterator implements Iterator { - - private final Iterable iterable; - private final Iterator iterator; - - public IterableIterator(Iterable iterable) { - this.iterable = iterable; - this.iterator = iterable.iterator(); - } - - @Override - public boolean hasNext() { - return this.iterator.hasNext(); - } - - @Override - public T next() { - return this.iterator.next(); - } - - @Override - public void remove() { - this.iterator.remove(); - } - - public Iterable iterable() { - return iterable; - } -} diff --git a/src/main/java/com/baidu/hugegraph/iterator/ListIterator.java b/src/main/java/com/baidu/hugegraph/iterator/ListIterator.java index a46f73857..f21a43646 100644 --- a/src/main/java/com/baidu/hugegraph/iterator/ListIterator.java +++ b/src/main/java/com/baidu/hugegraph/iterator/ListIterator.java @@ -19,6 +19,7 @@ package com.baidu.hugegraph.iterator; +import java.util.Collection; import java.util.Collections; import java.util.Iterator; import java.util.List; @@ -29,18 +30,27 @@ public class ListIterator extends WrappedIterator { private final Iterator originIterator; private final Iterator resultsIterator; - private final List results; + private final Collection results; public ListIterator(long capacity, Iterator origin) { - this.originIterator = origin; - this.results = InsertionOrderUtil.newList(); + List results = InsertionOrderUtil.newList(); while (origin.hasNext()) { - if (capacity >= 0L && this.results.size() >= capacity) { + if (capacity >= 0L && results.size() >= capacity) { throw new IllegalArgumentException( "The iterator exceeded capacity " + capacity); } - this.results.add(origin.next()); + results.add(origin.next()); } + this.originIterator = origin; + this.results = Collections.unmodifiableList(results); + this.resultsIterator = this.results.iterator(); + } + + public ListIterator(Collection origin) { + this.originIterator = origin.iterator(); + this.results = origin instanceof List ? + Collections.unmodifiableList((List) origin) : + Collections.unmodifiableCollection(origin); this.resultsIterator = this.results.iterator(); } @@ -49,8 +59,8 @@ public void remove() { this.resultsIterator.remove(); } - public List list() { - return Collections.unmodifiableList(this.results); + public Collection list() { + return this.results; } @Override diff --git a/src/test/java/com/baidu/hugegraph/unit/iterator/ListIteratorTest.java b/src/test/java/com/baidu/hugegraph/unit/iterator/ListIteratorTest.java index 8fff02ff5..8ddd995ba 100644 --- a/src/test/java/com/baidu/hugegraph/unit/iterator/ListIteratorTest.java +++ b/src/test/java/com/baidu/hugegraph/unit/iterator/ListIteratorTest.java @@ -141,40 +141,36 @@ public void testHasNextAndNext() { @Test public void testRemove() { - List list3 = new ArrayList<>(DATA3); - Iterator results = new ListIterator<>(-1, list3.iterator()); + List list = new ArrayList<>(DATA3); + ListIterator results = new ListIterator<>(-1, list.iterator()); - Assert.assertEquals(ImmutableList.of(4, 5, 6), list3); + Assert.assertEquals(ImmutableList.of(4, 5, 6), list); + Assert.assertEquals(ImmutableList.of(4, 5, 6), results.list()); - Assert.assertEquals(4, (int) results.next()); - Assert.assertEquals(5, (int) results.next()); - results.remove(); - Assert.assertEquals(6, (int) results.next()); + Assert.assertThrows(UnsupportedOperationException.class, () -> { + results.remove(); + }); + results.next(); + Assert.assertThrows(UnsupportedOperationException.class, () -> { + results.remove(); + }); - Assert.assertEquals(ImmutableList.of(4, 5, 6), list3); + Assert.assertEquals(ImmutableList.of(4, 5, 6), list); + Assert.assertEquals(ImmutableList.of(4, 5, 6), results.list()); } @Test public void testRemoveWithoutResult() { Iterator results = new ListIterator<>(-1, EMPTY); - Assert.assertThrows(IllegalStateException.class, () -> { + Assert.assertThrows(UnsupportedOperationException.class, () -> { results.remove(); }); List list0 = new ArrayList<>(); Iterator results2 = new ListIterator<>(-1, list0.iterator()); - Assert.assertThrows(IllegalStateException.class, () -> { + Assert.assertThrows(UnsupportedOperationException.class, () -> { results2.remove(); }); - - List list1 = new ArrayList<>(DATA1); - Iterator results3 = new ListIterator<>(-1, list1.iterator()); - results3.next(); - Assert.assertThrows(NoSuchElementException.class, () -> { - results3.next(); - }); - results3.remove(); // OK - Assert.assertEquals(ImmutableList.of(1), list1); } @Test @@ -189,4 +185,78 @@ public void testClose() throws Exception { Assert.assertTrue(c1.closed()); } + + @Test + public void testListWithConstructFromList() { + ListIterator results; + + results = new ListIterator<>(ImmutableList.of()); + Assert.assertEquals(ImmutableList.of(), results.list()); + + results = new ListIterator<>(DATA1); + Assert.assertEquals(ImmutableList.of(1), results.list()); + + results = new ListIterator<>(DATA2); + Assert.assertEquals(ImmutableList.of(2, 3), results.list()); + + results = new ListIterator<>(DATA3); + Assert.assertEquals(ImmutableList.of(4, 5, 6), results.list()); + } + + @Test + public void testHasNextAndNextWithConstructFromList() { + ListIterator results0 = new ListIterator<>(ImmutableList.of()); + Assert.assertFalse(results0.hasNext()); + Assert.assertThrows(NoSuchElementException.class, () -> { + results0.next(); + }); + + ListIterator results1 = new ListIterator<>(DATA1); + Assert.assertTrue(results1.hasNext()); + Assert.assertEquals(1, results1.next()); + Assert.assertFalse(results1.hasNext()); + Assert.assertThrows(NoSuchElementException.class, () -> { + results1.next(); + }); + + ListIterator results3 = new ListIterator<>(DATA3); + Assert.assertTrue(results3.hasNext()); + Assert.assertEquals(4, results3.next()); + Assert.assertTrue(results3.hasNext()); + Assert.assertEquals(5, results3.next()); + Assert.assertTrue(results3.hasNext()); + Assert.assertEquals(6, results3.next()); + Assert.assertFalse(results3.hasNext()); + Assert.assertThrows(NoSuchElementException.class, () -> { + results3.next(); + }); + } + + @Test + public void testNextWithConstructFromList() { + ListIterator results0 = new ListIterator<>(ImmutableList.of()); + Assert.assertThrows(NoSuchElementException.class, () -> { + results0.next(); + }); + + ListIterator results3 = new ListIterator<>(DATA3); + Assert.assertEquals(4, results3.next()); + Assert.assertEquals(5, results3.next()); + Assert.assertEquals(6, results3.next()); + Assert.assertThrows(NoSuchElementException.class, () -> { + results3.next(); + }); + } + + @Test + public void testNextAfterListWithConstructFromList() { + ListIterator results3 = new ListIterator<>(DATA3); + Assert.assertEquals(ImmutableList.of(4, 5, 6), results3.list()); + Assert.assertEquals(4, results3.next()); + Assert.assertEquals(5, results3.next()); + Assert.assertEquals(6, results3.next()); + Assert.assertThrows(NoSuchElementException.class, () -> { + results3.next(); + }); + } } From 9964a8fb4cef8c31447c85c4b2726bab5d58caba Mon Sep 17 00:00:00 2001 From: Zhangmei Li Date: Wed, 1 Jan 2020 01:20:16 +0800 Subject: [PATCH 5/6] upgrade version Change-Id: I12698bc3aa17b8b9cd71fe6646a483a1c3dedbc9 --- pom.xml | 4 ++-- src/main/java/com/baidu/hugegraph/version/CommonVersion.java | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pom.xml b/pom.xml index 9ad4475d3..7f639e5c2 100644 --- a/pom.xml +++ b/pom.xml @@ -6,7 +6,7 @@ com.baidu.hugegraph hugegraph-common - 1.7.1 + 1.7.2 hugegraph-common https://github.com/hugegraph/hugegraph-common @@ -218,7 +218,7 @@ - 1.7.1.0 + 1.7.2.0 diff --git a/src/main/java/com/baidu/hugegraph/version/CommonVersion.java b/src/main/java/com/baidu/hugegraph/version/CommonVersion.java index 5bfcbcb53..5c4ed4b68 100644 --- a/src/main/java/com/baidu/hugegraph/version/CommonVersion.java +++ b/src/main/java/com/baidu/hugegraph/version/CommonVersion.java @@ -27,5 +27,5 @@ public class CommonVersion { // The second parameter of Version.of() is for all-in-one JAR public static final Version VERSION = Version.of(CommonVersion.class, - "1.7.1"); + "1.7.2"); } From e206849979781779badd561df216cc7d7c48eb76 Mon Sep 17 00:00:00 2001 From: Zhangmei Li Date: Wed, 8 Jan 2020 14:26:26 +0800 Subject: [PATCH 6/6] tiny improve Change-Id: I5c6fe38612ead857ab25d8ebabf2f9775b4ac3da --- .../hugegraph/iterator/BatchMapperIterator.java | 10 +++++----- .../hugegraph/iterator/FlatMapperFilterIterator.java | 4 ++-- .../baidu/hugegraph/iterator/FlatMapperIterator.java | 12 ++++++------ .../unit/iterator/BatchMapperIteratorTest.java | 7 ++++++- 4 files changed, 19 insertions(+), 14 deletions(-) diff --git a/src/main/java/com/baidu/hugegraph/iterator/BatchMapperIterator.java b/src/main/java/com/baidu/hugegraph/iterator/BatchMapperIterator.java index 343c7b5f1..6b2a9135f 100644 --- a/src/main/java/com/baidu/hugegraph/iterator/BatchMapperIterator.java +++ b/src/main/java/com/baidu/hugegraph/iterator/BatchMapperIterator.java @@ -55,7 +55,7 @@ protected final boolean fetch() { return true; } - List list = this.fetchBatch(); + List list = this.nextBatch(); if (!list.isEmpty()) { assert this.batchIterator == null; // Do fetch @@ -67,7 +67,7 @@ protected final boolean fetch() { return false; } - protected List fetchBatch() { + protected final List nextBatch() { if (!this.originIterator.hasNext()) { return ImmutableList.of(); } @@ -79,7 +79,7 @@ protected List fetchBatch() { return list; } - protected boolean fetchFromBatch() { + protected final boolean fetchFromBatch() { E.checkNotNull(this.batchIterator, "mapper results"); while (this.batchIterator.hasNext()) { R result = this.batchIterator.next(); @@ -89,11 +89,11 @@ protected boolean fetchFromBatch() { return true; } } - this.resetResults(); + this.resetBatchIterator(); return false; } - protected final void resetResults() { + protected final void resetBatchIterator() { if (this.batchIterator == null) { return; } diff --git a/src/main/java/com/baidu/hugegraph/iterator/FlatMapperFilterIterator.java b/src/main/java/com/baidu/hugegraph/iterator/FlatMapperFilterIterator.java index 577c3da7f..a222319b1 100644 --- a/src/main/java/com/baidu/hugegraph/iterator/FlatMapperFilterIterator.java +++ b/src/main/java/com/baidu/hugegraph/iterator/FlatMapperFilterIterator.java @@ -36,7 +36,7 @@ public FlatMapperFilterIterator(Iterator origin, } @Override - protected final boolean fetchFromMap() { + protected final boolean fetchFromBatch() { E.checkNotNull(this.batchIterator, "mapper results"); while (this.batchIterator.hasNext()) { R result = this.batchIterator.next(); @@ -46,7 +46,7 @@ protected final boolean fetchFromMap() { return true; } } - this.resetResults(); + this.resetBatchIterator(); return false; } } diff --git a/src/main/java/com/baidu/hugegraph/iterator/FlatMapperIterator.java b/src/main/java/com/baidu/hugegraph/iterator/FlatMapperIterator.java index 098ba8888..6b4cd9855 100644 --- a/src/main/java/com/baidu/hugegraph/iterator/FlatMapperIterator.java +++ b/src/main/java/com/baidu/hugegraph/iterator/FlatMapperIterator.java @@ -40,7 +40,7 @@ public FlatMapperIterator(Iterator origin, @Override public void close() throws Exception { - this.resetResults(); + this.resetBatchIterator(); super.close(); } @@ -51,7 +51,7 @@ protected Iterator originIterator() { @Override protected final boolean fetch() { - if (this.batchIterator != null && this.fetchFromMap()) { + if (this.batchIterator != null && this.fetchFromBatch()) { return true; } @@ -60,14 +60,14 @@ protected final boolean fetch() { assert this.batchIterator == null; // Do fetch this.batchIterator = this.mapperCallback.apply(next); - if (this.batchIterator != null && this.fetchFromMap()) { + if (this.batchIterator != null && this.fetchFromBatch()) { return true; } } return false; } - protected boolean fetchFromMap() { + protected boolean fetchFromBatch() { E.checkNotNull(this.batchIterator, "mapper results"); while (this.batchIterator.hasNext()) { R result = this.batchIterator.next(); @@ -77,11 +77,11 @@ protected boolean fetchFromMap() { return true; } } - this.resetResults(); + this.resetBatchIterator(); return false; } - protected final void resetResults() { + protected final void resetBatchIterator() { if (this.batchIterator == null) { return; } diff --git a/src/test/java/com/baidu/hugegraph/unit/iterator/BatchMapperIteratorTest.java b/src/test/java/com/baidu/hugegraph/unit/iterator/BatchMapperIteratorTest.java index 5aa196de9..e08ff6921 100644 --- a/src/test/java/com/baidu/hugegraph/unit/iterator/BatchMapperIteratorTest.java +++ b/src/test/java/com/baidu/hugegraph/unit/iterator/BatchMapperIteratorTest.java @@ -142,6 +142,11 @@ public void testNext() { Assert.assertEquals(4, results.next()); Assert.assertEquals(5, results.next()); Assert.assertEquals(6, results.next()); + + results = new BatchMapperIterator<>(2, DATA3.iterator(), MAPPER); + Assert.assertEquals(4, results.next()); + Assert.assertEquals(5, results.next()); + Assert.assertEquals(6, results.next()); } @Test @@ -204,7 +209,7 @@ public void testMapperWithBatch() { } @Test - public void testMapperTHenReturn2X() { + public void testMapperThenReturn2X() { Iterator results; results = new BatchMapperIterator<>(1, DATA3.iterator(), batch -> {