Skip to content

Commit

Permalink
Use IProgress instead of custom type (#131)
Browse files Browse the repository at this point in the history
* Use IProgress instead of custom type

* Format

* ProgressArgs are a readonly struct

* fix tests
  • Loading branch information
adamhathcock authored Oct 2, 2024
1 parent 9a3eddf commit 75d88c5
Show file tree
Hide file tree
Showing 23 changed files with 99 additions and 108 deletions.
9 changes: 4 additions & 5 deletions src/Speckle.Sdk/Api/Operations/Operations.Receive.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
using System.Collections.Concurrent;
using Microsoft.Extensions.Logging;
using Speckle.Sdk.Logging;
using Speckle.Sdk.Models;
Expand Down Expand Up @@ -35,7 +34,7 @@ public async Task<Base> Receive(
string objectId,
ITransport? remoteTransport = null,
ITransport? localTransport = null,
Action<ConcurrentBag<ProgressArgs>>? onProgressAction = null,
IProgress<ProgressArgs>? onProgressAction = null,
Action<int>? onTotalChildrenCountKnown = null,
CancellationToken cancellationToken = default
)
Expand All @@ -57,7 +56,7 @@ public async Task<Base> Receive(
objectId,
remoteTransport,
localTransport,
GetInternalProgressAction(onProgressAction),
onProgressAction,
onTotalChildrenCountKnown,
cancellationToken
)
Expand All @@ -74,12 +73,12 @@ public async Task<Base> Receive(
}
}

/// <inheritdoc cref="Receive(string,ITransport?,ITransport?,Action{ConcurrentBag{ProgressArgs}}?,Action{int}?,CancellationToken)"/>
/// <inheritdoc cref="Receive(string,ITransport?,ITransport?,IProgress{ProgressArgs}?,Action{int}?,CancellationToken)"/>
private async Task<Base> ReceiveImpl(
string objectId,
ITransport? remoteTransport,
ITransport localTransport,
Action<ProgressArgs>? internalProgressAction,
IProgress<ProgressArgs>? internalProgressAction,
Action<int>? onTotalChildrenCountKnown,
CancellationToken cancellationToken
)
Expand Down
15 changes: 6 additions & 9 deletions src/Speckle.Sdk/Api/Operations/Operations.Send.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
using System.Collections.Concurrent;
using System.Diagnostics;
using Microsoft.Extensions.Logging;
using Speckle.Newtonsoft.Json.Linq;
Expand All @@ -15,7 +14,7 @@ public partial class Operations
/// Sends a Speckle Object to the provided <paramref name="transport"/> and (optionally) the default local cache
/// </summary>
/// <remarks/>
/// <inheritdoc cref="Send(Base, IReadOnlyCollection{ITransport}, Action{ConcurrentBag{ProgressArgs}}?, CancellationToken)"/>
/// <inheritdoc cref="Send(Base, IReadOnlyCollection{ITransport}, IProgress{ProgressArgs}?, CancellationToken)"/>
/// <param name="useDefaultCache">When <see langword="true"/>, an additional <see cref="SQLiteTransport"/> will be included</param>
/// <exception cref="ArgumentNullException">The <paramref name="transport"/> or <paramref name="value"/> was <see langword="null"/></exception>
/// <example><code>
Expand All @@ -26,7 +25,7 @@ public partial class Operations
Base value,
ITransport transport,
bool useDefaultCache,
Action<ConcurrentBag<ProgressArgs>>? onProgressAction = null,
IProgress<ProgressArgs>? onProgressAction = null,
CancellationToken cancellationToken = default
)
{
Expand Down Expand Up @@ -62,7 +61,7 @@ public partial class Operations
public async Task<(string rootObjId, IReadOnlyDictionary<string, ObjectReference> convertedReferences)> Send(
Base value,
IReadOnlyCollection<ITransport> transports,
Action<ConcurrentBag<ProgressArgs>>? onProgressAction = null,
IProgress<ProgressArgs>? onProgressAction = null,
CancellationToken cancellationToken = default
)
{
Expand All @@ -85,13 +84,11 @@ public partial class Operations
var sendTimer = Stopwatch.StartNew();
logger.LogDebug("Starting send operation");

var internalProgressAction = GetInternalProgressAction(onProgressAction);

SpeckleObjectSerializer serializerV2 = new(transports, internalProgressAction, true, cancellationToken);
SpeckleObjectSerializer serializerV2 = new(transports, onProgressAction, true, cancellationToken);

foreach (var t in transports)
{
t.OnProgressAction = internalProgressAction;
t.OnProgressAction = onProgressAction;
t.CancellationToken = cancellationToken;
t.BeginWrite();
}
Expand Down Expand Up @@ -135,7 +132,7 @@ public partial class Operations
}
}

