Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix(revit): CNX-657 revit expiration checks seem to be not fully working for #316

Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,11 @@ internal sealed class RevitSendBinding : RevitBaseBinding, ISendBinding
/// </summary>
private ConcurrentDictionary<string, byte> ChangedObjectIds { get; set; } = new();

/// <summary>
/// We need it to get UniqueId whenever it is not available i.e. GetDeletedElementIds returns ElementId and cannot find its Element to get UniqueId. We store them both just before send to remember later.
/// </summary>
private ConcurrentDictionary<string, string> IdMap { get; } = new();

public RevitSendBinding(
IRevitIdleManager idleManager,
RevitContext revitContext,
Expand Down Expand Up @@ -129,15 +134,22 @@ public async Task Send(string modelCardId)
var activeUIDoc =
RevitContext.UIApplication?.ActiveUIDocument
?? throw new SpeckleException("Unable to retrieve active UI document");
List<ElementId> revitObjects = modelCard

List<Element> elements = modelCard
.SendFilter.NotNull()
.GetObjectIds()
.Select(uid => activeUIDoc.Document.GetElement(uid))
.Where(el => el is not null) // NOTE: elements can get deleted from the host app.
.Select(el => el.Id)
.Where(el => el is not null)
.ToList();

if (revitObjects.Count == 0)
foreach (Element element in elements)
{
IdMap[element.Id.ToString()] = element.UniqueId;
}

List<ElementId> elementIds = elements.Select(el => el.Id).ToList();

if (elementIds.Count == 0)
{
// Handle as CARD ERROR in this function
throw new SpeckleSendFilterException("No objects were found to convert. Please update your publish filter!");
Expand All @@ -146,7 +158,7 @@ public async Task Send(string modelCardId)
var sendResult = await scope
.ServiceProvider.GetRequiredService<SendOperation<ElementId>>()
.Execute(
revitObjects,
elementIds,
modelCard.GetSendInfo(_speckleApplication.Slug),
_operationProgressManager.CreateOperationProgressEventHandler(Parent, modelCardId, cancellationToken),
cancellationToken
Expand Down Expand Up @@ -242,25 +254,25 @@ private bool HaveUnitsChanged(Document doc)
private async Task RunExpirationChecks()
{
var senders = Store.GetSenders();
string[] objectIdsList = ChangedObjectIds.Keys.ToArray();
// string[] objectIdsList = ChangedObjectIds.Keys.ToArray();
var doc = RevitContext.UIApplication?.ActiveUIDocument.Document;

if (doc == null)
{
return;
}

// Note: We're using unique ids as application ids in revit, so cache eviction must happen by those.
// NOTE: this is currently broken when deleting freestanding elements (e.g. unhosted elements)
// To reproduce, draw two unconnected walls, send, delete one wall -> no expiration notice
// I do not yet know the solution besides going back to element ids, but it would mean revisiting why we switched to unique ids (conflicting ids)
var objUniqueIds = objectIdsList
.Select(id => new ElementId(Convert.ToInt32(id)))
.Select(doc.GetElement)
.Where(el => el is not null)
.Select(el => el.UniqueId)
.ToList();
_sendConversionCache.EvictObjects(objUniqueIds);
var objUniqueIds = new List<string>();
foreach (string changedElementId in ChangedObjectIds.Keys.ToArray())
{
if (IdMap.TryGetValue(changedElementId, out var uniqueId))
{
objUniqueIds.Add(uniqueId);
}
}

var unpackedObjectIds = _elementUnpacker.GetUnpackedElementIds(objUniqueIds);
_sendConversionCache.EvictObjects(unpackedObjectIds);

// Note: we're doing object selection and card expiry management by old school ids
List<string> expiredSenderIds = new();
Expand All @@ -282,6 +294,9 @@ private async Task RunExpirationChecks()
// That's why don't bother for now how to get rid of from dup logic in other bindings.
private async Task OnDocumentChanged()
{
_sendConversionCache.ClearCache();
IdMap.Clear();

if (_cancellationManager.NumberOfOperations > 0)
{
_cancellationManager.CancelAllOperations();
Expand Down
Loading