hypar-io / Elements

The smallest useful BIM.
https://www.hypar.io
MIT License
347 stars 74 forks source link

Lost GeometricElements during json roundtrip. #1040

Closed DmytroMuravskyi closed 10 months ago

DmytroMuravskyi commented 10 months ago

Describe the bug While working on a function with GeometricEIement with a list of GeometricElement noticed that nested items are not restored after the object is used in the next function of a workflow.

To Reproduce This is simplified code that expresses data loss:

    public class FramePort : GeometricElement
    {
    }

    public class FrameConfiguration : GeometricElement
    {
        public List<FramePort> Ports = new List<FramePort>();
    }

   ...

       [Fact]
        public void NestedObject()
        {
            var model = new Model();
            var frame = new FrameConfiguration();
            frame.Ports.Add(new FramePort() { Name = "Test" });
            model.AddElement(frame);
            var json = model.ToJson();
            var newModel = Model.FromJson(json);
            frame = newModel.AllElementsOfType<FrameConfiguration>().First();
            Assert.Single(frame.Ports);
        }

FramePort serialization is not included in json. Note that if FramePort is no longer GeometricElement - everythin works since it will be included in FrameConfiguration instead of being standalone Element.

Expected behavior Restored object has 1 port inside

Desktop (please complete the following information):

srudenkoamc commented 10 months ago

It seems class fields are not considered during the recursive Elements search in Model.ToJson(). Only class properties are. If Ports field becomes a property, serialization in the test works correctly.

Also FrameConfiguration and FramePort shouldn’t be inner classes, or they will be deserialized as GeometricElement.

wynged commented 10 months ago

ahh, thank you, yea, this is expected behavior, and a good argument for making Schemas of the Classes we are using for Frames @DmytroMuravskyi . Closing this as resolved, thanks for investigating!