/// <returns><inheritdoc cref="Send(Base, IReadOnlyCollection{ITransport}, Action{ConcurrentBag{ProgressArgs}}?, CancellationToken)"/></returns>
/// <returns><inheritdoc cref="Send(Base, IReadOnlyCollection{ITransport}, IProgress{ProgressArgs}?, CancellationToken)"/></returns>
internal static async Task<string> SerializerSend(
Base value,
SpeckleObjectSerializer serializer,
Expand Down
4 changes: 2 additions & 2 deletions src/Speckle.Sdk/Api/Operations/Operations.Serialize.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public partial class Operations
/// <remarks>
/// If you want to save and persist an object to Speckle Transport or Server,
/// please use any of the "Send" methods.
/// <see cref="Send(Base,Speckle.Sdk.Transports.ITransport,bool,System.Action{System.Collections.Concurrent.ConcurrentBag{ProgressArgs}}?,System.Threading.CancellationToken)"/>
/// <see cref="Send(Base,Speckle.Sdk.Transports.ITransport,bool,System.IProgress{ProgressArgs}?,System.Threading.CancellationToken)"/>
/// </remarks>
/// <param name="value">The object to serialise</param>
/// <param name="cancellationToken"></param>
Expand All @@ -29,7 +29,7 @@ public string Serialize(Base value, CancellationToken cancellationToken = defaul
/// <remarks>
/// Note: if you want to pull an object from a Speckle Transport or Server,
/// please use
/// <see cref="Receive(string,Speckle.Sdk.Transports.ITransport?,Speckle.Sdk.Transports.ITransport?,System.Action{System.Collections.Concurrent.ConcurrentBag{ProgressArgs}}?,System.Action{int}?,System.Threading.CancellationToken)"/>
/// <see cref="Receive(string,Speckle.Sdk.Transports.ITransport?,Speckle.Sdk.Transports.ITransport?,System.IProgress{ProgressArgs}?,System.Action{int}?,System.Threading.CancellationToken)"/>
/// </remarks>
/// <param name="value">The json string representation of a speckle object that you want to deserialize</param>
/// <param name="cancellationToken"></param>
Expand Down
26 changes: 1 addition & 25 deletions src/Speckle.Sdk/Api/Operations/Operations.cs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
using System.Collections.Concurrent;
using Microsoft.Extensions.Logging;
using Speckle.InterfaceGenerator;
using Speckle.Sdk.Logging;
using Speckle.Sdk.Transports;

namespace Speckle.Sdk.Api;

Expand All @@ -12,26 +10,4 @@ namespace Speckle.Sdk.Api;
/// <para>Push/Pull (methods to serialize and send data to one or more servers)</para>
/// </summary>
[GenerateAutoInterface]
public partial class Operations(ILogger<Operations> logger, ISdkActivityFactory activityFactory) : IOperations
{
/// <summary>
/// Factory for progress actions used internally inside send and receive methods.
/// </summary>
/// <param name="onProgressAction"></param>
/// <returns></returns>
private static Action<ProgressArgs>? GetInternalProgressAction(Action<ConcurrentBag<ProgressArgs>>? onProgressAction)
{
if (onProgressAction is null)
{
return null;
}

return (args) =>
{
var localProgressDict = new ConcurrentBag<ProgressArgs>();
localProgressDict.Add(args);
onProgressAction.Invoke(localProgressDict);
};
}
}
public partial class Operations(ILogger<Operations> logger, ISdkActivityFactory activityFactory) : IOperations;
14 changes: 5 additions & 9 deletions src/Speckle.Sdk/Serialisation/SpeckleObjectDeserializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ namespace Speckle.Sdk.Serialisation;
public sealed class SpeckleObjectDeserializer
{
private volatile bool _isBusy;
private readonly object _callbackLock = new();
private readonly object?[] _invokeNull = [null];

// id -> Base if already deserialized or id -> ValueTask<object> if was handled by a bg thread
Expand All @@ -32,7 +31,7 @@ public sealed class SpeckleObjectDeserializer
/// </summary>
public ITransport ReadTransport { get; set; }

public Action<ProgressArgs>? OnProgressAction { get; set; }
public IProgress<ProgressArgs>? OnProgressAction { get; set; }

private long _currentCount;
private readonly HashSet<string> _ids = new();
Expand Down Expand Up @@ -99,13 +98,10 @@ public async ValueTask<Base> DeserializeAsync([NotNull] string? rootObjectJson)
throw new SpeckleDeserializeException("Failed to deserialize", ex);
}

lock (_callbackLock)
{
_processedCount++;
OnProgressAction?.Invoke(
new ProgressArgs(ProgressEvent.DeserializeObject, _currentCount, _ids.Count, _processedCount)
);
}
_processedCount++;
OnProgressAction?.Report(
new ProgressArgs(ProgressEvent.DeserializeObject, _currentCount, _ids.Count, _processedCount)
);

return converted;
}
Expand Down
6 changes: 3 additions & 3 deletions src/Speckle.Sdk/Serialisation/SpeckleObjectSerializer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ public class SpeckleObjectSerializer
private List<Dictionary<string, int>> _parentClosures = new();
private HashSet<object> _parentObjects = new();
private readonly Dictionary<string, List<(PropertyInfo, PropertyAttributeInfo)>> _typedPropertiesCache = new();
private readonly Action<ProgressArgs>? _onProgressAction;
private readonly IProgress<ProgressArgs>? _onProgressAction;

private readonly bool _trackDetachedChildren;
private int _serializedCount;
Expand Down Expand Up @@ -51,7 +51,7 @@ public SpeckleObjectSerializer()
/// <param name="cancellationToken"></param>
public SpeckleObjectSerializer(
IReadOnlyCollection<ITransport> writeTransports,
Action<ProgressArgs>? onProgressAction = null,
IProgress<ProgressArgs>? onProgressAction = null,
bool trackDetachedChildren = false,
CancellationToken cancellationToken = default
)
Expand Down Expand Up @@ -279,7 +279,7 @@ private void SerializeProperty(
var json2 = writer2.ToString();
UpdateParentClosures(id);

_onProgressAction?.Invoke(new(ProgressEvent.SerializeObject, ++_serializedCount, null));
_onProgressAction?.Report(new(ProgressEvent.SerializeObject, ++_serializedCount, null));

// add to obj refs to return
if (baseObj.applicationId != null && _trackDetachedChildren) // && baseObj is not DataChunk && baseObj is not Abstract) // not needed, as data chunks will never have application ids, and abstract objs are not really used.
Expand Down
4 changes: 2 additions & 2 deletions src/Speckle.Sdk/Transports/DiskTransport.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public object Clone()

public CancellationToken CancellationToken { get; set; }

public Action<ProgressArgs>? OnProgressAction { get; set; }
public IProgress<ProgressArgs>? OnProgressAction { get; set; }

public Action<string, Exception>? OnErrorAction { get; set; }

Expand Down Expand Up @@ -93,7 +93,7 @@ public void SaveObject(string id, string serializedObject)
}

SavedObjectCount++;
OnProgressAction?.Invoke(new(ProgressEvent.DownloadObject, SavedObjectCount, null));
OnProgressAction?.Report(new(ProgressEvent.DownloadObject, SavedObjectCount, null));
stopwatch.Stop();
Elapsed += stopwatch.Elapsed;
}
Expand Down
9 changes: 7 additions & 2 deletions src/Speckle.Sdk/Transports/ITransport.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,12 @@ public enum ProgressEvent
SerializeObject,
}

