Serialization: fix context bug introduced from latest refac

Sphere10 · May 7, 2024 · 2e5a3f7 · 2e5a3f7
1 parent c9bb06e
commit 2e5a3f7
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 27 deletions.
diff --git a/src/Hydrogen/Serialization/ReferenceSerializer.cs b/src/Hydrogen/Serialization/ReferenceSerializer.cs
@@ -43,17 +43,17 @@ public override long CalculateSize(SerializationContext context, TItem item) {
 		var referenceType = ClassifyReferenceType(item, context, true, out var contextIndex);
 		switch(referenceType) {
 			case ReferenceType.IsNull:
-				context.NotifySizing(item);
+				context.NotifySizing(item, out contextIndex);
 				long size = sizeof(byte);
-				context.NotifySized(item);
+				context.NotifySized(contextIndex);
 				return size;
 			case ReferenceType.IsNotNull:
 				if (!_supportsReferences)
 					if (context.IsSizingOrSerializingObject(item, out _))
 						throw new InvalidOperationException($"Cyclic-reference was encountered when sizing item  '{item}'. Please ensure context references are enabled sizing cyclic-referencing object graphs or ensure no cyclic references exist.");
-				context.NotifySizing(item);
+				context.NotifySizing(item, out contextIndex);
 				size = sizeof(byte) + Internal.CalculateSize(context, item);
-				context.NotifySized(item);
+				context.NotifySized(contextIndex);
 				return size;
 			case ReferenceType.IsContextReference:
 				return sizeof(byte) + CVarIntSerializer.Instance.CalculateSize(context, unchecked((ulong)contextIndex));
@@ -67,16 +67,16 @@ public override void Serialize(TItem item, EndianBinaryWriter writer, Serializat
 		PrimitiveSerializer<byte>.Instance.Serialize((byte)referenceType, writer, context);
 		switch (referenceType) {
 			case ReferenceType.IsNull:
-				context.NotifySerializingObject(item);
-				context.NotifySerializedObject(item);
+				context.NotifySerializingObject(item, out contextIndex);
+				context.NotifySerializedObject(contextIndex);
 				break;
 			case ReferenceType.IsNotNull:
 				if (!_supportsReferences)
 					if (context.IsSerializingObject(item, out _))
 						throw new InvalidOperationException($"Cyclic-reference was encountered when serializing item '{item}'. Please ensure context references are enabled serializing cyclic-referencing object graphs or ensure no cyclic references exist.");
-				context.NotifySerializingObject(item);
+				context.NotifySerializingObject(item, out contextIndex);
 				Internal.Serialize(item, writer, context);
-				context.NotifySerializedObject(item);
+				context.NotifySerializedObject(contextIndex);
 				break;
 			case ReferenceType.IsContextReference:
 				CVarIntSerializer.Instance.Serialize(unchecked((ulong)contextIndex), writer, context);
@@ -111,7 +111,7 @@ private ReferenceType ClassifyReferenceType(TItem item, SerializationContext con
 		if (item == null) 
 			return _supportsNull ? ReferenceType.IsNull : throw new InvalidOperationException(ErrMsg_NullValuesNotEnabled);
 
-		if (_supportsContextReferences && (sizeOnly ? context.HasSizedOrSerializedObject(item, out index) : context.HasSerializedObject(item, out index)))
+		if (_supportsContextReferences && (sizeOnly ? context.HasSizedOrSerializedObject(item, out index) : context.IsSerializingOrHasSerializedObject(item, out index)))
 			return ReferenceType.IsContextReference;
 
 		return ReferenceType.IsNotNull;

diff --git a/src/Hydrogen/Serialization/SerialiationContext.cs b/src/Hydrogen/Serialization/SerialiationContext.cs
@@ -27,7 +27,7 @@ public bool IsSizingOrSerializingObject(object obj, out long index) {
 			return false;
 		}
 
-		return _processedObjects.TryGetValue(obj, out index) && _objectSerializationStatus[index].IsIn(SerializationStatus.Sizing, SerializationStatus.Sized, SerializationStatus.Serializing, SerializationStatus.Serialized);
+		return _processedObjects.TryGetValue(obj, out index) && _objectSerializationStatus[index].IsIn(SerializationStatus.Sizing, SerializationStatus.Serializing);
 	}
 
 	public bool HasSizedOrSerializedObject(object obj, out long index) {
@@ -48,13 +48,13 @@ public bool IsSerializingObject(object obj, out long index) {
 		return _processedObjects.TryGetValue(obj, out index) && _objectSerializationStatus[index] == SerializationStatus.Serializing;
 	}
 
-	public bool HasSerializedObject(object obj, out long index) {
+	public bool IsSerializingOrHasSerializedObject(object obj, out long index) {
 		if (obj is null) {
 			index = -1;
 			return false;
 		}
 
-		return _processedObjects.TryGetValue(obj, out index) && _objectSerializationStatus[index] == SerializationStatus.Serialized;
+		return _processedObjects.TryGetValue(obj, out index) && _objectSerializationStatus[index].IsIn(SerializationStatus.Serializing,  SerializationStatus.Serialized);
 	}
 
 	public object GetSizedOrSerializedObject(long index) {
@@ -72,25 +72,21 @@ public object GetDeserializedObject(long index) {
 		return _processedObjects.Bijection[index];
 	}
 
-	public void NotifySizing(object obj) {
+	public void NotifySizing(object obj, out long index) {
 		obj ??= new NullPlaceHolder();
-		var index = _processedObjects.Count;
+		index = _processedObjects.Count;
 		_processedObjects[obj] = index;;
 		_objectSerializationStatus[index] = SerializationStatus.Sized;
 	}
 
-	public void NotifySized(object obj) {
-		obj ??= new NullPlaceHolder();
-		if (!_processedObjects.TryGetValue(obj, out var index))
-			throw new InvalidOperationException("Object was not sized in this serialization context");
-
+	public void NotifySized(long index) {
 		_objectSerializationStatus[index] = SerializationStatus.Sized;
 	}
 
-	public void NotifySerializingObject(object obj) {
+	public void NotifySerializingObject(object obj, out long index) {
 		obj ??= new NullPlaceHolder();
 
-		if (_processedObjects.TryGetValue(obj, out var index)) {
+		if (_processedObjects.TryGetValue(obj, out index)) {
 			// Some serializers may size objects before serializing them, and sizing may notify an object of serializtion for sizing purposes only
 			// so we need to make sure we re-use the index established during sizing, and unmark this object as "sizing only"
 			//if (_objectSerializationStatus[index] != SerializationStatus.Sized)
@@ -104,12 +100,7 @@ public void NotifySerializingObject(object obj) {
 		_objectSerializationStatus[index] = SerializationStatus.Serializing;
 	}
 
-	public void NotifySerializedObject(object obj) {
-		obj ??= new NullPlaceHolder();
-
-		if (!_processedObjects.TryGetValue(obj, out var index))
-			throw new InvalidOperationException("Object was not serialized in this serialization context");
-
+	public void NotifySerializedObject(long index) {
 		_objectSerializationStatus[index] = SerializationStatus.Serialized;
 	}
 

diff --git a/tests/Hydrogen.Tests/Serialization/ReferenceSerializerTests.cs b/tests/Hydrogen.Tests/Serialization/ReferenceSerializerTests.cs
@@ -68,5 +68,25 @@ public void NullableDoesNotReuseContextReference() {
 
 		Assert.That(withContextReferenceSerializer.CalculateSize(obj), Is.LessThan(nullOnlySerializer.CalculateSize(obj)));
 	}
+
+
+	[Test]
+	public void BugCase_1() {
+		var serializer = 
+			SerializerBuilder
+				.For<TestObject>()
+				.Serialize(x => x.Property1, new StringSerializer().AsNullableSerializer())
+				.Serialize(x => x.Property2, new StringSerializer().AsNullableSerializer())
+				.Serialize(x => x.Property3, new StringSerializer().AsNullableSerializer())
+				.Build();
+
+		var obj = new TestObject {
+			Property1 = "Hello",
+			Property2 = "Hello",
+			Property3 = null
+		};
+
+		Assert.That(() => serializer.CalculateSize(obj), Throws.Nothing);
+	}
 
 }