bjorn/cnx 882 send etabsobjects with their properties

[bjoernsteinhagen]

Description & motivation

• Related to CNX-882

Changes:

• Minor architectural / semantic changes: CSi -> Csi and ETABS -> Etabs
• Updated the EtabsObjectToSpeckleConverter to return EtabsObject. For explanation, see Figma section here.

• Created a SharedPropertiesExtractor (common to all Csi products) and an ApplicationPropertiesExtractor (product specific). For explanation, see Figma section here.

• Extracted all properties deemed to be relevant. The function coverage is noted on the Notion page Function Coverage. Included 34 of 83 possible (viable) methods. Remaining methods are either deemed not useful (I've never used them before) or are being blocked until loading is implemented.

To-do before merge:

Discuss:

• Naming conventions and properties returns. Etabs commit can be found here. Areas of concern shown below (link to Figma section here):

• EtabsSendCollectionManager reliant on properties after extraction. Currently indexing properties dict by string (not nice). Better approach? See below:

  private string GetObjectLevelFromObject(Base obj)
  {
    // Properties from converter are stored in "Object ID" dictionary
    // NOTE: Introduce enums for these object keys? I don't like string indexing.
    if (obj["properties"] is not Dictionary<string, object> properties)
    {
      return DEFAULT_LEVEL;
    }

    if (properties.TryGetValue("Object ID", out var objectId) && objectId is Dictionary<string, object> parameters)
    {
      return parameters.TryGetValue("level", out var level) ? level?.ToString() ?? DEFAULT_LEVEL : DEFAULT_LEVEL;
    }

    return DEFAULT_LEVEL;
  }

• CsiJointPropertiesExtractor, CsiFramePropertiesExtractor, CsiShellPropertiesExtractor, EtabsJointPropertiesExtractor, EtabsFramePropertiesExtractor and EtabsShellPropertiesExtractor are a lot of files with similar looking code, but:
  • Example: ...AreaObj.GetGUID and ...PointObj.GetGUID means we can't have a generic extractor. Found it easier to create extractors for each ICsiWrapper.
  • Each file follows a "individual method approach" for each API easier to maintain and since each API call is independent, I don't believe batching will bring much performance improvement (?).

Validation of changes:

Verification for Frame:

Verification for Joint:

Verification for Shell:

Verification of collection structure:

Checklist:

• My pull request follows the guidelines in the Contributing guide?
• My pull request does not duplicate any other open Pull Requests for the same update/change?
• My commits are related to the pull request and do not amend unrelated code or documentation.
• My code follows a similar style to existing code.
• I have added appropriate tests.
• I have updated or added relevant documentation.

References:

• Figma Jam here
• Notion Notes here

[linear]

CNX-882 Send EtabsObjects with their properties

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 9.43%. Comparing base (0e0dd81) to head (66abf71).
Report is 1 commits behind head on dev.

Additional details and impacted files

@@          Coverage Diff          @@
##             dev    #442   +/-   ##
=====================================
  Coverage   9.43%   9.43%           
=====================================
  Files        223     223           
  Lines       4209    4209           
  Branches     487     487           
=====================================
  Hits         397     397           
  Misses      3796    3796           
  Partials      16      16           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[clairekuang]

Notes from our live pr review:

• Rename the GeneralPropertiesExtractor to SharedPropertiesExtractor and add it as a field to the IApplicationPropertiesExtractor. This enforces any concretization of IApplicationPropertiesExtractor to inject the shared properties extractor for each object in the specific applications (etabs, sap, etc).
• Type the output of SharedPropertiesExtractor.Extract() method to be of type PropertyExtractorResult struct, since this contains the application id and shared props dictionary. The shared props dictionary can then be mutated within the application proeprty extractor to add addtional props.

[bjoernsteinhagen]

Notes from our live pr review:

• Rename the GeneralPropertiesExtractor to SharedPropertiesExtractor and add it as a field to the IApplicationPropertiesExtractor. This enforces any concretization of IApplicationPropertiesExtractor to inject the shared properties extractor for each object in the specific applications (etabs, sap, etc).
• Type the output of SharedPropertiesExtractor.Extract() method to be of type PropertyExtractorResult struct, since this contains the application id and shared props dictionary. The shared props dictionary can then be mutated within the application proeprty extractor to add addtional props.

Incorporated, see latest commit :)

[clairekuang]

Looks good! great work :)