Refactoring of the RevitIdentifiers fragment

[pawelbaran]

Description

Linked to BHoM/BHoM_Adapter#256: ElementId and UniqueId to be renamed to Id and PersistentId, respectively.

Note (edited by @alelom):

The RevitIdentifier fragment will then be able to implement the IPersistentAdapterId interface.

Once the interface is implemented, the Diffing will work automatically for Revit objects imported through the Pull.

References regarding Revit Id naming conventions and usage

https://wrw.is/the-many-ids-of-a-revit-element-and-how-to-work-with-them-elementid-uniqueid-dwf-guid-ifcguid/
https://thebuildingcoder.typepad.com/blog/2009/02/uniqueid-dwf-and-ifc-guid.html
https://thebuildingcoder.typepad.com/blog/2014/04/element-id-export-unique-navisworks-and-other-ids.html
https://thebuildingcoder.typepad.com/blog/2015/02/understanding-the-use-of-the-uniqueid.html

@alelom @al-fisher FYI

[pawelbaran]

Self-confirming the renaming order mentioned above, based on the links and https://forums.autodesk.com/t5/revit-api-forum/unique-id-changing/td-p/5239453

[alelom]

Thanks @pawelbaran .

I had a read of the link you posted, and there was another useful resource (a blog post from Jeremy Tammik) pointed out.
As everything in Revit, things are messy, but luckily they manage to work the way you would expect.
I will post here the explanation of the mechanism for the record.

From http://thebuildingcoder.typepad.com/blog/2009/02/uniqueid-dwf-and-ifc-guid.html:

Every element has such a unique identifier, which is returned through the API as a string. This string is formatted in groups of 8-4-4-4-12-8 hexadecimal characters. It is thus similar to the standard GUID format, but has 8 additional characters at the end. These 8 additional hexadecimal characters are large enough to store 4 bytes or a 32 bit number, which is exactly the size of a Revit element id.
[...]
If I create a couple of walls in a row, I note that the GUID part of the UniqueId is the same for all of them, and the last 8 bytes differ and exactly represent their individual element ids.

This means that in effect, in Revit, the UniqueId is a combination of a GUID (which should be the "Unique part") and a ElementID (non-unique part):
UniqueId = 60f91daf-3dd7-4283-a86d-24137b73f3da-0001fd0b;
the first part is a GUID; the last part (after the last -) is the ElementId.

However, since:

If I create a couple of walls in a row, I note that the GUID part of the UniqueId is the same for all of them, and the last 8 bytes differ and exactly represent their individual element ids.

... somehow Revit manages to create two identical GUIDs "when you create multiple items in a row"!!
Copying over the GUID for different object instances surely isn't wise in any parallel universe.
Anyway, they must have realised this was an obstacle in having a truly "unique" id, so they "patched" the issue by appending an ElementID to the (not-globally-unique) GUID, so with this combination they get

Therefore, Revit must ensure that "when two items are created in a row", their ElementId differs. They are leaving developers and users no other choice other than believing this is always guaranteed. It goes without saying that this is not wise for them, considering that they could have avoided this responsibility by not copying the GUID around. Anyway.

The final conclusion is:
GUID (not globally unique) + ElementId (not globally unique) = UniqueId (the combination of the two is always globally unique)

Well, so it works.

[pawelbaran]

I think that in general you are correct @alelom. Just a bit more explanation of the UniqueId from Jeremy's blog:

The UniqueId 40 is characters long and made up from 2 parts, the first is internally called the EpisodeId and the second is the hexadecimal representation of the element id.

Which means that the first part is episode(project?)-specific GUID, while the second is the identifier of an element within this episode. On top of that, in case of identical Ids created by two users locally, the conflict gets resolved on synchronization.

[pawelbaran]

Following the chat with @alelom, this is going to be resolved in 4.1 as not super urgent to happen in this Milestone.

[alelom]

Hey @pawelbaran , this issue is getting again some relevance as @kayleighhoude is starting some heavy use of Diffing for MEP workflows and data pulled from Revit. You think we could start working on it?

[alelom]

Happy to start again a discussion on this. For example, in this issue description we write:

ElementId and UniqueId to be renamed to Id and PersistentId, respectively.

but we are actually using ElementId as our ID for the diffing workflow (while, in theory, the Id used for the Diffing should be the PersistentId, AKA the UniqueId following the above quote). Are we using the right ID to do the diffing @kayleighhoude and @pawelbaran ?