public record ProgressArgs(ProgressEvent ProgressEvent, long? Count, long? Total, long? ProcessedTotal = null);
public readonly record struct ProgressArgs(
ProgressEvent ProgressEvent,
long? Count,
long? Total,
long? ProcessedTotal = null
);

/// <summary>
/// Interface defining the contract for transport implementations.
Expand Down Expand Up @@ -42,7 +47,7 @@ public interface ITransport
/// <summary>
/// Used to report progress during the transport's longer operations.
/// </summary>
public Action<ProgressArgs>? OnProgressAction { get; set; }
public IProgress<ProgressArgs>? OnProgressAction { get; set; }

/// <summary>
/// Signals to the transport that writes are about to begin.
Expand Down
4 changes: 2 additions & 2 deletions src/Speckle.Sdk/Transports/MemoryTransport.cs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ public object Clone()

public string TransportName { get; set; } = "Memory";

public Action<ProgressArgs>? OnProgressAction { get; set; }
public IProgress<ProgressArgs>? OnProgressAction { get; set; }

public int SavedObjectCount { get; private set; }

Expand Down Expand Up @@ -85,7 +85,7 @@ public void SaveObject(string id, string serializedObject)
_objects[id] = serializedObject;

SavedObjectCount++;
OnProgressAction?.Invoke(new(ProgressEvent.UploadObject, 1, 1));
OnProgressAction?.Report(new(ProgressEvent.UploadObject, 1, 1));
stopwatch.Stop();
Elapsed += stopwatch.Elapsed;
}
Expand Down
4 changes: 2 additions & 2 deletions src/Speckle.Sdk/Transports/SQLiteTransport.cs
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ public void Dispose()

