From 73ed8d658755a82de8c09d1cd7844d82a6aa8af4 Mon Sep 17 00:00:00 2001 From: katehryhorenko Date: Thu, 16 Jan 2025 17:29:52 -0400 Subject: [PATCH 1/4] Added SharedObjects serialization to the model; --- Elements/src/Element.cs | 7 ++ Elements/src/Model.cs | 85 ++++++++++++--- .../JSON/JsonInheritanceConverter.cs | 103 +++++++++++++++++- 3 files changed, 177 insertions(+), 18 deletions(-) diff --git a/Elements/src/Element.cs b/Elements/src/Element.cs index a09c7cfa3..37bb8f835 100644 --- a/Elements/src/Element.cs +++ b/Elements/src/Element.cs @@ -95,6 +95,13 @@ protected virtual void RaisePropertyChanged([System.Runtime.CompilerServices.Cal [JsonProperty("Mappings", Required = Required.Default, NullValueHandling = NullValueHandling.Ignore)] internal Dictionary Mappings { get; set; } = null; + /// + /// An optional shared object that can be used to share data between elements. + /// + /// + [JsonProperty("SharedObject", Required = Required.Default, NullValueHandling = NullValueHandling.Ignore)] + public SharedObject SharedObject { get; set; } + /// /// The method used to set a mapping for a given context. /// diff --git a/Elements/src/Model.cs b/Elements/src/Model.cs index e86af5726..c40985ea9 100644 --- a/Elements/src/Model.cs +++ b/Elements/src/Model.cs @@ -35,6 +35,11 @@ public class Model [System.ComponentModel.DataAnnotations.Required] public System.Collections.Generic.IDictionary Elements { get; set; } = new System.Collections.Generic.Dictionary(); + /// A collection of SharedObjects keyed by their identifiers. + [JsonProperty("SharedObjects", Required = Required.Default)] + [System.ComponentModel.DataAnnotations.Required] + public System.Collections.Generic.IDictionary SharedObjects { get; set; } = new System.Collections.Generic.Dictionary(); + /// /// Collection of subelements from shared objects or RepresentationInstances (e.g. SolidRepresentation.Profile or RepresentationInstance.Material). /// We do not serialize shared objects to json, but we do include them in other formats like gltf. @@ -123,7 +128,7 @@ public void AddElement(Element element, bool gatherSubElements = true, bool upda // to the elements dictionary first. This will ensure that // those elements will be read out and be available before // an attempt is made to deserialize the element itself. - var subElements = RecursiveGatherSubElements(element, out var elementsToIgnore); + var (subElements, sharedObjects) = RecursiveGatherSubElements(element, out var elementsToIgnore); foreach (var e in subElements) { if (!this.Elements.ContainsKey(e.Id)) @@ -138,6 +143,14 @@ public void AddElement(Element element, bool gatherSubElements = true, bool upda } } + foreach (var sharedObject in sharedObjects) + { + if (!SharedObjects.ContainsKey(sharedObject.Id)) + { + SharedObjects.Add(sharedObject.Id, sharedObject); + } + } + foreach (var e in elementsToIgnore) { if (!SubElementsFromSharedObjects.ContainsKey(e.Id)) @@ -453,23 +466,24 @@ public static Model FromJson(string json, bool forceTypeReload = false) return FromJson(json, out _, forceTypeReload); } - private List RecursiveGatherSubElements(object obj, out List elementsToIgnore) + private (List elements, List sharedObjects) RecursiveGatherSubElements(object obj, out List sharedObjectSubElements) { // A dictionary created for the purpose of caching properties // that we need to recurse, for types that we've seen before. var props = new Dictionary>(); - return RecursiveGatherSubElementsInternal(obj, props, out elementsToIgnore); + return RecursiveGatherSubElementsInternal(obj, props, out sharedObjectSubElements); } - private List RecursiveGatherSubElementsInternal(object obj, Dictionary> properties, out List elementsToIgnore) + private (List elements, List sharedObjects) RecursiveGatherSubElementsInternal(object obj, Dictionary> properties, out List sharedObjectSubElements) { var elements = new List(); - elementsToIgnore = new List(); + sharedObjectSubElements = new List(); + var sharedObjects = new List(); if (obj == null) { - return elements; + return (elements, sharedObjects); } var e = obj as Element; @@ -478,7 +492,7 @@ private List RecursiveGatherSubElementsInternal(object obj, Dictionary< // Do nothing. The Element has already // been added. This assumes that that the sub-elements // have been added as well and we don't need to continue. - return elements; + return (elements, sharedObjects); } // This explicit loop is because we have mappings marked as internal so it's elements won't be automatically serialized. @@ -491,6 +505,16 @@ private List RecursiveGatherSubElementsInternal(object obj, Dictionary< } } + var sharedObject = obj as SharedObject; + + if (sharedObject != null) + { + if (SharedObjects.ContainsKey(sharedObject.Id)) + { + return (elements, sharedObjects); + } + } + var t = obj.GetType(); // Ignore value types and strings @@ -498,7 +522,7 @@ private List RecursiveGatherSubElementsInternal(object obj, Dictionary< // could be elements. if (!t.IsClass || t == typeof(string)) { - return elements; + return (elements, sharedObjects); } List constrainedProps; @@ -516,6 +540,7 @@ private List RecursiveGatherSubElementsInternal(object obj, Dictionary< } var elementsFromProperties = new List(); + var sharedObjectsFromProperties = new List(); foreach (var p in constrainedProps) { try @@ -526,12 +551,21 @@ private List RecursiveGatherSubElementsInternal(object obj, Dictionary< continue; } + // Do not save shared object to the model if it is marked with JsonIgnore (e.g. ElementRepresentation) + bool hasJsonIgnore = p.GetCustomAttributes(typeof(JsonIgnoreAttribute), true).Any(); + if (pValue is IList elems) { foreach (var item in elems) { - elementsFromProperties.AddRange(RecursiveGatherSubElementsInternal(item, properties, out var elementsFromItemToIgnore)); - elementsToIgnore.AddRange(elementsFromItemToIgnore); + var (subElements, subSharedObjects) = RecursiveGatherSubElementsInternal(item, properties, out var elementsFromSharedObjectProperties); + elementsFromProperties.AddRange(subElements); + // do not save shared objects marked with JsonIgnore + if (!hasJsonIgnore) + { + sharedObjectsFromProperties.AddRange(subSharedObjects); + } + sharedObjectSubElements.AddRange(elementsFromSharedObjectProperties); } continue; } @@ -541,14 +575,26 @@ private List RecursiveGatherSubElementsInternal(object obj, Dictionary< { foreach (var value in dict.Values) { - elementsFromProperties.AddRange(RecursiveGatherSubElementsInternal(value, properties, out var elementsFromValueToIgnore)); - elementsToIgnore.AddRange(elementsFromValueToIgnore); + var (subElements, subSharedObjects) = RecursiveGatherSubElementsInternal(value, properties, out var elementsFromSharedObjectProperties); + elementsFromProperties.AddRange(subElements); + // do not save shared objects marked with JsonIgnore + if (!hasJsonIgnore) + { + sharedObjectsFromProperties.AddRange(subSharedObjects); + } + sharedObjectSubElements.AddRange(elementsFromSharedObjectProperties); } continue; } - elementsFromProperties.AddRange(RecursiveGatherSubElementsInternal(pValue, properties, out var elementsFromPropertyToIgnore)); - elementsToIgnore.AddRange(elementsFromPropertyToIgnore); + var (gatheredSubElements, gatheredSubSharedObjects) = RecursiveGatherSubElementsInternal(pValue, properties, out var elementsFromPropertyToIgnore); + elementsFromProperties.AddRange(gatheredSubElements); + // do not save shared objects marked with JsonIgnore + if (!hasJsonIgnore) + { + sharedObjectsFromProperties.AddRange(gatheredSubSharedObjects); + } + sharedObjectSubElements.AddRange(elementsFromPropertyToIgnore); } catch (Exception ex) { @@ -557,19 +603,25 @@ private List RecursiveGatherSubElementsInternal(object obj, Dictionary< } if (IsTypeRelatedToSharedObjects(t)) { - elementsToIgnore.AddRange(elementsFromProperties); + sharedObjectSubElements.AddRange(elementsFromProperties); } else { elements.AddRange(elementsFromProperties); } + sharedObjects.AddRange(sharedObjectsFromProperties); if (e != null) { elements.Add(e); } - return elements; + if (sharedObject != null) + { + sharedObjects.Add(sharedObject); + } + + return (elements, sharedObjects); } /// @@ -624,7 +676,6 @@ internal static bool IsValidForRecursiveAddition(Type t) private static bool IsTypeRelatedToSharedObjects(Type t) { - return typeof(SharedObject).IsAssignableFrom(t) || typeof(RepresentationInstance).IsAssignableFrom(t); } diff --git a/Elements/src/Serialization/JSON/JsonInheritanceConverter.cs b/Elements/src/Serialization/JSON/JsonInheritanceConverter.cs index b0de0761e..704249e35 100644 --- a/Elements/src/Serialization/JSON/JsonInheritanceConverter.cs +++ b/Elements/src/Serialization/JSON/JsonInheritanceConverter.cs @@ -50,6 +50,20 @@ public static Dictionary Elements } } + private static Dictionary _sharedObjects = null; + + public static Dictionary SharedObjects + { + get + { + if (_sharedObjects == null) + { + _sharedObjects = new Dictionary(); + } + return _sharedObjects; + } + } + [System.ThreadStatic] private static List _deserializationWarnings; @@ -170,11 +184,17 @@ public override void WriteJson(Newtonsoft.Json.JsonWriter writer, object value, // Operate on all identifiable Elements with a path less than Entities.xxxxx // This will get all properties. - if (value is Element element && !WritingTopLevelElement(writer.Path) && !ElementwiseSerialization) + var element = value as Element; + if (element != null && !WritingTopLevelElement(writer.Path) && !ElementwiseSerialization) { var ident = element; writer.WriteValue(ident.Id); } + else if (value is SharedObject sharedObject && !WritingTopLevelSharedObject(writer.Path) && !ElementwiseSerialization) + { + var ident = sharedObject; + writer.WriteValue(ident.Id); + } else { var jObject = Newtonsoft.Json.Linq.JObject.FromObject(value, serializer); @@ -195,6 +215,13 @@ public override void WriteJson(Newtonsoft.Json.JsonWriter writer, object value, { jObject.AddFirst(new Newtonsoft.Json.Linq.JProperty(_discriminator, discriminatorName)); } + + // Remove properties that are the same as in SharedObject + if (element != null && element.SharedObject != null) + { + RemovePropertiesSameAsInSharedObject(element, jObject); + + } writer.WriteToken(jObject.CreateReader()); } } @@ -204,6 +231,50 @@ public override void WriteJson(Newtonsoft.Json.JsonWriter writer, object value, } } + private void RemovePropertiesSameAsInSharedObject(Element element, JObject jObject) + { + var sharedProperties = element.SharedObject.GetType().GetProperties(BindingFlags.Public | BindingFlags.Instance); + var elementProperties = element.GetType().GetProperties(BindingFlags.Public | BindingFlags.Instance); + + foreach (var property in elementProperties) + { + // Check if property is in SharedObject + var sharedProperty = sharedProperties.FirstOrDefault(p => p.Name == property.Name); + if (sharedProperty != null) + { + var sharedValue = sharedProperty.GetValue(element.SharedObject); + var elementValue = property.GetValue(element); + + // If property value in SharedObject and Element are the same, remove property from jObject + if (Equals(sharedValue, elementValue)) // Compare values + { + jObject.Remove(property.Name); + } + // If property has JsonExtensionDataAttribute (e.g. AdditionalProperties) + // compare each value in the dictionary + else if (Attribute.IsDefined(sharedProperty, typeof(JsonExtensionDataAttribute))) + { + if (sharedProperty.GetValue(element.SharedObject) is IDictionary extraDataFromSharedObject + && property.GetValue(element) is IDictionary extraDataFromElement) + { + foreach (var extraDataFromSharedObjectKey in extraDataFromSharedObject.Keys) + { + if (string.Equals(extraDataFromSharedObjectKey, _discriminator)) + { + continue; + } + if (extraDataFromElement.ContainsKey(extraDataFromSharedObjectKey) && + Equals(extraDataFromSharedObject[extraDataFromSharedObjectKey], extraDataFromElement[extraDataFromSharedObjectKey])) + { + jObject.Remove(extraDataFromSharedObjectKey); + } + } + } + } + } + } + } + private static bool WritingTopLevelElement(string path) { var parts = path.Split('.'); @@ -214,6 +285,16 @@ private static bool WritingTopLevelElement(string path) return false; } + private static bool WritingTopLevelSharedObject(string path) + { + var parts = path.Split('.'); + if (parts.Length == 2 && parts[0] == "SharedObjects" && Guid.TryParse(parts[1], out var _)) + { + return true; + } + return false; + } + private static string GetDiscriminatorName(object value) { var t = value.GetType(); @@ -280,6 +361,17 @@ public override object ReadJson(Newtonsoft.Json.JsonReader reader, System.Type o return Elements[id]; } + if (typeof(SharedObject).IsAssignableFrom(objectType) && !WritingTopLevelSharedObject(reader.Path) && reader.Value != null) + { + var id = Guid.Parse(reader.Value.ToString()); + if (!SharedObjects.ContainsKey(id)) + { + DeserializationWarnings.Add($"SharedObject {id} was not found during deserialization. Check for other deserialization errors."); + return null; + } + return SharedObjects[id]; + } + var jObject = serializer.Deserialize(reader); if (jObject == null) { @@ -322,6 +414,15 @@ public override object ReadJson(Newtonsoft.Json.JsonReader reader, System.Type o } } + if (typeof(SharedObject).IsAssignableFrom(objectType) && WritingTopLevelSharedObject(reader.Path)) + { + var ident = (SharedObject)obj; + if (!SharedObjects.ContainsKey(ident.Id)) + { + SharedObjects.Add(ident.Id, ident); + } + } + return obj; } catch (Exception ex) From 69cbe692faa80a332b7658ddfc99416b7b99c927 Mon Sep 17 00:00:00 2001 From: katehryhorenko Date: Tue, 21 Jan 2025 15:17:01 -0400 Subject: [PATCH 2/4] Fixes from PR comments; --- Elements/src/Model.cs | 120 +++++++++--------- .../JSON/JsonInheritanceConverter.cs | 26 ++-- 2 files changed, 71 insertions(+), 75 deletions(-) diff --git a/Elements/src/Model.cs b/Elements/src/Model.cs index c40985ea9..9c7ddcde2 100644 --- a/Elements/src/Model.cs +++ b/Elements/src/Model.cs @@ -12,7 +12,6 @@ using Elements.Geometry.Solids; using Elements.GeoJSON; using System.IO; -using System.Text; namespace Elements { @@ -21,6 +20,45 @@ namespace Elements /// public class Model { + private class GatherSubElementsResult + { + /// + /// List of elements collected from the object. + /// + public List Elements { get; } = new List(); + /// + /// List of shared objects collected from the object. + /// + public List SharedObjects { get; } = new List(); + /// + /// List of elements collected from the shared object's properties. + /// + /// If shared object is marked as JsonIgnore (e.g. RepresentationInstance), it will not be + /// serialized to JSON, but it's properties will be collected here so they can be used + /// during gltf serialization. + /// + public List ElementsFromSharedObjectProperties { get; } = new List(); + + public void MergeSubResult(GatherSubElementsResult gatherResult, bool hasJsonIgnore, bool isTypeRelatedToSharedObjects) + { + if (isTypeRelatedToSharedObjects) + { + ElementsFromSharedObjectProperties.AddRange(gatherResult.ElementsFromSharedObjectProperties); + } + else + { + Elements.AddRange(gatherResult.Elements); + } + // do not save shared objects marked with JsonIgnore + if (!hasJsonIgnore) + { + SharedObjects.AddRange(gatherResult.SharedObjects); + Elements.AddRange(gatherResult.ElementsFromSharedObjectProperties); + } + ElementsFromSharedObjectProperties.AddRange(gatherResult.ElementsFromSharedObjectProperties); + } + } + /// The origin of the model. [JsonProperty("Origin", NullValueHandling = Newtonsoft.Json.NullValueHandling.Ignore)] [Obsolete("Use Transform instead.")] @@ -37,7 +75,6 @@ public class Model /// A collection of SharedObjects keyed by their identifiers. [JsonProperty("SharedObjects", Required = Required.Default)] - [System.ComponentModel.DataAnnotations.Required] public System.Collections.Generic.IDictionary SharedObjects { get; set; } = new System.Collections.Generic.Dictionary(); /// @@ -128,8 +165,8 @@ public void AddElement(Element element, bool gatherSubElements = true, bool upda // to the elements dictionary first. This will ensure that // those elements will be read out and be available before // an attempt is made to deserialize the element itself. - var (subElements, sharedObjects) = RecursiveGatherSubElements(element, out var elementsToIgnore); - foreach (var e in subElements) + var gatherSubElementsResult = RecursiveGatherSubElements(element); + foreach (var e in gatherSubElementsResult.Elements) { if (!this.Elements.ContainsKey(e.Id)) { @@ -143,7 +180,7 @@ public void AddElement(Element element, bool gatherSubElements = true, bool upda } } - foreach (var sharedObject in sharedObjects) + foreach (var sharedObject in gatherSubElementsResult.SharedObjects) { if (!SharedObjects.ContainsKey(sharedObject.Id)) { @@ -151,7 +188,7 @@ public void AddElement(Element element, bool gatherSubElements = true, bool upda } } - foreach (var e in elementsToIgnore) + foreach (var e in gatherSubElementsResult.ElementsFromSharedObjectProperties) { if (!SubElementsFromSharedObjects.ContainsKey(e.Id)) { @@ -466,24 +503,21 @@ public static Model FromJson(string json, bool forceTypeReload = false) return FromJson(json, out _, forceTypeReload); } - private (List elements, List sharedObjects) RecursiveGatherSubElements(object obj, out List sharedObjectSubElements) + private GatherSubElementsResult RecursiveGatherSubElements(object obj) { // A dictionary created for the purpose of caching properties // that we need to recurse, for types that we've seen before. var props = new Dictionary>(); - - return RecursiveGatherSubElementsInternal(obj, props, out sharedObjectSubElements); + return RecursiveGatherSubElementsInternal(obj, props); } - private (List elements, List sharedObjects) RecursiveGatherSubElementsInternal(object obj, Dictionary> properties, out List sharedObjectSubElements) + private GatherSubElementsResult RecursiveGatherSubElementsInternal(object obj, Dictionary> properties) { - var elements = new List(); - sharedObjectSubElements = new List(); - var sharedObjects = new List(); + GatherSubElementsResult result = new GatherSubElementsResult(); if (obj == null) { - return (elements, sharedObjects); + return result; } var e = obj as Element; @@ -492,7 +526,7 @@ public static Model FromJson(string json, bool forceTypeReload = false) // Do nothing. The Element has already // been added. This assumes that that the sub-elements // have been added as well and we don't need to continue. - return (elements, sharedObjects); + return result; } // This explicit loop is because we have mappings marked as internal so it's elements won't be automatically serialized. @@ -501,17 +535,17 @@ public static Model FromJson(string json, bool forceTypeReload = false) foreach (var map in e.Mappings ?? new Dictionary()) { if (!Elements.ContainsKey(map.Value.Id)) - { elements.Add(map.Value); } + { result.Elements.Add(map.Value); } } } var sharedObject = obj as SharedObject; - + // if this shared object is already in the list, we don't need to process and add it again if (sharedObject != null) { if (SharedObjects.ContainsKey(sharedObject.Id)) { - return (elements, sharedObjects); + return result; } } @@ -522,7 +556,7 @@ public static Model FromJson(string json, bool forceTypeReload = false) // could be elements. if (!t.IsClass || t == typeof(string)) { - return (elements, sharedObjects); + return result; } List constrainedProps; @@ -539,8 +573,7 @@ public static Model FromJson(string json, bool forceTypeReload = false) properties.Add(t, constrainedProps); } - var elementsFromProperties = new List(); - var sharedObjectsFromProperties = new List(); + bool isTypeRelatedToSharedObjects = IsTypeRelatedToSharedObjects(t); foreach (var p in constrainedProps) { try @@ -558,14 +591,8 @@ public static Model FromJson(string json, bool forceTypeReload = false) { foreach (var item in elems) { - var (subElements, subSharedObjects) = RecursiveGatherSubElementsInternal(item, properties, out var elementsFromSharedObjectProperties); - elementsFromProperties.AddRange(subElements); - // do not save shared objects marked with JsonIgnore - if (!hasJsonIgnore) - { - sharedObjectsFromProperties.AddRange(subSharedObjects); - } - sharedObjectSubElements.AddRange(elementsFromSharedObjectProperties); + var subElements = RecursiveGatherSubElementsInternal(item, properties); + result.MergeSubResult(subElements, hasJsonIgnore, isTypeRelatedToSharedObjects); } continue; } @@ -575,53 +602,32 @@ public static Model FromJson(string json, bool forceTypeReload = false) { foreach (var value in dict.Values) { - var (subElements, subSharedObjects) = RecursiveGatherSubElementsInternal(value, properties, out var elementsFromSharedObjectProperties); - elementsFromProperties.AddRange(subElements); - // do not save shared objects marked with JsonIgnore - if (!hasJsonIgnore) - { - sharedObjectsFromProperties.AddRange(subSharedObjects); - } - sharedObjectSubElements.AddRange(elementsFromSharedObjectProperties); + var subElements = RecursiveGatherSubElementsInternal(value, properties); + result.MergeSubResult(subElements, hasJsonIgnore, isTypeRelatedToSharedObjects); } continue; } - var (gatheredSubElements, gatheredSubSharedObjects) = RecursiveGatherSubElementsInternal(pValue, properties, out var elementsFromPropertyToIgnore); - elementsFromProperties.AddRange(gatheredSubElements); - // do not save shared objects marked with JsonIgnore - if (!hasJsonIgnore) - { - sharedObjectsFromProperties.AddRange(gatheredSubSharedObjects); - } - sharedObjectSubElements.AddRange(elementsFromPropertyToIgnore); + var gatheredSubElements = RecursiveGatherSubElementsInternal(pValue, properties); + result.MergeSubResult(gatheredSubElements, hasJsonIgnore, isTypeRelatedToSharedObjects); } catch (Exception ex) { throw new Exception($"The {p.Name} property or one of its children was not valid for introspection. Check the inner exception for details.", ex); } } - if (IsTypeRelatedToSharedObjects(t)) - { - sharedObjectSubElements.AddRange(elementsFromProperties); - } - else - { - elements.AddRange(elementsFromProperties); - } - sharedObjects.AddRange(sharedObjectsFromProperties); if (e != null) { - elements.Add(e); + result.Elements.Add(e); } if (sharedObject != null) { - sharedObjects.Add(sharedObject); + result.SharedObjects.Add(sharedObject); } - return (elements, sharedObjects); + return result; } /// diff --git a/Elements/src/Serialization/JSON/JsonInheritanceConverter.cs b/Elements/src/Serialization/JSON/JsonInheritanceConverter.cs index 704249e35..18dcf88e9 100644 --- a/Elements/src/Serialization/JSON/JsonInheritanceConverter.cs +++ b/Elements/src/Serialization/JSON/JsonInheritanceConverter.cs @@ -185,12 +185,12 @@ public override void WriteJson(Newtonsoft.Json.JsonWriter writer, object value, // Operate on all identifiable Elements with a path less than Entities.xxxxx // This will get all properties. var element = value as Element; - if (element != null && !WritingTopLevelElement(writer.Path) && !ElementwiseSerialization) + if (element != null && !PathIsTopLevel(writer.Path, "Elements") && !ElementwiseSerialization) { var ident = element; writer.WriteValue(ident.Id); } - else if (value is SharedObject sharedObject && !WritingTopLevelSharedObject(writer.Path) && !ElementwiseSerialization) + else if (value is SharedObject sharedObject && !PathIsTopLevel(writer.Path, "SharedObjects") && !ElementwiseSerialization) { var ident = sharedObject; writer.WriteValue(ident.Id); @@ -275,20 +275,10 @@ private void RemovePropertiesSameAsInSharedObject(Element element, JObject jObje } } - private static bool WritingTopLevelElement(string path) + private static bool PathIsTopLevel(string path, string propertyName) { var parts = path.Split('.'); - if (parts.Length == 2 && parts[0] == "Elements" && Guid.TryParse(parts[1], out var _)) - { - return true; - } - return false; - } - - private static bool WritingTopLevelSharedObject(string path) - { - var parts = path.Split('.'); - if (parts.Length == 2 && parts[0] == "SharedObjects" && Guid.TryParse(parts[1], out var _)) + if (parts.Length == 2 && parts[0] == propertyName && Guid.TryParse(parts[1], out var _)) { return true; } @@ -350,7 +340,7 @@ public override object ReadJson(Newtonsoft.Json.JsonReader reader, System.Type o { // The serialized value is an identifier, so the expectation is // that the element with that id has already been deserialized. - if (typeof(Element).IsAssignableFrom(objectType) && !WritingTopLevelElement(reader.Path) && reader.Value != null) + if (typeof(Element).IsAssignableFrom(objectType) && !PathIsTopLevel(reader.Path, "Elements") && reader.Value != null) { var id = Guid.Parse(reader.Value.ToString()); if (!Elements.ContainsKey(id)) @@ -361,7 +351,7 @@ public override object ReadJson(Newtonsoft.Json.JsonReader reader, System.Type o return Elements[id]; } - if (typeof(SharedObject).IsAssignableFrom(objectType) && !WritingTopLevelSharedObject(reader.Path) && reader.Value != null) + if (typeof(SharedObject).IsAssignableFrom(objectType) && !PathIsTopLevel(reader.Path, "SharedObjects") && reader.Value != null) { var id = Guid.Parse(reader.Value.ToString()); if (!SharedObjects.ContainsKey(id)) @@ -405,7 +395,7 @@ public override object ReadJson(Newtonsoft.Json.JsonReader reader, System.Type o // Write the id to the cache so that we can retrieve it next time // instead of de-serializing it again. - if (typeof(Element).IsAssignableFrom(objectType) && WritingTopLevelElement(reader.Path)) + if (typeof(Element).IsAssignableFrom(objectType) && PathIsTopLevel(reader.Path, "Elements")) { var ident = (Element)obj; if (!Elements.ContainsKey(ident.Id)) @@ -414,7 +404,7 @@ public override object ReadJson(Newtonsoft.Json.JsonReader reader, System.Type o } } - if (typeof(SharedObject).IsAssignableFrom(objectType) && WritingTopLevelSharedObject(reader.Path)) + if (typeof(SharedObject).IsAssignableFrom(objectType) && PathIsTopLevel(reader.Path, "SharedObjects")) { var ident = (SharedObject)obj; if (!SharedObjects.ContainsKey(ident.Id)) From 9dba881ef132b7cab70ef484329c2dd9f8adb618 Mon Sep 17 00:00:00 2001 From: katehryhorenko Date: Wed, 22 Jan 2025 10:27:31 -0400 Subject: [PATCH 3/4] Small typo + use sharedValue and elementValue instead of getting value again --- Elements/src/Model.cs | 2 +- Elements/src/Serialization/JSON/JsonInheritanceConverter.cs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Elements/src/Model.cs b/Elements/src/Model.cs index 9c7ddcde2..01f17f9e4 100644 --- a/Elements/src/Model.cs +++ b/Elements/src/Model.cs @@ -34,7 +34,7 @@ private class GatherSubElementsResult /// List of elements collected from the shared object's properties. /// /// If shared object is marked as JsonIgnore (e.g. RepresentationInstance), it will not be - /// serialized to JSON, but it's properties will be collected here so they can be used + /// serialized to JSON, but its properties will be collected here so they can be used /// during gltf serialization. /// public List ElementsFromSharedObjectProperties { get; } = new List(); diff --git a/Elements/src/Serialization/JSON/JsonInheritanceConverter.cs b/Elements/src/Serialization/JSON/JsonInheritanceConverter.cs index 18dcf88e9..1113d8987 100644 --- a/Elements/src/Serialization/JSON/JsonInheritanceConverter.cs +++ b/Elements/src/Serialization/JSON/JsonInheritanceConverter.cs @@ -254,8 +254,8 @@ private void RemovePropertiesSameAsInSharedObject(Element element, JObject jObje // compare each value in the dictionary else if (Attribute.IsDefined(sharedProperty, typeof(JsonExtensionDataAttribute))) { - if (sharedProperty.GetValue(element.SharedObject) is IDictionary extraDataFromSharedObject - && property.GetValue(element) is IDictionary extraDataFromElement) + if (sharedValue is IDictionary extraDataFromSharedObject + && elementValue is IDictionary extraDataFromElement) { foreach (var extraDataFromSharedObjectKey in extraDataFromSharedObject.Keys) { From 5ed2a7264b0cc300d64d5f6f910ca6b256f4f41d Mon Sep 17 00:00:00 2001 From: katehryhorenko Date: Wed, 22 Jan 2025 11:52:52 -0400 Subject: [PATCH 4/4] Three tests to make sure SharedObjects are serialized and properties from shared onjects are deleted from elements --- Elements/test/ModelTests.cs | 158 ++++++++++++++++++++++++++++++++++++ 1 file changed, 158 insertions(+) diff --git a/Elements/test/ModelTests.cs b/Elements/test/ModelTests.cs index 703156312..b312546c2 100644 --- a/Elements/test/ModelTests.cs +++ b/Elements/test/ModelTests.cs @@ -19,6 +19,15 @@ public class TestMapping : MappingBase public string MapProp = "test"; } + class ColumnSharedObject : SharedObject + { + public string Name { get; set; } + public Material Material { get; set; } + + [JsonExtensionData] + public Dictionary AdditionalProperties { get; set; } = new Dictionary(); + } + public class ModelTests { private ITestOutputHelper _output; @@ -424,5 +433,154 @@ private Model QuadPanelModel() model.AddElement(panel); return model; } + + [Fact] + public void SharedObjectsAreSerialized() + { + var model = new Model(); + + var column1 = new Column(new Vector3(5, 5, 5), 2.0, null, Polygon.Rectangle(1, 1)); + string commonPropertyName = "CommonProperty"; + string commonPropertyValue = "CommonValue"; + column1.AdditionalProperties.Add(commonPropertyName, commonPropertyValue); + + var column2 = new Column(new Vector3(5, 5, 5), 2.0, null, Polygon.Rectangle(1, 1)); + column2.AdditionalProperties.Add(commonPropertyName, commonPropertyValue); + + var column1SharedObject = new ColumnSharedObject + { + AdditionalProperties = new Dictionary() + { + { commonPropertyName, commonPropertyValue }, + }, + }; + + string sharedObjectId = column1SharedObject.Id.ToString(); + column1.SharedObject = column1SharedObject; + column2.SharedObject = column1SharedObject; + model.AddElement(column1); + model.AddElement(column2); + + var json = model.ToJson(); + // one shared object is serialized + var savedSharedObjects = System.Text.RegularExpressions.Regex.Matches(json, @"""discriminator"":\s*""Elements.Tests.ColumnSharedObject"""); + Assert.Single(savedSharedObjects); + + // shared object is added as Id to elements + var sharedObjectUsage = System.Text.RegularExpressions.Regex.Matches(json, $@"""SharedObject"":\s*""{sharedObjectId}"""); + Assert.Equal(2, sharedObjectUsage.Count); + } + + [Fact] + public void ElementPropertiesEqualToSharedObjectPropertiesNotSerialized() + { + var model = new Model(); + var red = new Material("Red", Colors.Red); + string columnName = "Column Test"; + + var column1 = new Column(new Vector3(5, 5, 5), 2.0, null, Polygon.Rectangle(1, 1)); + column1.Material = red; + string commonPropertyName = "CommonProperty"; + string commonPropertyValue = "CommonValue"; + column1.AdditionalProperties.Add(commonPropertyName, commonPropertyValue); + column1.Name = columnName; + + var column2 = new Column(new Vector3(5, 5, 5), 2.0, null, Polygon.Rectangle(1, 1)); + column2.Material = red; + column2.AdditionalProperties.Add(commonPropertyName, commonPropertyValue); + column2.Name = columnName; + + var column1SharedObject = new ColumnSharedObject + { + AdditionalProperties = new Dictionary() + { + { commonPropertyName, commonPropertyValue }, + }, + Name = columnName, + Material = red + }; + + column1.SharedObject = column1SharedObject; + column2.SharedObject = column1SharedObject; + model.AddElement(column1); + model.AddElement(column2); + model.AddElement(red); + + string materialId = red.Id.ToString(); + + var json = model.ToJson(); + // Material was saved to shared object + var sharedMaterial = System.Text.RegularExpressions.Regex.Matches(json, $@"""Material"":\s*""{materialId}"""); + Assert.Single(sharedMaterial); + // material was deleted from elements + sharedMaterial = System.Text.RegularExpressions.Regex.Matches(json, $@"""Material"":\s*"); + Assert.Single(sharedMaterial); + + // common property from AdditionalProperties was saved to shared object once + var commonProperty = System.Text.RegularExpressions.Regex.Matches(json, $@"""{commonPropertyName}"":\s*""{commonPropertyValue}"""); + Assert.Single(commonProperty); + // common property was deleted from elements + commonProperty = System.Text.RegularExpressions.Regex.Matches(json, $@"""{commonPropertyName}"":\s*"); + Assert.Single(commonProperty); + + // name property was saved to shared object and deleted from elements + var nameProperty = System.Text.RegularExpressions.Regex.Matches(json, $@"""Name"":\s*""{columnName}"""); + Assert.Single(nameProperty); + } + + [Fact] + public void ElementPropertiesNotEqualToSharedObjectPropertiesSerializedToELements() + { + var model = new Model(); + var red = new Material("Red", Colors.Red); + string columnName = "Column Test"; + + var column1 = new Column(new Vector3(5, 5, 5), 2.0, null, Polygon.Rectangle(1, 1)); + column1.Material = red; + string commonPropertyName = "CommonProperty"; + string commonPropertyValue = "CommonValue"; + column1.AdditionalProperties.Add(commonPropertyName, commonPropertyValue); + column1.AdditionalProperties.Add("UniqueProperty", "UniqueValue"); + column1.Name = columnName; + + var column2 = new Column(new Vector3(5, 5, 5), 2.0, null, Polygon.Rectangle(1, 1)); + column2.Material = red; + column2.AdditionalProperties.Add(commonPropertyName, commonPropertyValue); + column2.AdditionalProperties.Add("UniqueProperty", "UniqueValue"); + column2.Name = columnName; + + var column3 = new Column(new Vector3(5, 5, 5), 2.0, null, Polygon.Rectangle(1, 1)); + column3.Material = red; + column3.AdditionalProperties.Add(commonPropertyName, "DifferentValue"); + column3.Name = columnName; + + var column1SharedObject = new ColumnSharedObject + { + AdditionalProperties = new Dictionary() + { + { commonPropertyName, commonPropertyValue }, + }, + Name = columnName, + Material = red + }; + + column1.SharedObject = column1SharedObject; + column2.SharedObject = column1SharedObject; + model.AddElement(column1); + model.AddElement(column2); + model.AddElement(column3); + model.AddElement(red); + + string materialId = red.Id.ToString(); + + var json = model.ToJson(); + // Values not from shared object are serialized to both elements + var uniqueProperties = System.Text.RegularExpressions.Regex.Matches(json, $@"""UniqueProperty"":\s*""UniqueValue"""); + Assert.Equal(2, uniqueProperties.Count); + + // common property from AdditionalProperties was saved to shared object and deleted from elements + var commonProperty = System.Text.RegularExpressions.Regex.Matches(json, $@"""{commonPropertyName}"":\s*"); + Assert.Equal(2, commonProperty.Count); + } } }