A few transfer API improvements and deprecations (#3204)

* A few transfer API improvements and deprecations

* Forward implementation of deprecated methods

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

@@ -32,11 +32,14 @@
 /**
  * An object that can store resources.
  *
+ * <p>Most of the documentation that follows is quite technical.
+ * For an easier introduction to the API, see the <a href="https://fabricmc.net/wiki/tutorial:transfer-api">wiki page</a>.
+ *
  * <p><ul>
  *     <li>{@link #supportsInsertion} and {@link #supportsExtraction} can be used to tell if insertion and extraction
  *     functionality are possibly supported by this storage.</li>
  *     <li>{@link #insert} and {@link #extract} can be used to insert or extract resources from this storage.</li>
- *     <li>{@link #iterator} and {@link #exactView} can be used to inspect the contents of this storage.</li>
+ *     <li>{@link #iterator} can be used to inspect the contents of this storage.</li>
  *     <li>{@link #getVersion()} can be used to quickly check if a storage has changed, without having to rescan its contents.</li>
  * </ul>
  *
@@ -92,17 +95,6 @@ default boolean supportsInsertion() {
 	 */
 	long insert(T resource, long maxAmount, TransactionContext transaction);
 
-	/**
-	 * Convenient helper to simulate an insertion, i.e. get the result of insert without modifying any state.
-	 * The passed transaction may be null if a new transaction should be opened for the simulation.
-	 * @see #insert
-	 */
-	default long simulateInsert(T resource, long maxAmount, @Nullable TransactionContext transaction) {
-		try (Transaction simulateTransaction = Transaction.openNested(transaction)) {
-			return insert(resource, maxAmount, simulateTransaction);
-		}
-	}
-
 	/**
 	 * Return false if calling {@link #extract} will absolutely always return 0, or true otherwise or in doubt.
 	 *
@@ -123,17 +115,6 @@ default boolean supportsExtraction() {
 	 */
 	long extract(T resource, long maxAmount, TransactionContext transaction);
 
-	/**
-	 * Convenient helper to simulate an extraction, i.e. get the result of extract without modifying any state.
-	 * The passed transaction may be null if a new transaction should be opened for the simulation.
-	 * @see #extract
-	 */
-	default long simulateExtract(T resource, long maxAmount, @Nullable TransactionContext transaction) {
-		try (Transaction simulateTransaction = Transaction.openNested(transaction)) {
-			return extract(resource, maxAmount, simulateTransaction);
-		}
-	}
-
 	/**
 	 * Iterate through the contents of this storage.
 	 * Every visited {@link StorageView} represents a stored resource and an amount.
@@ -187,22 +168,6 @@ default Iterable<StorageView<T>> nonEmptyViews() {
 		return this::nonEmptyIterator;
 	}
 
-	/**
-	 * Return a view over this storage, for a specific resource, or {@code null} if none is quickly available.
-	 *
-	 * <p>This function should only return a non-null view if this storage can provide it quickly,
-	 * for example with a hashmap lookup.
-	 * If returning the requested view would require iteration through a potentially large number of views,
-	 * {@code null} should be returned instead.
-	 *
-	 * @param resource The resource for which a storage view is requested. May be blank, for example to estimate capacity.
-	 * @return A view over this storage for the passed resource, or {@code null} if none is quickly available.
-	 */
-	@Nullable
-	default StorageView<T> exactView(T resource) {
-		return null;
-	}
-
 	/**
 	 * Return an integer representing the current version of this storage instance to allow for fast change detection:
 	 * if the version hasn't changed since the last time, <b>and the storage instance is the same</b>, the storage has the same contents.
@@ -247,4 +212,44 @@ default long getVersion() {
 	static <T> Class<Storage<T>> asClass() {
 		return (Class<Storage<T>>) (Object) Storage.class;
 	}
+
+	/**
+	 * Convenient helper to simulate an insertion, i.e. get the result of insert without modifying any state.
+	 * The passed transaction may be null if a new transaction should be opened for the simulation.
+	 * @see #insert
+	 * @deprecated Either use transactions directly, or use {@link StorageUtil#simulateInsert}.
+	 */
+	@Deprecated(forRemoval = true)
+	default long simulateInsert(T resource, long maxAmount, @Nullable TransactionContext transaction) {
+		return StorageUtil.simulateInsert(this, resource, maxAmount, transaction);
+	}
+
+	/**
+	 * Convenient helper to simulate an extraction, i.e. get the result of extract without modifying any state.
+	 * The passed transaction may be null if a new transaction should be opened for the simulation.
+	 * @see #extract
+	 * @deprecated Either use transactions directly, or use {@link StorageUtil#simulateExtract}.
+	 */
+	@Deprecated(forRemoval = true)
+	default long simulateExtract(T resource, long maxAmount, @Nullable TransactionContext transaction) {
+		return StorageUtil.simulateExtract(this, resource, maxAmount, transaction);
+	}
+
+	/**
+	 * Return a view over this storage, for a specific resource, or {@code null} if none is quickly available.
+	 *
+	 * <p>This function should only return a non-null view if this storage can provide it quickly,
+	 * for example with a hashmap lookup.
+	 * If returning the requested view would require iteration through a potentially large number of views,
+	 * {@code null} should be returned instead.
+	 *
+	 * @param resource The resource for which a storage view is requested. May be blank, for example to estimate capacity.
+	 * @return A view over this storage for the passed resource, or {@code null} if none is quickly available.
+	 * @deprecated Deprecated for removal without direct replacement. Use {@link #insert}, {@link #extract} or {@link #iterator} instead.
+	 */
+	@Deprecated(forRemoval = true)
+	@Nullable
+	default StorageView<T> exactView(T resource) {
+		return null;
+	}
 }

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

