BIMserver icon indicating copy to clipboard operation
BIMserver copied to clipboard

Model comparers: How to catch updates and deletions

Open dkurillo opened this issue 3 years ago • 13 comments

I checked how standard model comparers work. It seems that they catch only new objects and updates in attributes. I tried the following:

  1. Changed direction for LocalPlacement of some object (just moved this object)
  2. Created brand new LocalPlacement for that object

Comparers didn't catch any modifications between models (revisions)...

I also tried to remove an object. Deletions are also not catched....

What is the proper way to change model so that comparers could see the difference?

dkurillo avatar Nov 10 '21 11:11 dkurillo

I assume you are using one of the ModelCompare plugins from the IfcPlugins bundle? Be aware that those are made to demonstrate how to implement model compare functionality in a respective plugin, as noted here and here. You can study the source in the IfcPlugins repository, there are two compare plugins in the bundle: NameBasedModelCompare and GuidBasedModelCompare. NameBasedModelCompare includes only elements with a "name" attribute, while GuidBasedModelCompare compares only elements with a GUID. Thus changes in LocalPlacement will not be captured by any of those two. When you roll your own, you are not bound to these restrictions, of course.

hlg avatar Nov 10 '21 12:11 hlg

I rolled my own comparer but it's not working. Seems to be some problems in getting models with different revisions to run comparison on server side in CompareDatabaseAction.

For testing purposes I removed two elements from the model and tried to run comparison between two revisions. The result depends on order: if I compare rev1 and rev2 two models that are loaded by CompareDatabaseAction are the same and comparer can't see the difference. If I compare rev2 with rev1 CompareDatabaseAction throws exception that removed element is not found.

On the other hand if I add two elements to the model comparison goes well and I get those two added elements no matter in which order I compare rev1 and rev2.

If I set new IfcLocalPlacement to some element and submit it to the model, CompareDatabaseAction again gets absolutely identical models for comparison and any comparer won't find the difference.

There something wrong with this code:

IfcModelInterface model1 = new DownloadDatabaseAction(bimServer, getDatabaseSession(), getAccessMethod(), roid1, -1, serializerOid, authorization).execute();
IfcModelInterface model2 = new DownloadDatabaseAction(bimServer, getDatabaseSession(), getAccessMethod(), roid2, -1, serializerOid, authorization).execute();

It actually doesn't return valid models corresponding to given roids in case I update or delete exisitng elements. And it's pretty hard to guess what's the problem. The code is a bit complicated.

Or maybe it is expected behaviour and I can't see the idea. It seems like deleted elements are not actually deleted. They loose uuid but if I query deleted element by oid I get it with empty fields. And there is no sign that it is removed except that uuid is null. If I download IFC-file of given revision there is no deleted element. If I query this revision programatically I get the deleted element. Apparantly there are some side effects from previous revisions...

dkurillo avatar Nov 12 '21 06:11 dkurillo

To overcome this issue I download models to the client and pass ClientIfcModel to comparer code. In this case it works well except that I have to check deleted elements by checking uuid for null...

dkurillo avatar Nov 12 '21 06:11 dkurillo

Just for future reference, I guess you are talking about these two lines: https://github.com/opensourceBIM/BIMserver/blob/9c9d4748eef697b527604dc42140c43ce17c758a/BimServer/src/org/bimserver/database/actions/CompareDatabaseAction.java#L92-L93

When you say "I query deleted element by oid", do you mean you call IfcModelInterface.get(long oid) in your ModelCompare.compare implementation? Indeed, this should return null when no object with this oid exists in the model. Then I guess IfcModelInterface.contains(long oid) also returns true instead of false? And this is the same for both ClientIfcModel and ServerIfcModel, right? So you could use the workaround with checking uuid for null also in the Model.compare implementation? If your client downloads the full model anyway, it does not make much of a difference, otherwise it may be beneficial to do it in the plugin on the server side.