public CancellationToken CancellationToken { get; set; }

public Action<ProgressArgs>? OnProgressAction { get; set; }
public IProgress<ProgressArgs>? OnProgressAction { get; set; }

public int SavedObjectCount { get; private set; }

Expand Down Expand Up @@ -330,7 +330,7 @@ private void ConsumeQueue()
CancellationToken.ThrowIfCancellationRequested();
}

OnProgressAction?.Invoke(new(ProgressEvent.DownloadObject, saved, _queue.Count + 1));
OnProgressAction?.Report(new(ProgressEvent.DownloadObject, saved, _queue.Count + 1));

CancellationToken.ThrowIfCancellationRequested();

Expand Down
2 changes: 1 addition & 1 deletion src/Speckle.Sdk/Transports/ServerTransport.cs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ public void Dispose()
};

public CancellationToken CancellationToken { get; set; }
public Action<ProgressArgs>? OnProgressAction { get; set; }
public IProgress<ProgressArgs>? OnProgressAction { get; set; }
public TimeSpan Elapsed { get; private set; } = TimeSpan.Zero;

public async Task<string> CopyObjectAndChildren(
Expand Down
10 changes: 5 additions & 5 deletions src/Speckle.Sdk/Transports/ServerUtils/IServerApi.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ namespace Speckle.Sdk.Transports.ServerUtils;

internal interface IServerApi
{
public Task<string?> DownloadSingleObject(string streamId, string objectId, Action<ProgressArgs>? progress);
public Task<string?> DownloadSingleObject(string streamId, string objectId, IProgress<ProgressArgs>? progress);

public Task DownloadObjects(
string streamId,
IReadOnlyList<string> objectIds,
Action<ProgressArgs>? progress,
IProgress<ProgressArgs>? progress,
CbObjectDownloaded onObjectCallback
);

Expand All @@ -18,14 +18,14 @@ CbObjectDownloaded onObjectCallback
public Task UploadObjects(
string streamId,
IReadOnlyList<(string id, string data)> objects,
Action<ProgressArgs>? progress
IProgress<ProgressArgs>? progress
);

public Task UploadBlobs(
string streamId,
IReadOnlyList<(string id, string data)> objects,
Action<ProgressArgs>? progress
IProgress<ProgressArgs>? progress
);

public Task DownloadBlobs(string streamId, IReadOnlyList<string> blobIds, Action<ProgressArgs>? progress);
public Task DownloadBlobs(string streamId, IReadOnlyList<string> blobIds, IProgress<ProgressArgs>? progress);
}
Loading

0 comments on commit 75d88c5

Please sign in to comment.