Fix empty storage iterator returning views that become empty during i…

fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/api/transfer/v1/storage/Storage.java

@@ -18,6 +18,8 @@
 
 import java.util.Iterator;
 
+import com.google.common.collect.Iterators;
+
 import net.fabricmc.fabric.api.transfer.v1.storage.base.CombinedStorage;
 import net.fabricmc.fabric.api.transfer.v1.storage.base.ExtractionOnlyStorage;
 import net.fabricmc.fabric.api.transfer.v1.storage.base.InsertionOnlyStorage;
@@ -145,7 +147,7 @@ default boolean supportsExtraction() {
 	 * @return An iterator over the non-empty views of this storage. Calling remove on the iterator is not allowed.
 	 */
 	default Iterator<StorageView<T>> nonEmptyIterator() {
-		return TransferApiImpl.filterEmptyViews(iterator());
+		return Iterators.filter(iterator(), view -> view.getAmount() > 0 && !view.isResourceBlank());
 	}
 
 	/**

fabric-transfer-api-v1/src/main/java/net/fabricmc/fabric/impl/transfer/TransferApiImpl.java

@@ -94,44 +94,6 @@ public T next() {
 		};
 	}
 
-	public static <T> Iterator<StorageView<T>> filterEmptyViews(Iterator<StorageView<T>> iterator) {
-		return new Iterator<>() {
-			StorageView<T> next;
-
-			{
-				findNext();
-			}
-
-			private void findNext() {
-				while (iterator.hasNext()) {
-					next = iterator.next();
-
-					if (next.getAmount() > 0 && !next.isResourceBlank()) {
-						return;
-					}
-				}
-
-				next = null;
-			}
-
-			@Override
-			public boolean hasNext() {
-				return next != null;
-			}
-
-			@Override
-			public StorageView<T> next() {
-				if (!hasNext()) {
-					throw new NoSuchElementException();
-				}
-
-				StorageView<T> ret = next;
-				findNext();
-				return ret;
-			}
-		};
-	}
-
 	public static <T> List<SingleSlotStorage<T>> makeListView(SlottedStorage<T> storage) {
 		return new AbstractList<>() {
 			@Override

fabric-transfer-api-v1/src/testmod/java/net/fabricmc/fabric/test/transfer/unittests/BaseStorageTests.java

@@ -19,18 +19,23 @@
 import static net.fabricmc.fabric.api.transfer.v1.fluid.FluidConstants.BUCKET;
 import static net.fabricmc.fabric.test.transfer.unittests.TestUtil.assertEquals;
 
+import java.util.Iterator;
+
 import net.minecraft.fluid.Fluids;
 
 import net.fabricmc.fabric.api.transfer.v1.fluid.FluidVariant;
+import net.fabricmc.fabric.api.transfer.v1.fluid.base.SingleFluidStorage;
 import net.fabricmc.fabric.api.transfer.v1.storage.Storage;
 import net.fabricmc.fabric.api.transfer.v1.storage.StorageUtil;
+import net.fabricmc.fabric.api.transfer.v1.storage.StorageView;
 import net.fabricmc.fabric.api.transfer.v1.storage.base.FilteringStorage;
 import net.fabricmc.fabric.api.transfer.v1.storage.base.SingleVariantStorage;
 import net.fabricmc.fabric.api.transfer.v1.transaction.Transaction;
 
 public class BaseStorageTests {
 	public static void run() {
 		testFilteringStorage();
+		testNonEmptyIteratorWithModifiedView();
 	}
 
 	private static void testFilteringStorage() {
@@ -92,4 +97,24 @@ protected boolean canInsert(FluidVariant resource) {
 
 		assertEquals(BUCKET, StorageUtil.simulateExtract(storage, lava, BUCKET, null));
 	}
+
+	/**
+	 * Regression test for <a href="https://github.com/FabricMC/fabric/issues/3414">
+	 * {@code nonEmptyIterator} not handling views that become empty during iteration correctly</a>.
+	 */
+	private static void testNonEmptyIteratorWithModifiedView() {
+		SingleVariantStorage<FluidVariant> storage = SingleFluidStorage.withFixedCapacity(BUCKET, () -> { });
+		storage.variant = FluidVariant.of(Fluids.WATER);
+
+		Iterator<StorageView<FluidVariant>> iterator = storage.nonEmptyIterator();
+		storage.amount = BUCKET;
+		// Iterator should have a next element now
+		assertEquals(true, iterator.hasNext());
+		assertEquals(storage, iterator.next());
+
+		iterator = storage.nonEmptyIterator();
+		storage.amount = 0;
+		// Iterator should not have a next element...
+		assertEquals(false, iterator.hasNext());
+	}
 }