Are you adding and deleting through the low level interface? If so, it may rather be an issue with the low level delete method and I will have a look at those methods. Unfortunately the low level interface is not well-tested at the moment and deserves some attention. One more note, but you are probably aware of this: You can't rely on OIDs for comparison. They may be the same when you only have low level modifications, but not when you upload an identical model to a new revision.

hlg avatar Nov 12 '21 07:11 hlg

Can you share your compare code and the exception that is thrown when running it? It is hard to tell what is going wrong without this.

To be on the same page, let me explain what I would expect from model comparison. Comparison always depends on order. Let's say we have rev1, we add obj1 and delete obj2 to obtain rev2.

  • When I compare (rev1, rev2) then the result should report obj1 as added and obj2 as deleted, remaining objects unchanged.
  • When I compare (rev2, rev1) then the result shoudl report obj1 as deleted and obj2 as added, remaining objects unchanged.

hlg avatar Nov 12 '21 08:11 hlg

When you say "I query deleted element by oid", do you mean you call IfcModelInterface.get(long oid) in your ModelCompare.compare implementation? Indeed, this should return null when no object with this oid exists in the model. Then I guess IfcModelInterface.contains(long oid) also returns true instead of false? And this is the same for both ClientIfcModel and ServerIfcModel, right? So you could use the workaround with checking uuid for null also in the Model.compare implementation? If your client downloads the full model anyway, it does not make much of a difference, otherwise it may be beneficial to do it in the plugin on the server side.

I checked the behaviour. So initially I have a model with multiple revisions and want to compare two revisions. Second revision is generated by calling low-level delete method for some element.

For the server side (ServerIfcModel):

// roid1 = 917507
// roid2 = 983043
IfcModelInterface model1 = new DownloadDatabaseAction(bimServer, getDatabaseSession(), getAccessMethod(), roid1, -1, serializerOid, authorization).execute();
IfcModelInterface model2 = new DownloadDatabaseAction(bimServer, getDatabaseSession(), getAccessMethod(), roid2, -1, serializerOid, authorization).execute();

IdEObject o1 = model1.get(328622L); // returned org.bimserver.models.ifc4.impl.IfcPipeSegmentImpl@746f4e4e
IdEObject o2 = model2.get(328622L); // returned org.bimserver.models.ifc4.impl.IfcPipeSegmentImpl@746f4e4e
boolean c1 = model1.contains(328622L); // returned true
boolean c2 = model2.contains(328622L); // returned true
IdEObject obj1 = model1.getByGuid("2Mtf2k4znFIxE3_6dUfhp4"); // returned org.bimserver.models.ifc4.impl.IfcPipeSegmentImpl@746f4e4e
IdEObject obj2 = model2.getByGuid("2Mtf2k4znFIxE3_6dUfhp4"); // returned org.bimserver.models.ifc4.impl.IfcPipeSegmentImpl@746f4e4e

For the client side (ClientIfcModel):

// rev3Id = 917507
// rev4Id = 983043
IfcModelInterface rev3Model = client.getModel(project, rev3Id, true, false);
IfcModelInterface rev4Model = client.getModel(project, rev4Id, true, false);

IdEObject o1 = rev3Model.get(328622L); // returned org.bimserver.models.ifc4.impl.IfcPipeSegmentImpl@62099f75
IdEObject o2 = rev4Model.get(328622L); // returned org.bimserver.models.ifc4.impl.IfcPipeSegmentImpl@4ae836cf
boolean c1 = rev3Model.contains(328622L); // returned true
boolean c2 = rev4Model.contains(328622L); // returned true
IdEObject object1 = rev3Model.getByGuid("2Mtf2k4znFIxE3_6dUfhp4"); // returned org.bimserver.models.ifc4.impl.IfcPipeSegmentImpl@62099f75
IdEObject object2 = rev4Model.getByGuid("2Mtf2k4znFIxE3_6dUfhp4"); // returned null

Check that ServerIfcModel returns the same pointer to the object regardless of passed roid. It means that comparers will never work on updates and deletes