We need to write down a good reference wiki page on the choice of the Id for the Diffing and why we chose it. Due to my limited knowledge of Revit things are still confusing for me. @kayleighhoude maybe we could have a session with @pawelbaran and write down a stub for it. Have a look at what I wrote back then.

[pawelbaran]

Sure I am up for getting this sorted. Just to sum up all the discussions we had on different occasions:

• UniqueId is the persistent Id in Revit, consisting of the id of the model and id of the element within the model
• ElementId is the nonpersistent one, unique only within a single model

So we have two separate actions to be taken on the RevitIdentifiers fragment:

• implement IPersistentAdapterId interface and rename UniqueId to PersistentId - relevant to diffing and therefore urgent for @kayleighhoude
• implement IAdapterId interface and rename ElementId to Id - relevant to CRUD actions, not urgent because the Revit adapter is handling ids explicitly and it works

I believe the best we can do for now is to tackle the first and then think what to do about the second (I am a bit concerned about renaming ElementId to Id - the former is clear to the users, the other not really). How does that sound @kayleighhoude @alelom?

[FraserGreenroyd]

I would add that if we're renaming any properties using Id they should be renamed to ID as part of this exercise to make the versioning simpler.

[kayleighhoude]

I'm going to veto renaming ElementId, Id. All Revit users are very aware of the "Element ID" property within their Revit models. Having two non-familiar ID names is confusing.

[alelom]

I'm going to veto renaming ElementId, Id. All Revit users are very aware of the "Element ID" property within their Revit models. Having two non-familiar ID names is confusing.

Thanks @kayleighhoude , this is exactly one of the first points we considered, because Revit is something of a special case amongst software, in that it has many IDs that have a precise name and meaning. I think it's valuable to have you in this conversation to find the solution we think is best. I can think of alternatives to the "renaming" solution proposed above. Each solutions comes with a tradeoff.

Solution 1: RevitIdentifiers class to implement IPersistentId (and, less importantly, later on, IAdapterId).

This means that we make the RevitIdentifiers fragment class our "BHoM fragment id official class". All adapters that deal with objects that deal with IDs have (or should have) one of such fragment classes – it is expected by the BHoM paradigm itself: it allows to get the ID of any BHoMObject for any software without having to know how that ID is named in each software.

This however means that we need to "map" the concepts/names of IDs from the external software to what BHoM calls them: the fragment property names must prioritize BHoM names, hence UniqueId becomes PersistentId (and, less of a priority but not irrelevant, ElementId should become Id).

Solution 2: add an additional Revit Identifiers class

We keep the RevitIdentifiers class as it is, but we add another class – RevitId – that is always pulled at the same time the RevitIdentifiers class is pulled. This new fragment class could implement IPersistentId and IAdapterId, ergo holding the two properties Id and PersistentId that map to ElementId and UniqueId.

The disadvantage is that we have two fragments pulled on the same BHoMObject and some information is repeated. However this new fragment could be seen as more of a "system" kind of thing that allows BHoM to do its things, while leaving the users with familiar identifier names in the other class.

Solution 3: review the entire AdapterIds mechanism

Another alternative, more complex and less efficient from the coding and performance perspective but feasible, is to change how we look for AdapterIds in fragments. For example, we could introduce two reflection attributes: AdapterId and PersistentId. So a property of a fragment could be "tagged", like:

[AdapterId("ElementId")]
[PersistentId("UniqueId")]
public class RevitIdentifiers : IObject, IFragment, IImmutable
{
    public virtual string UniqueId { get; } = "";

    public virtual int ElementId { get; } = -1;
    ...

This looks nice, but has the main disadvantage of needing extra attributes (maybe we could get away with only one) to be created for a "system" mechanism. In other words, we shift the "conversion" responsibility from an Engine Convert method to an attribute. Secondarily, it would slow down retrieval of the Ids because we'd need Reflection to get them (although I doubt it would be a significant performance hit in 99% of scenarios); finally, the base Adapter and all existing adapters would need to be refactored.

[alelom]

So after a chat with @pawelbaran @al-fisher and me we decided to implement Solution 1 for now, but only partially - limited to the renaming of UniqueId to PersistentId (and implementing the IPersistentAdapterId interface), keeping ElementId as is.

Now this means that we end up in a "hybrid" situation where the same fragment is used for BHoM concepts and Revit concepts. Therefore, on the long term, we still need to evaluate and implement Solution 2 or 3.