Revit element selection nodes should serialize and deserialize properly in Json by alfarok · Pull Request #8131 · DynamoDS/Dynamo

alfarok · 2017-08-25T18:44:22Z

Purpose

The purpose of this PR is to enable the serialization and deserialization of Revit element selection nodes in Json. Previously Revit/Dynamo would freeze when attempting save a graph containing Revit selection nodes and the Json file is never written. The selection node should also maintain the Revit element id that is referenced so it will automatically be recognized when reopening a graph.

Supplemental to D4R PR #1768

Updated Json Output

{
  "Uuid": "ac033b17-4df4-4c56-bc42-533f7bad27fe",
  "IsCustomNode": false,
  "Description": null,
  "Name": "Home",
  "ElementResolver": {
    "ResolutionMap": {}
  },
  "Inputs": [],
  "Nodes": [
    {
      "ConcreteType": "Dynamo.Nodes.SelectFaces, DSRevitNodesUI",
      "NodeType": "ExtensionNode",
      "InstanceId": [
        "3a376239-8db9-4f12-8dbb-774857dd156f-0000097b:5:SURFACE",
        "3a376239-8db9-4f12-8dbb-774857dd156f-0000097b:6:SURFACE",
        "3a376239-8db9-4f12-8dbb-774857dd156f-0000097b:10:SURFACE",
        "3a376239-8db9-4f12-8dbb-774857dd156f-0000097b:18:SURFACE"
      ],
      "Id": "4d90c28040f247ef8cc0a3d0e6eed4d7",
      "Inputs": [],
      "Outputs": [
        {
          "Id": "4953c930ce1040ea828e6f8a94328d8a",
          "Name": "Surfaces",
          "Description": "The selected elements.",
          "Level": 2,
          "UseLevels": false,
          "KeepListStructure": false
        }
      ],
      "Replication": "Disabled"
    }
  ],
  "Connectors": [],
  "Dependencies": [],
  "Bindings": [],
  "View": {
    "Camera": {
      "Name": "Background Preview",
      "EyeX": -17.0,
      "EyeY": 24.0,
      "EyeZ": 50.0,
      "LookX": 12.0,
      "LookY": -13.0,
      "LookZ": -58.0,
      "UpX": 0.0,
      "UpY": 1.0,
      "UpZ": 0.0
    },
    "NodeViews": [
      {
        "ShowGeometry": true,
        "Name": "Select Faces",
        "Id": "4d90c28040f247ef8cc0a3d0e6eed4d7",
        "IsUpstreamVisible": true,
        "Excluded": false,
        "X": 135.60000000000002,
        "Y": 128.79999999999993
      }
    ],
    "Notes": [],
    "Annotations": [],
    "X": 0.0,
    "Y": 0.0,
    "Zoom": 1.0
  }
}

All RTF selection tests are passing including new tests:

Declarations

Check these if you believe they are true

The code base is in a better state after this PR
Is documented according to the standards
The level of testing this PR includes is appropriate
User facing strings, if any, are extracted into *.resx files
All tests pass using the self-service CI.
Snapshot of UI changes, if any.
Changes to the API follow Semantic Versioning, and are documented in the API Changes document.

Reviewers

@mjkkirschner
@QilongTang

FYIs

jnealb · 2017-08-25T18:46:08Z

@alfarok Is there a JIRA task for this?

alfarok · 2017-08-25T18:46:50Z

src/Libraries/CoreNodeModels/Selection.cs

    /// <typeparam name="TSelection">The type which is used to constrain the selection.</typeparam>
    /// <typeparam name="TResult">The type which is returned from the selection or subselection.</typeparam>
+    [DataContract]
    public abstract class SelectionBase<TSelection, TResult> : NodeModel


This is the only way I have been able to prevent the freezing from occurring, even when adding [JsonIgnore] on all public properties. The [DataContract] prevents serialization unless explicitly defined

Not sure if this is the issue or not, but just in case it helps 😄 : where the deserialization is happening, have you tried specifying a JsonSerializerSettings settings object. This is where you declare how to handle nulls or missing properties during deserialization.
I've found explicitly declaring how to handle these clears up this kind of issues.