If I swap model2 and model1 on the server side (or swap roid1 and roid2 on the client when I call server compare method):

IfcModelInterface model2 = new DownloadDatabaseAction(bimServer, getDatabaseSession(), getAccessMethod(), roid2, -1, serializerOid, authorization).execute();
IfcModelInterface model1 = new DownloadDatabaseAction(bimServer, getDatabaseSession(), getAccessMethod(), roid1, -1, serializerOid, authorization).execute();

I get an exception:

Caused by: org.bimserver.BimserverDatabaseException: Object not found: 9 4 328622 IfcPipeSegment
	at org.bimserver.database.DatabaseSession.processTodoList(DatabaseSession.java:1352)
	at org.bimserver.database.DatabaseSession.getMap(DatabaseSession.java:1304)
	at org.bimserver.database.actions.DownloadDatabaseAction.execute(DownloadDatabaseAction.java:117)
	at org.bimserver.database.actions.CompareDatabaseAction.execute(CompareDatabaseAction.java:99)
	at org.bimserver.database.actions.CompareDatabaseAction.execute(CompareDatabaseAction.java:46)
	at org.bimserver.database.DatabaseSession.executeAndCommitAction(DatabaseSession.java:775)
	at org.bimserver.database.DatabaseSession.executeAndCommitAction(DatabaseSession.java:767)
	at org.bimserver.webservices.impl.ServiceImpl.compare(ServiceImpl.java:1776)

dkurillo avatar Nov 12 '21 09:11 dkurillo

I fixed comparison for updates by adding this code on the server side (CompareDatabaseAction.java):

DatabaseSession session1 = bimServer.getDatabase().createSession(OperationType.READ_ONLY); // added this line
DatabaseSession session2 = bimServer.getDatabase().createSession(OperationType.READ_ONLY); // added this line
IfcModelInterface model1 = new DownloadDatabaseAction(bimServer, session1, getAccessMethod(), roid1, -1, serializerOid, authorization).execute();
IfcModelInterface model2 = new DownloadDatabaseAction(bimServer, session2, getAccessMethod(), roid2, -1, serializerOid, authorization).execute();

The reason for the identical pointers was probably the same database session. But for deletes I still get an exception. It seems that low-level delete is actually a little buggy. But this is also can be worked around. Instead of throwing aforementioned exception I can swallow it and continue. In this case deleted objects are also catched by comparer.

dkurillo avatar Nov 12 '21 10:11 dkurillo

Ok, I see. Thanks for digging. Even though ServerIfcModels are created and kept in the database session, I don't immediately see why it wouldn't work in one session, but apparently it does not.

But for deletes I still get an exception.

Still same as above? I will try to reproduce the issue.

hlg avatar Nov 12 '21 13:11 hlg

Still same as above? I will try to reproduce the issue.

Yes, the same exception

dkurillo avatar Nov 13 '21 04:11 dkurillo

The BimserverDatabaseException (Object not found) stems most likely from inconsistencies in the model introduced through low level manipulation. With the low level calls, you are responsible to keep the model in consistent state. For example upon deletion you must also remove/unset all references to the deleted object, including references via inverse attributes. If there are dangling references that still point to the deleted object, the DownloadDatabaseAction will try to include those deleted objects into the server model and throw the exception. For the same reason, the client model may have the rudimentary objects included, probably. I was just digging through an example and stepped into the same pitfall.

For the other issues, whether it is necessary to have two database sessions or can be fixed in another way, I still have to verify.

hlg avatar Nov 17 '21 18:11 hlg

The database session holds an object cache by oid. When reading objects from the database, it searches for objects of a type by revision from latest backwards. Once it finds an object with equal or smaller revision than the revision searched for, it looks up the cache first and only then reads the value from the database when the oid is not yet cached. Thus if the second model has changed or deleted objects (value -1) it will still use the cache. If the second model has additional objects, they are not in the cache and go into comparison as expected.

As an alternative to using a second database session, we could make the database session able to handle multiple revisions of the same project. This could be done in two different ways:

a) change the cache to a map by oid and rid (not oid only) or b) compare cached object rid with key rid before using and passing the cached object.

However, I am not sure about the impact on memory (a) and runtime (b) for other database actions. Thus I lean towards your idea with a second database session for now, as long as there are no other places with similar issues. Of course, for comparing models changed with low level calls, a single cache and unchanged objects only instantiated once would be more efficient. But for now, low level calls are not used so much anyway. The second session must be properly closed though.

hlg avatar Nov 17 '21 22:11 hlg

The BimserverDatabaseException (Object not found) stems most likely from inconsistencies in the model introduced through low level manipulation. With the low level calls, you are responsible to keep the model in consistent state. For example upon deletion you must also remove/unset all references to the deleted object, including references via inverse attributes. If there are dangling references that still point to the deleted object, the DownloadDatabaseAction will try to include those deleted objects into the server model and throw the exception. For the same reason, the client model may have the rudimentary objects included, probably. I was just digging through an example and stepped into the same pitfall.

For the other issues, whether it is necessary to have two database sessions or can be fixed in another way, I still have to verify.

Yes. But you will get an exception much earlier: when you try to save inconsistent model. When I created IfcPipeSegment I also created IfcRelContainedInSpatialStructure. If I try to remove just IfcPipeSegment via LowLevelInterface and try to save the model I will get an exception about inconsistency. So this is not the rudimentary objects case... The problem is in database sessions and revisions as you pointed out in your second post.

Also you say that low level calls are not used so much. But to delete something in the model this is the only way AFAIK... So even if I do everything right I still get an exception mentioned earlier. Probably because "it finds object with equal or smaller revision". Just change the order of comparison: take revision with deleted object (properly deleted) as first one and you will get this exception.

dkurillo avatar Nov 18 '21 08:11 dkurillo

If you do everything right with deletions, you won't get the exception during compare, see test case below.

you will get an exception much earlier: when you try to save inconsistent model

Not necessarily. If you are seeing "org.bimserver.BimserverDatabaseException: Referenced object with oid ... (IfcPipeSegment), referenced from IfcRelContainedInSpatialStructure not found" then this is a side effect of restoring references for inverse relations and it only occurs if you modified the model such that there are dangling references for direct attributes which do have an inverse. If the attributes of dangling references don't have inverses or are themselves inverse attributes, then this issue will not appear. So even though you did not see this exception, you may have an inconsistent model. Of course the low level interface could be improved in the future to prevent or at least warn about such violations of consistency. But in the current state responsibility is entirely at the caller side.

But to delete something in the model this [low level calls] is the only way AFAIK...

You cannot delete anything, neither with nor without low level calls. When a low level commit removes an object, it creates a new revision with a delete record for that particular object. You could achieve similar by deleting with an external application and then checking in a new revision with the object removed. The difference between low level calls and checkins is that revisions from low level calls don't replicate unchanged objects in the database, but instead use references to objects of previous revisions. That is the case no matter whether you delete, add or change objects via the low level interface.

Just change the order of comparison

There are two issues here. First one is the issue with object cache in shared database session. Once this is fixed (for example by using two sessions), the exception will appear independent of the order of comparison, because each model is loaded independently and can't get away with dead links that happen to be cached from loading the other (earlier) revision. The second issue is inconsistent model state. If you fix this first, you won't get the exception but may still get mixed revision models not useful for comparison due to the shared database session.

Probably because "it finds object with equal or smaller revision".

That's fine, because we want to find unchanged objects as well. Basically, when loading, we want to find the latest change for every object, which may be in the revision we are about to load or in a previous revision. Or it may also be a delete record from this or previous revisions. See the Wiki on Database versioning if interested.

Here is a test case inspired by your example. Tested with the GUID-based model compare. Hope this clarifies.

import bimserver
import base64
from datetime import datetime

client = bimserver.Api("http://localhost:8080/", "admin@localhost", "admin")
mc = client.PluginInterface.getDefaultModelCompare()['oid']