@@ -93,13 +93,9 @@ public static <T> long move(@Nullable Storage<T> from, @Nullable Storage<T> to,
 			for (StorageView<T> view : from.nonEmptyViews()) {
 				T resource = view.getResource();
 				if (!filter.test(resource)) continue;
-				long maxExtracted;
 
 				// check how much can be extracted
-				try (Transaction extractionTestTransaction = iterationTransaction.openNested()) {
-					maxExtracted = view.extract(resource, maxAmount - totalMoved, extractionTestTransaction);
-					extractionTestTransaction.abort();
-				}
+				long maxExtracted = simulateExtract(view, resource, maxAmount - totalMoved, iterationTransaction);
 
 				try (Transaction transferTransaction = iterationTransaction.openNested()) {
 					// check how much can be inserted
@@ -134,6 +130,52 @@ public static <T> long move(@Nullable Storage<T> from, @Nullable Storage<T> to,
 		return totalMoved;
 	}
 
+	/**
+	 * Convenient helper to simulate an insertion, i.e. get the result of insert without modifying any state.
+	 * The passed transaction may be null if a new transaction should be opened for the simulation.
+	 * @see Storage#insert
+	 */
+	public static <T> long simulateInsert(Storage<T> storage, T resource, long maxAmount, @Nullable TransactionContext transaction) {
+		try (Transaction simulateTransaction = Transaction.openNested(transaction)) {
+			return storage.insert(resource, maxAmount, simulateTransaction);
+		}
+	}
+
+	/**
+	 * Convenient helper to simulate an extraction, i.e. get the result of extract without modifying any state.
+	 * The passed transaction may be null if a new transaction should be opened for the simulation.
+	 * @see Storage#insert
+	 */
+	public static <T> long simulateExtract(Storage<T> storage, T resource, long maxAmount, @Nullable TransactionContext transaction) {
+		try (Transaction simulateTransaction = Transaction.openNested(transaction)) {
+			return storage.extract(resource, maxAmount, simulateTransaction);
+		}
+	}
+
+	/**
+	 * Convenient helper to simulate an extraction, i.e. get the result of extract without modifying any state.
+	 * The passed transaction may be null if a new transaction should be opened for the simulation.
+	 * @see Storage#insert
+	 */
+	public static <T> long simulateExtract(StorageView<T> storageView, T resource, long maxAmount, @Nullable TransactionContext transaction) {
+		try (Transaction simulateTransaction = Transaction.openNested(transaction)) {
+			return storageView.extract(resource, maxAmount, simulateTransaction);
+		}
+	}
+
+	/**
+	 * Convenient helper to simulate an extraction, i.e. get the result of extract without modifying any state.
+	 * The passed transaction may be null if a new transaction should be opened for the simulation.
+	 * @see Storage#insert
+	 * @apiNote This function handles the method overload conflict for objects that implement both {@link Storage} and {@link StorageView}.
+	 */
+	// Object & is used to have a different erasure than the other overloads.
+	public static <T, S extends Object & Storage<T> & StorageView<T>> long simulateExtract(S storage, T resource, long maxAmount, @Nullable TransactionContext transaction) {
+		try (Transaction simulateTransaction = Transaction.openNested(transaction)) {
+			return storage.extract(resource, maxAmount, simulateTransaction);
+		}
+	}
+
 	/**
 	 * Try to extract any resource from a storage, up to a maximum amount.
 	 *
@@ -336,7 +378,7 @@ public static <T> ResourceAmount<T> findExtractableContent(@Nullable Storage<T>
 		T extractableResource = findExtractableResource(storage, filter, transaction);
 
 		if (extractableResource != null) {
-			long extractableAmount = storage.simulateExtract(extractableResource, Long.MAX_VALUE, transaction);
+			long extractableAmount = simulateExtract(storage, extractableResource, Long.MAX_VALUE, transaction);
 
 			if (extractableAmount > 0) {
 				return new ResourceAmount<>(extractableResource, extractableAmount);

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

@@ -21,7 +21,7 @@
 import net.fabricmc.fabric.api.transfer.v1.transaction.TransactionContext;
 
 /**
- * A view of a single stored resource in a {@link Storage}, for use with {@link Storage#iterator} or {@link Storage#exactView}.
+ * A view of a single stored resource in a {@link Storage}, for use with {@link Storage#iterator}.
  *
  * @param <T> The type of the stored resource.
  *
@@ -57,7 +57,7 @@ public interface StorageView<T> {
 
 	/**
 	 * @return The total amount of {@link #getResource} that could be stored in this view,
-	 * or an estimate of the number of resources that could be stored if this view has a blank resource.
+	 * or an estimated upper bound on the number of resources that could be stored if this view has a blank resource.
 	 */
 	long getCapacity();
 

fabric-transfer-api-v1/src/testmod/java/net/fabricmc/fabric/test/transfer/gametests/VanillaStorageTests.java

@@ -45,6 +45,7 @@
 import net.fabricmc.fabric.api.transfer.v1.item.ItemVariant;
 import net.fabricmc.fabric.api.transfer.v1.storage.SlottedStorage;
 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.transaction.Transaction;
 import net.fabricmc.fabric.test.transfer.mixin.AbstractFurnaceBlockEntityAccessor;
 
@@ -224,7 +225,7 @@ public void testShulkerNoInsert(TestContext context) {
 		ShulkerBoxBlockEntity shulker = (ShulkerBoxBlockEntity) context.getBlockEntity(pos);
 		InventoryStorage storage = InventoryStorage.of(shulker, null);
 
-		if (storage.simulateInsert(ItemVariant.of(Items.SHULKER_BOX), 1, null) > 0) {
+		if (StorageUtil.simulateInsert(storage, ItemVariant.of(Items.SHULKER_BOX), 1, null) > 0) {
 			context.throwPositionedException("Expected shulker box to be rejected", pos);
 		}
 

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

@@ -66,9 +66,9 @@ protected boolean canInsert(FluidVariant resource) {
 		}
 
 		// Insertion through the filter should fail.
-		assertEquals(0L, noWater.simulateInsert(water, BUCKET, null));
+		assertEquals(0L, StorageUtil.simulateInsert(noWater, water, BUCKET, null));
 		// Extraction should also fail.
-		assertEquals(0L, noWater.simulateExtract(water, BUCKET, null));
+		assertEquals(0L, StorageUtil.simulateExtract(noWater, water, BUCKET, null));
 		// The fluid should be visible.
 		assertEquals(water, StorageUtil.findStoredResource(noWater));
 		// Test the filter.
@@ -83,13 +83,13 @@ protected boolean canInsert(FluidVariant resource) {
 		// Lava insertion and extract should proceed just fine.
 		try (Transaction tx = Transaction.openOuter()) {
 			assertEquals(BUCKET, noWater.insert(lava, BUCKET, tx));
-			assertEquals(BUCKET, noWater.simulateExtract(lava, BUCKET, tx));
+			assertEquals(BUCKET, StorageUtil.simulateExtract(noWater, lava, BUCKET, tx));
 			// Test that simulating doesn't change the state...
-			assertEquals(BUCKET, noWater.simulateExtract(lava, BUCKET, tx));
-			assertEquals(BUCKET, noWater.simulateExtract(lava, BUCKET, tx));
+			assertEquals(BUCKET, StorageUtil.simulateExtract(noWater, lava, BUCKET, tx));
+			assertEquals(BUCKET, StorageUtil.simulateExtract(noWater, lava, BUCKET, tx));
 			tx.commit();
 		}
 
-		assertEquals(BUCKET, storage.simulateExtract(lava, BUCKET, null));
+		assertEquals(BUCKET, StorageUtil.simulateExtract(storage, lava, BUCKET, null));
 	}
 }

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

@@ -22,6 +22,7 @@
 import net.minecraft.nbt.NbtCompound;
 
 import net.fabricmc.fabric.api.transfer.v1.fluid.FluidVariant;
+import net.fabricmc.fabric.api.transfer.v1.storage.StorageUtil;
 import net.fabricmc.fabric.api.transfer.v1.storage.base.SingleSlotStorage;
 import net.fabricmc.fabric.api.transfer.v1.storage.base.SingleVariantStorage;
 import net.fabricmc.fabric.api.transfer.v1.transaction.Transaction;
@@ -78,7 +79,7 @@ private static void testFluidStorage() {
 			// Should not allow lava (canInsert returns false)
 			if (waterStorage.insert(LAVA, BUCKET, tx) != 0) throw new AssertionError("Lava inserted");
 			// Should allow insert, but without mutating the storage.
-			if (waterStorage.simulateInsert(WATER, BUCKET, tx) != BUCKET) throw new AssertionError("Simulated insert failed");
+			if (StorageUtil.simulateInsert(waterStorage, WATER, BUCKET, tx) != BUCKET) throw new AssertionError("Simulated insert failed");
 			// Should allow insert
 			if (waterStorage.insert(TAGGED_WATER, BUCKET, tx) != BUCKET) throw new AssertionError("Tagged water insert 1 failed");
 			// Variants are different, should not allow insert
@@ -90,7 +91,7 @@ private static void testFluidStorage() {
 			// Should allow extraction
 			if (waterStorage.extract(TAGGED_WATER_2, BUCKET, tx) != BUCKET) throw new AssertionError("Extraction failed");
 			// Simulated extraction should succeed but do nothing
-			if (waterStorage.simulateExtract(TAGGED_WATER, Long.MAX_VALUE, tx) != BUCKET) throw new AssertionError("Simulated extraction failed");
+			if (StorageUtil.simulateExtract(waterStorage, TAGGED_WATER, Long.MAX_VALUE, tx) != BUCKET) throw new AssertionError("Simulated extraction failed");
 			// Re-insert
 			if (waterStorage.insert(TAGGED_WATER_2, BUCKET, tx) != BUCKET) throw new AssertionError("Tagged water insert 3 failed");
 			// Test contents