Also, I've had issues in the past with [JsonIgnore] being ignored itself, had to implement my own attribute.

alfarok · 2017-08-25T18:53:11Z

src/Libraries/CoreNodeModels/Selection.cs

+            IEnumerable<string> selectionIdentifier,
+            IEnumerable<PortModel> inPorts,
+            IEnumerable<PortModel> outPorts) : base(inPorts, outPorts)
+        {


I am also still getting double output ports when deserializing even after adding these parameters. I know this was due to recent changes in the node model base class but not sure why it is still occurring. Maybe an empty constructor should be created at the DSFaceSelection level

yeah try that and make sure NodeModel(IEnumerable inPorts, IEnumerable outPorts) is called

@QilongTang @mjkkirschner I have actually made some progress by adding JsonConstructors through the entire inheritance chain shown in the description making sure to pass all the original parameters plus the revit element id propety data and port data. I am now getting a single output and the Revit referenced element is deserializing properly. The issue now is I can't select a new element after opening the file but I have a few ideas on resolving this.

@alfarok I believe the comment above is no longer valid? Do you have an update pic of the same DYN?

@QilongTang yup this has all been resolved and I verified model elements outside of the standard geometry types are properly restoring as well (wall, roof, etc).

alfarok · 2017-08-25T18:54:45Z

@jnealb yes QNTM-1284 adding it to the description

alfarok · 2017-08-25T22:29:27Z

@mjkkirschner after updating all the references in D4R to point at my local build of Dynamo (opposed to the default NuGet packages) I still find that the base class I am referencing in Dynamo isn't updating in D4R. It seems it is still referencing some old cached metadata? For example, if I were to add a public property to the base class in Dynamo I am unable to access that inherited property in D4R

mjkkirschner · 2017-08-28T04:19:30Z

@alfarok please confirm your json constructor is called at all?

dump all the nuget packages for dynamo from the revit packages folder and save all your csproj files in dynamoRevit after they have been modified to point to your new dlls, not sure how you did this.
force a rebuild and clear all bin folders
If you replaced the dlls but did not change any references you might need to force them to repoint if version numbers changed or something like that.
try unloading all projects in the solution and forcing them to reload (right click project in solution explorer -> unload project, then -> reload project. See if this updates your referenced dll.
verify same build type (release v debug)

mjkkirschner · 2017-08-31T00:31:34Z

src/Libraries/CoreNodeModels/Selection.cs

        private List<TResult> selectionResults = new List<TResult>();
        private List<TSelection> selection = new List<TSelection>();
-        private IEnumerable<string> selectionIdentifier;
+        private List<string> selectionIdentifier;


curious why this needed to be a List?

alfarok · 2017-08-31T00:34:26Z

src/Libraries/CoreNodeModels/Selection.cs

+                {
+                    selectionIdentifier = selection.Select(GetIdentifierFromModelObject).Where(x => x != null).ToList();
+                }
+


A small change is still needed here. I believe this logic prevents the selectionIdentifier from updating once an initial value is stored. It will appear as if the new selection has been updated in Dynamo background preview, however the initial selection id will serialize.

you should write a test for that. Your own fault for bringing it up 😉

haha probably for the best anyway...but does that mean RTF? 😕

alfarok · 2017-08-31T00:44:10Z

src/Libraries/CoreNodeModels/Selection.cs

+            ShouldDisplayPreviewCore = true;
+
+            SelectionIdentifier = selectionIdentifier;
+            ResetSelectionFromIds(SelectionIdentifier);


@mjkkirschner it could be an enum and this line changes to ResetSelectionFromIds(SelectionIdentifier.toList());

alfarok · 2017-08-31T13:14:20Z

Seems to be working as expected now, going to work on some testing to verify further

alfarok · 2017-09-01T15:03:06Z

"Nodes": [
    {
      "ConcreteType": "Dynamo.Nodes.SelectFaces, DSRevitNodesUI",
      "NodeType": "ExtensionNode",
      "InstanceId": [
        "3a376239-8db9-4f12-8dbb-774857dd156f-0000097b:5:SURFACE",
        "3a376239-8db9-4f12-8dbb-774857dd156f-0000097b:6:SURFACE",
        "3a376239-8db9-4f12-8dbb-774857dd156f-0000097b:10:SURFACE",
        "3a376239-8db9-4f12-8dbb-774857dd156f-0000097b:18:SURFACE"
      ],
      "Id": "4d90c28040f247ef8cc0a3d0e6eed4d7",

@gregmarr Hey Greg, I was wondering if there should be another NodeType for SelectionNodes that select elements and return objects? Currently I am serializing as an ExtensionNode and adding a InstanceId property for Revit selection elements.

gregmarr · 2017-09-01T15:59:22Z

Revit bindings should probably go in here, that's what this is designed for:
https://git.autodesk.com/AEC-Generative-Design/ComputationGraphService/blob/master/api/service.yaml#L1462-L1466
https://git.autodesk.com/AEC-Generative-Design/ComputationGraphService/blob/master/api/service.yaml#L1982-L1986

gregmarr · 2017-09-01T16:01:24Z

Does this node do anything else, or is it just a place to store these IDs?

gregmarr · 2017-09-01T16:02:05Z

Ah, this sounds like a geometry input node. We've talked about adding one but haven't yet. https://jira.autodesk.com/browse/AGD-47

Edit: Ah, yes, I see that you edited the original message to say that, I didn't see the update until after my first two responses.

serialize nodeType and instanceIDs for selection nodes

d6e92b7

alfarok added JSON WIP labels Aug 25, 2017

alfarok commented Aug 25, 2017

View reviewed changes

alfarok mentioned this pull request Aug 28, 2017

Revit element selection nodes should serialize and deserialize properly in Json DynamoDS/DynamoRevit#1768

Merged

5 tasks

serialize & deserialize selectionID

6b2f9ee

mjkkirschner reviewed Aug 31, 2017

View reviewed changes

alfarok commented Aug 31, 2017

View reviewed changes

alfarok added 2 commits August 31, 2017 08:38

replace List with IEnumerable

5fb5dad

selectionID must update when selection changed

8b1946e

alfarok changed the title ~~[WIP] Revit element selection nodes should serialize and deserialize properly in Json~~ Revit element selection nodes should serialize and deserialize properly in Json Aug 31, 2017

alfarok added PTAL Please Take A Look 👀 and removed WIP labels Aug 31, 2017

QilongTang added LGTM and removed PTAL Please Take A Look 👀 labels Sep 1, 2017

alfarok merged commit 3b1f6ee into DynamoDS:master Sep 5, 2017

This was referenced Sep 5, 2017

Revit element selection nodes should serialize and deserialize properly in Json DynamoDS/DynamoRevit#1772

Merged

Ensure empty selection nodes serialize/deserialize properly #8166

Merged

Conversation

alfarok commented Aug 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose

Updated Json Output

Declarations

Reviewers

FYIs

Uh oh!

jnealb commented Aug 25, 2017

Uh oh!

alfarok Aug 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

radumg Aug 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alfarok Aug 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alfarok commented Aug 25, 2017

Uh oh!

alfarok commented Aug 25, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mjkkirschner commented Aug 28, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alfarok Aug 31, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alfarok commented Aug 31, 2017

Uh oh!

alfarok commented Sep 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gregmarr commented Sep 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gregmarr commented Sep 1, 2017

Uh oh!

gregmarr commented Sep 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

alfarok commented Aug 25, 2017 •

edited

Loading

alfarok Aug 25, 2017 •

edited

Loading

radumg Aug 27, 2017 •

edited

Loading

alfarok Aug 25, 2017 •

edited

Loading

alfarok commented Aug 25, 2017 •

edited

Loading

mjkkirschner commented Aug 28, 2017 •

edited

Loading

alfarok Aug 31, 2017 •

edited

Loading

alfarok commented Sep 1, 2017 •

edited

Loading

gregmarr commented Sep 1, 2017 •

edited

Loading

gregmarr commented Sep 1, 2017 •

edited

Loading