def createProjectRevisionStorey():
  now = datetime.now().strftime("%Y%m%d%H%M%S") 
  project = client.ServiceInterface.addProject(projectName='test #1221 '+now, schema='IFC4')
  poid = project['oid']
  t = client.LowLevelInterface.startTransaction(poid=poid)
  storeyOid = client.LowLevelInterface.createObject(tid=t, className='IfcBuildingStorey', generateGuid=True)
  rev = client.LowLevelInterface.commitTransaction(tid=t, comment='added initial building storey', regenerateAllGeometry=True)
  return (rev, poid, storeyOid)

def addPipeSementToStorey(poid, storeyOid):
  t = client.LowLevelInterface.startTransaction(poid=poid)
  pipeOid = client.LowLevelInterface.createObject(tid=t, className='IfcPipeSegment', generateGuid=True)
  relOid = client.LowLevelInterface.createObject(tid=t, className='IfcRelContainedInSpatialStructure', generateGuid=True)
  client.LowLevelInterface.addReference(tid=t, oid=relOid, referenceName='RelatedElements', referenceOid=pipeOid)
  client.LowLevelInterface.setReference(tid=t, oid=relOid, referenceName='RelatingStructure', referenceOid=storeyOid)
  rev = client.LowLevelInterface.commitTransaction(tid=t, comment='added pipe segement contained in storey', regenerateAllGeometry=False)
  return (rev, pipeOid, relOid)

def deletePipeSegmentOnly(poid, pipeOid):
  t = client.LowLevelInterface.startTransaction(poid=poid)
  client.LowLevelInterface.removeObject(tid=t, oid=pipeOid)
  client.LowLevelInterface.commitTransaction(tid=t, comment='remove pipe segment without removing relation', regenerateAllGeometry=False) # returns -1 (no revision created)
  # org.bimserver.BimserverDatabaseException: Referenced object with oid 67304 (IfcPipeSegment), referenced from IfcRelContainedInSpatialStructure not found
  client.LowLevelInterface.abortTransaction(tid=t)

def deletePipeSegmentAndContainment(poid, pipeOid, relOid):
  t = client.LowLevelInterface.startTransaction(poid=poid)
  client.LowLevelInterface.removeObject(tid=t, oid=pipeOid)
  client.LowLevelInterface.removeObject(tid=t, oid=relOid)
  return client.LowLevelInterface.commitTransaction(tid=t, comment='remove pipe segment without removing references', regenerateAllGeometry=False)

def deleteInverseAttributeReference(poid, storeyOid):
  t = client.LowLevelInterface.startTransaction(poid=poid)
  client.LowLevelInterface.removeAllReferences(tid=t, oid=storeyOid, referenceName='ContainsElements')
  # if other elements in storey, first find index of the one to remove and then remove only that particular one
  return client.LowLevelInterface.commitTransaction(tid=t, comment='remove inverse attribute reference', regenerateAllGeometry=False)

def compare(a, b):
  client.ServiceInterface.compare(roid1 = a, roid2 = b, mcid = mc, sCompareType = 'ALL')
  client.ServiceInterface.compare(roid1 = b, roid2 = a, mcid = mc, sCompareType = 'ALL')

(r1, poid, storeyOid) = createProjectRevisionStorey()
(r2, pipeOid, relOid) = addPipeSementToStorey(poid, storeyOid)
# deletePipeSegmentOnly(poid, pipeOid) # won't work
r3 = deletePipeSegmentAndContainment(poid, pipeOid, relOid)  # this is not enough
r4 = deleteInverseAttributeReference(poid, storeyOid) # we also need to delete the inverse from storey

compare(r2, r3)
# org.bimserver.BimserverDatabaseException: Object not found: 6 3 460440 IfcRelContainedInSpatialStructure (for both directions!)

compare(r2, r4)
# works for both directions in a symmetric fashion (same objects reported as added or deleted)

compare(r1, r4)
# empty result as revisions are equal

hlg avatar Nov 18 '21 21:11 hlg