Skip to content

Commit

Permalink
Implement fault tolerant de-/serialization of AE stacks to no longer …
Browse files Browse the repository at this point in the history
…void entire drives if they contain broken content (#7982)
  • Loading branch information
shartte committed Jun 27, 2024
1 parent d0baf5a commit 9f429ac
Show file tree
Hide file tree
Showing 9 changed files with 194 additions and 13 deletions.
6 changes: 3 additions & 3 deletions src/main/java/appeng/api/ids/AEComponents.java
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ private AEComponents() {
* @see appeng.items.tools.MemoryCardItem
*/
public static final DataComponentType<List<GenericStack>> EXPORTED_CONFIG_INV = register("exported_config_inv",
builder -> builder.persistent(GenericStack.NULLABLE_LIST_CODEC)
builder -> builder.persistent(GenericStack.FAULT_TOLERANT_NULLABLE_LIST_CODEC)
.networkSynchronized(GenericStack.STREAM_CODEC.apply(ByteBufCodecs.list())));

/**
Expand Down Expand Up @@ -301,15 +301,15 @@ private AEComponents() {
* Content of a storage cell.
*/
public static final DataComponentType<List<GenericStack>> STORAGE_CELL_INV = register("storage_cell_inv",
builder -> builder.persistent(GenericStack.CODEC.listOf())
builder -> builder.persistent(GenericStack.FAULT_TOLERANT_LIST_CODEC)
.networkSynchronized(GenericStack.STREAM_CODEC.apply(ByteBufCodecs.list())));

/**
* Defines partitioning for a storage cell.
*/
public static final DataComponentType<List<GenericStack>> STORAGE_CELL_CONFIG_INV = register(
"storage_cell_config_inv",
builder -> builder.persistent(GenericStack.NULLABLE_LIST_CODEC)
builder -> builder.persistent(GenericStack.FAULT_TOLERANT_NULLABLE_LIST_CODEC)
.networkSynchronized(GenericStack.STREAM_CODEC.apply(ByteBufCodecs.list())));

/**
Expand Down
69 changes: 68 additions & 1 deletion src/main/java/appeng/api/stacks/GenericStack.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,12 @@
import java.util.List;
import java.util.Objects;

import com.mojang.datafixers.util.Pair;
import com.mojang.serialization.Codec;
import com.mojang.serialization.DataResult;
import com.mojang.serialization.Dynamic;
import com.mojang.serialization.DynamicOps;
import com.mojang.serialization.Lifecycle;
import com.mojang.serialization.codecs.RecordCodecBuilder;

import org.jetbrains.annotations.ApiStatus;
Expand All @@ -17,8 +22,11 @@
import net.minecraft.network.RegistryFriendlyByteBuf;
import net.minecraft.network.codec.StreamCodec;
import net.minecraft.world.item.ItemStack;
import net.minecraft.world.item.component.CustomData;
import net.neoforged.neoforge.fluids.FluidStack;

import appeng.api.ids.AEComponents;
import appeng.core.definitions.AEItems;
import appeng.items.misc.WrappedGenericStack;

/**
Expand All @@ -39,7 +47,66 @@ public record GenericStack(AEKey what, long amount) {
GenericStack::writeBuffer,
GenericStack::readBuffer);

public static final Codec<List<@Nullable GenericStack>> NULLABLE_LIST_CODEC = new GenericStackListCodec();
/**
* This result function converts failed serialization results for GenericStack into missing content storing the
* error message.
*/
private static final Codec.ResultFunction<GenericStack> MISSING_CONTENT_GENERICSTACK_RESULT = new Codec.ResultFunction<>() {
@Override
public <T> DataResult<Pair<GenericStack, T>> apply(DynamicOps<T> ops, T input,
DataResult<Pair<GenericStack, T>> a) {
if (a instanceof DataResult.Error<Pair<GenericStack, T>> error) {
var missingContent = AEItems.MISSING_CONTENT.stack();
var convert = Dynamic.convert(ops, NbtOps.INSTANCE, input);
if (convert instanceof CompoundTag compoundTag) {
missingContent.set(AEComponents.MISSING_CONTENT_ITEMSTACK_DATA, CustomData.of(compoundTag));
}
LOG.error("Failed to deserialize GenericStack {}: {}", input, error.message());
missingContent.set(AEComponents.MISSING_CONTENT_ERROR, error.message());

var replacement = new GenericStack(AEItemKey.of(missingContent), 1);

return DataResult.success(
Pair.of(replacement, input),
Lifecycle.stable());
}

// Return unchanged if deserialization succeeded
return a;
}

@Override
public <T> DataResult<T> coApply(DynamicOps<T> ops, GenericStack input, DataResult<T> t) {
// When the serialization result failed, we write a missing content item instead
// this one will NOT be recoverable
if (t instanceof DataResult.Error<T> error) {
var missingContent = AEItems.MISSING_CONTENT.stack();
LOG.error("Failed to serialize GenericStack {}: {}", input, error.message());
missingContent.set(AEComponents.MISSING_CONTENT_ERROR, error.message());

var replacement = new GenericStack(AEItemKey.of(missingContent), 1);
return CODEC.encodeStart(ops, replacement).setLifecycle(t.lifecycle());
}

// When the input is a MISSING_CONTENT item and has the original data attached,
// we write that back.
if (input.what() instanceof AEItemKey itemKey && itemKey.is(AEItems.MISSING_CONTENT)) {
var originalData = itemKey.get(AEComponents.MISSING_CONTENT_ITEMSTACK_DATA);
if (originalData != null) {
return DataResult.success(Dynamic.convert(NbtOps.INSTANCE, ops, originalData.getUnsafe()),
t.lifecycle());
}
}

return t;
}
};

public static final Codec<List<@Nullable GenericStack>> FAULT_TOLERANT_NULLABLE_LIST_CODEC = new GenericStackListCodec(
CODEC.mapResult(MISSING_CONTENT_GENERICSTACK_RESULT));

public static final Codec<List<GenericStack>> FAULT_TOLERANT_LIST_CODEC = CODEC
.mapResult(MISSING_CONTENT_GENERICSTACK_RESULT).listOf();

public GenericStack {
Objects.requireNonNull(what, "what");
Expand Down
14 changes: 10 additions & 4 deletions src/main/java/appeng/api/stacks/GenericStackListCodec.java
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,21 @@
import org.jetbrains.annotations.Nullable;

class GenericStackListCodec implements Codec<List<@Nullable GenericStack>> {
private final Codec<GenericStack> innerCodec;

public GenericStackListCodec(Codec<GenericStack> innerCodec) {
this.innerCodec = innerCodec;
}

@Override
public <T> DataResult<T> encode(List<@Nullable GenericStack> input, DynamicOps<T> ops, T prefix) {
final ListBuilder<T> builder = ops.listBuilder();

for (final GenericStack a : input) {
if (a == null) {
for (var genericStack : input) {
if (genericStack == null) {
builder.add(ops.emptyMap());
} else {
builder.add(GenericStack.CODEC.encodeStart(ops, a));
builder.add(innerCodec.encodeStart(ops, genericStack));
}
}

Expand All @@ -46,7 +52,7 @@ public <T> DataResult<T> encode(List<@Nullable GenericStack> input, DynamicOps<T
if (ops.emptyMap().equals(t)) {
elements.add(null);
} else {
DataResult<Pair<GenericStack, T>> element = GenericStack.CODEC.decode(ops, t);
DataResult<Pair<GenericStack, T>> element = innerCodec.decode(ops, t);
element.error().ifPresent(e -> failed.add(t));
result.setValue(result.getValue().apply2stable((r, v) -> {
elements.add(v.getFirst());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,9 @@ public record EncodedProcessingPattern(
}

public static final Codec<EncodedProcessingPattern> CODEC = RecordCodecBuilder.create(builder -> builder.group(
GenericStack.NULLABLE_LIST_CODEC.fieldOf("sparseInputs").forGetter(EncodedProcessingPattern::sparseInputs),
GenericStack.NULLABLE_LIST_CODEC.fieldOf("sparseOutputs")
GenericStack.FAULT_TOLERANT_NULLABLE_LIST_CODEC.fieldOf("sparseInputs")
.forGetter(EncodedProcessingPattern::sparseInputs),
GenericStack.FAULT_TOLERANT_NULLABLE_LIST_CODEC.fieldOf("sparseOutputs")
.forGetter(EncodedProcessingPattern::sparseOutputs))
.apply(builder, EncodedProcessingPattern::new));

Expand Down
6 changes: 6 additions & 0 deletions src/main/java/appeng/items/misc/MissingContentItem.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

import org.jetbrains.annotations.Nullable;

import net.minecraft.ChatFormatting;
import net.minecraft.nbt.Tag;
import net.minecraft.network.chat.Component;
import net.minecraft.resources.ResourceLocation;
Expand Down Expand Up @@ -78,6 +79,11 @@ public BrokenStackInfo getBrokenStackInfo(ItemStack stack) {
@Override
public void appendHoverText(ItemStack stack, TooltipContext context, List<Component> lines, TooltipFlag advanced) {
super.appendHoverText(stack, context, lines, advanced);

var error = stack.get(AEComponents.MISSING_CONTENT_ERROR);
if (error != null) {
lines.add(Component.literal(error).withStyle(ChatFormatting.GRAY));
}
}

public record BrokenStackInfo(Component displayName, @Nullable AEKeyType keyType, long amount) {
Expand Down
7 changes: 5 additions & 2 deletions src/main/java/appeng/me/cells/BasicCellInventory.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@
import java.util.List;
import java.util.Objects;

import org.jetbrains.annotations.Nullable;

import net.minecraft.network.chat.Component;
import net.minecraft.world.item.ItemStack;

Expand Down Expand Up @@ -52,6 +54,7 @@
public class BasicCellInventory implements StorageCell {
private static final int MAX_ITEM_TYPES = 63;

@Nullable
private final ISaveProvider container;
private final AEKeyType keyType;
private final IPartitionList partitionList;
Expand All @@ -66,7 +69,7 @@ public class BasicCellInventory implements StorageCell {
private final boolean hasVoidUpgrade;
private boolean isPersisted = true;

private BasicCellInventory(IBasicCellItem cellType, ItemStack o, ISaveProvider container) {
private BasicCellInventory(IBasicCellItem cellType, ItemStack o, @Nullable ISaveProvider container) {
this.i = o;
this.cellType = cellType;
this.maxItemTypes = this.cellType.getTotalTypes(this.i);
Expand Down Expand Up @@ -137,7 +140,7 @@ public boolean isFuzzy() {
return partitionList instanceof FuzzyPriorityList;
}

public static BasicCellInventory createInventory(ItemStack o, ISaveProvider container) {
public static BasicCellInventory createInventory(ItemStack o, @Nullable ISaveProvider container) {
Objects.requireNonNull(o, "Cannot create cell inventory for null itemstack");

if (!(o.getItem() instanceof IBasicCellItem cellType)) {
Expand Down
10 changes: 10 additions & 0 deletions src/main/java/appeng/util/AECodecs.java
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,16 @@ public <T> DataResult<Pair<ItemStack, T>> apply(DynamicOps<T> ops, T input, Data

@Override
public <T> DataResult<T> coApply(DynamicOps<T> ops, ItemStack input, DataResult<T> t) {
// When the serialization result failed, we write a missing content item instead
// this one will NOT be recoverable
if (t instanceof DataResult.Error<T> error) {
var missingContent = AEItems.MISSING_CONTENT.stack();
LOG.error("Failed to serialize ItemStack {}: {}", input, error.message());
missingContent.set(AEComponents.MISSING_CONTENT_ERROR, error.message());

return ItemStack.SINGLE_ITEM_CODEC.encodeStart(ops, missingContent).setLifecycle(t.lifecycle());
}

// When the input is a MISSING_CONTENT item and has the original data attached,
// we write that back.
if (AEItems.MISSING_CONTENT.isSameAs(input)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ void testNullableListRoundtrip() {
var list = new ArrayList<GenericStack>();
list.add(null);
list.add(new GenericStack(AEItemKey.of(Items.DIAMOND), 1000));
CodecTestUtil.testRoundtrip(GenericStack.NULLABLE_LIST_CODEC, list, JsonOps.INSTANCE, expected);
CodecTestUtil.testRoundtrip(GenericStack.FAULT_TOLERANT_NULLABLE_LIST_CODEC, list, JsonOps.INSTANCE, expected);
}

@Nested
Expand Down
88 changes: 88 additions & 0 deletions src/test/java/appeng/util/AECodecsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.util.Comparator;
Expand All @@ -14,14 +18,19 @@
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;

import net.minecraft.core.RegistryAccess;
import net.minecraft.core.component.DataComponents;
import net.minecraft.nbt.CompoundTag;
import net.minecraft.nbt.ListTag;
import net.minecraft.nbt.NbtOps;
import net.minecraft.world.item.ItemStack;
import net.minecraft.world.item.Items;
import net.minecraft.world.item.component.BundleContents;
import net.minecraft.world.item.component.CustomData;

import appeng.api.ids.AEComponents;
import appeng.api.stacks.AEItemKey;
import appeng.api.stacks.GenericStack;
import appeng.core.definitions.AEItems;

class AECodecsTest {
Expand Down Expand Up @@ -131,4 +140,83 @@ void testReserializationResultsInOriginalBrokenData() {
assertEquals(encodedWithBrokenItem, reencoded);
}
}

@Nested
class FaultTolerantGenericStackCodec {
@Test
void testFaultTolerantSerialization() {
// Construct a stack that will fail Serialization due to exceeding the maximum stack size
var stack = new ItemStack(Items.STICK);
stack.set(DataComponents.BUNDLE_CONTENTS, new BundleContents(List.of(
new ItemStack(Items.STICK, 999))));

// Validate our assumption that this throws
assertThrows(Exception.class, () -> stack.save(RegistryAccess.EMPTY));

// Build a generic stack out of this item
var listInput = List.of(
GenericStack.fromItemStack(AEItems.FORMATION_CORE.stack()),
GenericStack.fromItemStack(stack),
GenericStack.fromItemStack(AEItems.ANNIHILATION_CORE.stack()));
var encodedList = GenericStack.FAULT_TOLERANT_LIST_CODEC.encodeStart(NbtOps.INSTANCE, listInput)
.getOrThrow();

// Now decode again and assert the broken stick was replaced by a missing content item
var stackList = GenericStack.FAULT_TOLERANT_LIST_CODEC.decode(NbtOps.INSTANCE, encodedList)
.getOrThrow().getFirst();

// No item should be missing
assertThat(stackList).hasSize(3);

// The two unaffected items should be untouched
assertThat(stackList.get(0)).isEqualTo(GenericStack.fromItemStack(AEItems.FORMATION_CORE.stack()));
assertThat(stackList.get(2)).isEqualTo(GenericStack.fromItemStack(AEItems.ANNIHILATION_CORE.stack()));

// Assert that the missing content item is as we expect
var brokenStack = stackList.get(1);
assertNotNull(brokenStack);
assertEquals(1, brokenStack.amount());
var resultItemKey = assertInstanceOf(AEItemKey.class, brokenStack.what());
assertSame(AEItems.MISSING_CONTENT.asItem(), resultItemKey.getItem());
assertEquals("Value must be within range [1;99]: 999",
resultItemKey.get(AEComponents.MISSING_CONTENT_ERROR));
}

@Test
void testFaultTolerantDeserialization() {
// Use a known broken NBT to deserialize a generic stack list from by injecting an unknown item id
// Build a generic stack out of this item
var listInput = List.of(
GenericStack.fromItemStack(AEItems.FORMATION_CORE.stack()),
GenericStack.fromItemStack(new ItemStack(Items.DIAMOND)),
GenericStack.fromItemStack(AEItems.ANNIHILATION_CORE.stack()));
var encodedList = GenericStack.FAULT_TOLERANT_LIST_CODEC.encodeStart(NbtOps.INSTANCE, listInput)
.getOrThrow();

// Now break it by replacing diamonds with unknown_item
assertEquals(1, RecursiveTagReplace.replace(encodedList, "minecraft:diamond",
"unknown_mod:does_not_exist"));

// Now decode again and assert the broken stick was replaced by a missing content item
var stackList = GenericStack.FAULT_TOLERANT_LIST_CODEC.decode(NbtOps.INSTANCE, encodedList)
.getOrThrow().getFirst();

// No item should be missing
assertThat(stackList).hasSize(3);

// The two unaffected items should be untouched
assertThat(stackList.get(0)).isEqualTo(GenericStack.fromItemStack(AEItems.FORMATION_CORE.stack()));
assertThat(stackList.get(2)).isEqualTo(GenericStack.fromItemStack(AEItems.ANNIHILATION_CORE.stack()));

// Assert that the missing content item is as we expect
var brokenStack = stackList.get(1);
assertNotNull(brokenStack);
assertEquals(1, brokenStack.amount());
var resultItemKey = assertInstanceOf(AEItemKey.class, brokenStack.what());
assertSame(AEItems.MISSING_CONTENT.asItem(), resultItemKey.getItem());
assertEquals(
"Unknown registry key in ResourceKey[minecraft:root / minecraft:item]: unknown_mod:does_not_exist",
resultItemKey.get(AEComponents.MISSING_CONTENT_ERROR));
}
}
}

0 comments on commit 9f429ac

Please sign in to comment.