From 01c5f8e5ba28d66aa1ca48638ce80cce48735a50 Mon Sep 17 00:00:00 2001 From: Adam Hathcock Date: Wed, 18 Dec 2024 11:04:52 +0000 Subject: [PATCH] Add Result objects for Rhino --- .../Receive/RhinoHostObjectBuilder.cs | 12 +++-- .../Operations/Send/RhinoRootObjectBuilder.cs | 14 ++++- ...ckleToHostGeometryBaseTopLevelConverter.cs | 9 ++-- .../FallbackToHostTopLevelConverter.cs | 3 +- .../BrepObjectToSpeckleTopLevelConverter.cs | 6 +-- ...trusionObjectToSpeckleTopLevelConverter.cs | 6 +-- .../RhinoObjectToSpeckleTopLevelConverter.cs | 5 +- .../SubDObjectToSpeckleTopLevelConverter.cs | 6 +-- .../Conversion/ReportResult.cs | 12 +++++ .../Extensions/RootObjectBuilderExtensions.cs | 9 ++++ .../ToHostTopLevelConverterExtension.cs | 43 ++++++++++++++- .../IRootToHostConverter.cs | 3 +- .../Objects/IToHostTopLevelConverter.cs | 3 +- .../Objects/IToSpeckleTopLevelConverter.cs | 4 +- .../Registration/ConverterManager.cs | 11 ++-- .../Registration/IConversionResult.cs | 39 ++++++++++++++ .../Registration/IConverterManager.cs | 2 +- .../RootToSpeckleConverter.cs | 24 ++++----- .../ToHost/ConverterWithFallback.cs | 53 +++++-------------- .../ToHost/ConverterWithoutFallback.cs | 29 +++------- 20 files changed, 185 insertions(+), 108 deletions(-) create mode 100644 Sdk/Speckle.Converters.Common/Registration/IConversionResult.cs diff --git a/Connectors/Rhino/Speckle.Connectors.RhinoShared/Operations/Receive/RhinoHostObjectBuilder.cs b/Connectors/Rhino/Speckle.Connectors.RhinoShared/Operations/Receive/RhinoHostObjectBuilder.cs index fbcf65ae5..0d85d437b 100644 --- a/Connectors/Rhino/Speckle.Connectors.RhinoShared/Operations/Receive/RhinoHostObjectBuilder.cs +++ b/Connectors/Rhino/Speckle.Connectors.RhinoShared/Operations/Receive/RhinoHostObjectBuilder.cs @@ -147,15 +147,21 @@ CancellationToken cancellationToken // 2: convert var result = _converter.Convert(obj); + if (!result.IsSuccess) + { + conversionResults.Add(new(Status.ERROR, obj, result.Message.NotNull())); + convertActivity?.SetStatus(SdkActivityStatusCode.Error); + continue; + } // 3: bake var conversionIds = new List(); - if (result is GeometryBase geometryBase) + if (result.Host is GeometryBase geometryBase) { var guid = BakeObject(geometryBase, obj, null, atts); conversionIds.Add(guid.ToString()); } - else if (result is List geometryBases) // one to many raw encoding case + else if (result.Host is List geometryBases) // one to many raw encoding case { // NOTE: I'm unhappy about this case (dim). It's needed as the raw encoder approach can hypothetically return // multiple "geometry bases" - but this is not a fallback conversion. @@ -167,7 +173,7 @@ CancellationToken cancellationToken conversionIds.Add(guid.ToString()); } } - else if (result is IEnumerable<(object, Base)> fallbackConversionResult) // one to many fallback conversion + else if (result.Host is IEnumerable<(object, Base)> fallbackConversionResult) // one to many fallback conversion { var guids = BakeObjectsAsFallbackGroup(fallbackConversionResult, obj, atts, baseLayerName); conversionIds.AddRange(guids.Select(id => id.ToString())); diff --git a/Connectors/Rhino/Speckle.Connectors.RhinoShared/Operations/Send/RhinoRootObjectBuilder.cs b/Connectors/Rhino/Speckle.Connectors.RhinoShared/Operations/Send/RhinoRootObjectBuilder.cs index 5f337e491..bacc4601f 100644 --- a/Connectors/Rhino/Speckle.Connectors.RhinoShared/Operations/Send/RhinoRootObjectBuilder.cs +++ b/Connectors/Rhino/Speckle.Connectors.RhinoShared/Operations/Send/RhinoRootObjectBuilder.cs @@ -11,6 +11,7 @@ using Speckle.Converters.Common; using Speckle.Converters.Rhino; using Speckle.Sdk; +using Speckle.Sdk.Common; using Speckle.Sdk.Logging; using Speckle.Sdk.Models; using Speckle.Sdk.Models.Collections; @@ -159,8 +160,17 @@ string projectId } else { - converted = _rootToSpeckleConverter.Convert(rhinoObject); - converted.applicationId = applicationId; + var result = _rootToSpeckleConverter.Convert(rhinoObject); + if (result.IsSuccess) + { + converted = result.Base.NotNull(); + converted.applicationId = applicationId; + } + else + { + _logger.LogSendConversionError(sourceType, result.Message.NotNull()); + return new(Status.ERROR, applicationId, sourceType); + } } // add to host diff --git a/Converters/Rhino/Speckle.Converters.RhinoShared/SpeckleToHostGeometryBaseTopLevelConverter.cs b/Converters/Rhino/Speckle.Converters.RhinoShared/SpeckleToHostGeometryBaseTopLevelConverter.cs index fa510e5b7..1315ace46 100644 --- a/Converters/Rhino/Speckle.Converters.RhinoShared/SpeckleToHostGeometryBaseTopLevelConverter.cs +++ b/Converters/Rhino/Speckle.Converters.RhinoShared/SpeckleToHostGeometryBaseTopLevelConverter.cs @@ -1,5 +1,6 @@ using Speckle.Converters.Common; using Speckle.Converters.Common.Objects; +using Speckle.Converters.Common.Registration; using Speckle.Sdk.Common; using Speckle.Sdk.Common.Exceptions; using Speckle.Sdk.Models; @@ -21,7 +22,7 @@ ITypedConverter geometryBaseConverter _geometryBaseConverter = geometryBaseConverter; } - public object Convert(Base target) + public HostResult Convert(Base target) { var castedBase = (TIn)target; var result = _geometryBaseConverter.Convert(castedBase); @@ -37,7 +38,7 @@ public object Convert(Base target) if (result is RG.GeometryBase geometryBase && units is not null) { geometryBase.Transform(GetScaleTransform(units)); - return geometryBase; + return HostResult.Success(geometryBase); } if (result is List geometryBases && units is not null) @@ -48,10 +49,10 @@ public object Convert(Base target) gb.Transform(t); } - return geometryBases; + return HostResult.Success(geometryBases); } - return result; + return HostResult.Success(result); } private RG.Transform GetScaleTransform(string from) diff --git a/Converters/Rhino/Speckle.Converters.RhinoShared/ToHost/TopLevel/FallbackToHostTopLevelConverter.cs b/Converters/Rhino/Speckle.Converters.RhinoShared/ToHost/TopLevel/FallbackToHostTopLevelConverter.cs index 24beb37ad..f8ca30ccb 100644 --- a/Converters/Rhino/Speckle.Converters.RhinoShared/ToHost/TopLevel/FallbackToHostTopLevelConverter.cs +++ b/Converters/Rhino/Speckle.Converters.RhinoShared/ToHost/TopLevel/FallbackToHostTopLevelConverter.cs @@ -1,5 +1,6 @@ using Speckle.Converters.Common; using Speckle.Converters.Common.Objects; +using Speckle.Converters.Common.Registration; using Speckle.Sdk.Common; using Speckle.Sdk.Common.Exceptions; using Speckle.Sdk.Models; @@ -35,7 +36,7 @@ IConverterSettingsStore settingsStore _settingsStore = settingsStore; } - public object Convert(Base target) => Convert((DisplayableObject)target); + public HostResult Convert(Base target) => HostResult.Success(Convert((DisplayableObject)target)); public List Convert(DisplayableObject target) { diff --git a/Converters/Rhino/Speckle.Converters.RhinoShared/ToSpeckle/TopLevel/BrepObjectToSpeckleTopLevelConverter.cs b/Converters/Rhino/Speckle.Converters.RhinoShared/ToSpeckle/TopLevel/BrepObjectToSpeckleTopLevelConverter.cs index c79440bfe..83ec03ca3 100644 --- a/Converters/Rhino/Speckle.Converters.RhinoShared/ToSpeckle/TopLevel/BrepObjectToSpeckleTopLevelConverter.cs +++ b/Converters/Rhino/Speckle.Converters.RhinoShared/ToSpeckle/TopLevel/BrepObjectToSpeckleTopLevelConverter.cs @@ -1,9 +1,9 @@ using Rhino.DocObjects; using Speckle.Converters.Common; using Speckle.Converters.Common.Objects; +using Speckle.Converters.Common.Registration; using Speckle.Converters.Rhino.ToSpeckle.Encoding; using Speckle.Converters.Rhino.ToSpeckle.Meshing; -using Speckle.Sdk.Models; namespace Speckle.Converters.Rhino.ToSpeckle.TopLevel; @@ -22,7 +22,7 @@ IConverterSettingsStore settingsStore _settingsStore = settingsStore; } - public Base Convert(object target) + public BaseResult Convert(object target) { var brepObject = (BrepObject)target; var brepEncoding = RawEncodingCreator.Encode(brepObject.Geometry, _settingsStore.Current.Document); @@ -37,6 +37,6 @@ public Base Convert(object target) units = _settingsStore.Current.SpeckleUnits }; - return bx; + return BaseResult.Success(bx); } } diff --git a/Converters/Rhino/Speckle.Converters.RhinoShared/ToSpeckle/TopLevel/ExtrusionObjectToSpeckleTopLevelConverter.cs b/Converters/Rhino/Speckle.Converters.RhinoShared/ToSpeckle/TopLevel/ExtrusionObjectToSpeckleTopLevelConverter.cs index 255ed0162..144b847fc 100644 --- a/Converters/Rhino/Speckle.Converters.RhinoShared/ToSpeckle/TopLevel/ExtrusionObjectToSpeckleTopLevelConverter.cs +++ b/Converters/Rhino/Speckle.Converters.RhinoShared/ToSpeckle/TopLevel/ExtrusionObjectToSpeckleTopLevelConverter.cs @@ -1,9 +1,9 @@ using Rhino.DocObjects; using Speckle.Converters.Common; using Speckle.Converters.Common.Objects; +using Speckle.Converters.Common.Registration; using Speckle.Converters.Rhino.ToSpeckle.Encoding; using Speckle.Converters.Rhino.ToSpeckle.Meshing; -using Speckle.Sdk.Models; namespace Speckle.Converters.Rhino.ToSpeckle.TopLevel; @@ -22,7 +22,7 @@ IConverterSettingsStore settingsStore _settingsStore = settingsStore; } - public Base Convert(object target) + public BaseResult Convert(object target) { var extrusionObject = (ExtrusionObject)target; var extrusionEncoding = RawEncodingCreator.Encode(extrusionObject.Geometry, _settingsStore.Current.Document); @@ -37,6 +37,6 @@ public Base Convert(object target) units = _settingsStore.Current.SpeckleUnits }; - return bx; + return BaseResult.Success(bx); } } diff --git a/Converters/Rhino/Speckle.Converters.RhinoShared/ToSpeckle/TopLevel/RhinoObjectToSpeckleTopLevelConverter.cs b/Converters/Rhino/Speckle.Converters.RhinoShared/ToSpeckle/TopLevel/RhinoObjectToSpeckleTopLevelConverter.cs index 7ec873b64..0fd342fa8 100644 --- a/Converters/Rhino/Speckle.Converters.RhinoShared/ToSpeckle/TopLevel/RhinoObjectToSpeckleTopLevelConverter.cs +++ b/Converters/Rhino/Speckle.Converters.RhinoShared/ToSpeckle/TopLevel/RhinoObjectToSpeckleTopLevelConverter.cs @@ -1,4 +1,5 @@ using Speckle.Converters.Common.Objects; +using Speckle.Converters.Common.Registration; using Speckle.Sdk.Models; using RhinoObject = Rhino.DocObjects.RhinoObject; @@ -19,7 +20,7 @@ protected RhinoObjectToSpeckleTopLevelConverter(ITypedConverter // POC: IIndex would fix this as I would just request the type from `RhinoObject.Geometry` directly. protected abstract TInRaw GetTypedGeometry(TTopLevelIn input); - public virtual Base Convert(object target) + public BaseResult Convert(object target) { var typedTarget = (TTopLevelIn)target; var typedGeometry = GetTypedGeometry(typedTarget); @@ -33,6 +34,6 @@ public virtual Base Convert(object target) result["name"] = typedTarget.Attributes.Name; } - return result; + return BaseResult.Success( result); } } diff --git a/Converters/Rhino/Speckle.Converters.RhinoShared/ToSpeckle/TopLevel/SubDObjectToSpeckleTopLevelConverter.cs b/Converters/Rhino/Speckle.Converters.RhinoShared/ToSpeckle/TopLevel/SubDObjectToSpeckleTopLevelConverter.cs index f86839467..8647f4561 100644 --- a/Converters/Rhino/Speckle.Converters.RhinoShared/ToSpeckle/TopLevel/SubDObjectToSpeckleTopLevelConverter.cs +++ b/Converters/Rhino/Speckle.Converters.RhinoShared/ToSpeckle/TopLevel/SubDObjectToSpeckleTopLevelConverter.cs @@ -1,9 +1,9 @@ using Rhino.DocObjects; using Speckle.Converters.Common; using Speckle.Converters.Common.Objects; +using Speckle.Converters.Common.Registration; using Speckle.Converters.Rhino.ToSpeckle.Encoding; using Speckle.Converters.Rhino.ToSpeckle.Meshing; -using Speckle.Sdk.Models; namespace Speckle.Converters.Rhino.ToSpeckle.TopLevel; @@ -22,7 +22,7 @@ IConverterSettingsStore settingsStore _settingsStore = settingsStore; } - public Base Convert(object target) + public BaseResult Convert(object target) { var subDObject = (SubDObject)target; var subdEncoding = RawEncodingCreator.Encode(subDObject.Geometry, _settingsStore.Current.Document); @@ -37,6 +37,6 @@ public Base Convert(object target) units = _settingsStore.Current.SpeckleUnits }; - return bx; + return BaseResult.Success(bx); } } diff --git a/Sdk/Speckle.Connectors.Common/Conversion/ReportResult.cs b/Sdk/Speckle.Connectors.Common/Conversion/ReportResult.cs index 19b3ec485..ae5defe42 100644 --- a/Sdk/Speckle.Connectors.Common/Conversion/ReportResult.cs +++ b/Sdk/Speckle.Connectors.Common/Conversion/ReportResult.cs @@ -55,6 +55,18 @@ public ReceiveConversionResult( ResultType = resultType; Error = FormatError(exception); } + + public ReceiveConversionResult( + Status status, + Base source, + string exception + ) + { + Status = status; + SourceId = source.id.NotNull(); + SourceType = source.speckle_type; // Note: we'll parse it nicely in FE + Error = new ErrorWrapper() { Message = exception, StackTrace = "" }; + } } /// diff --git a/Sdk/Speckle.Connectors.Common/Extensions/RootObjectBuilderExtensions.cs b/Sdk/Speckle.Connectors.Common/Extensions/RootObjectBuilderExtensions.cs index fa630c4c8..c018271da 100644 --- a/Sdk/Speckle.Connectors.Common/Extensions/RootObjectBuilderExtensions.cs +++ b/Sdk/Speckle.Connectors.Common/Extensions/RootObjectBuilderExtensions.cs @@ -20,4 +20,13 @@ string objectType logger.Log(logLevel, ex, "Conversion of object {objectType} was not successful", objectType); } + + public static void LogSendConversionError( + this ILogger> logger, + string objectType, + string message + ) + { + logger.Log(LogLevel.Information, "Conversion of object {objectType} was not successful:" + message, objectType); + } } diff --git a/Sdk/Speckle.Converters.Common/Extensions/ToHostTopLevelConverterExtension.cs b/Sdk/Speckle.Converters.Common/Extensions/ToHostTopLevelConverterExtension.cs index 27919c6ac..2e1b30471 100644 --- a/Sdk/Speckle.Converters.Common/Extensions/ToHostTopLevelConverterExtension.cs +++ b/Sdk/Speckle.Converters.Common/Extensions/ToHostTopLevelConverterExtension.cs @@ -1,13 +1,54 @@ using Microsoft.Extensions.Logging; using Speckle.Converters.Common.Objects; +using Speckle.Converters.Common.Registration; using Speckle.Sdk; +using Speckle.Sdk.Common; using Speckle.Sdk.Models; namespace Speckle.Converters.Common.Extensions; public static class ToHostTopLevelConverterExtension { - public static object ConvertAndLog(this IToHostTopLevelConverter converter, Base target, ILogger logger) + public static HostResult ConvertAndLog(this ConverterResult converter, Base target, ILogger logger) + { + try + { + if (converter.IsSuccess) + { + return converter.Converter.NotNull().Convert(target); + } + + logger.Log( + LogLevel.Information, + "Conversion of object {target} using {converter} was not successful", + target.GetType(), + converter.GetType() + ); + return HostResult.NoConverter(converter.Message); + } +#pragma warning disable CA1031 + catch (Exception ex) +#pragma warning restore CA1031 + { + //SpeckleExceptions are expected, if a converter throws anything else, its considered an error that we should investigate and fix + LogLevel logLevel = ex switch + { + SpeckleException => LogLevel.Information, //If it's too noisy, we could demote to LogLevel.Debug + _ => LogLevel.Error + }; + + logger.Log( + logLevel, + ex, + "Conversion of object {target} using {converter} was not successful", + target.GetType(), + converter.GetType() + ); + + return HostResult.NoConversion($"Conversion of object {target} using {converter} was not successful: " + ex.Message); + } + } + public static HostResult ConvertAndLog(this IToHostTopLevelConverter converter, Base target, ILogger logger) { try { diff --git a/Sdk/Speckle.Converters.Common/IRootToHostConverter.cs b/Sdk/Speckle.Converters.Common/IRootToHostConverter.cs index 2d8c52fad..1a6775064 100644 --- a/Sdk/Speckle.Converters.Common/IRootToHostConverter.cs +++ b/Sdk/Speckle.Converters.Common/IRootToHostConverter.cs @@ -1,8 +1,9 @@ +using Speckle.Converters.Common.Registration; using Speckle.Sdk.Models; namespace Speckle.Converters.Common; public interface IRootToHostConverter { - object Convert(Base target); + HostResult Convert(Base target); } diff --git a/Sdk/Speckle.Converters.Common/Objects/IToHostTopLevelConverter.cs b/Sdk/Speckle.Converters.Common/Objects/IToHostTopLevelConverter.cs index 41a67f1a9..d803a034c 100644 --- a/Sdk/Speckle.Converters.Common/Objects/IToHostTopLevelConverter.cs +++ b/Sdk/Speckle.Converters.Common/Objects/IToHostTopLevelConverter.cs @@ -1,8 +1,9 @@ +using Speckle.Converters.Common.Registration; using Speckle.Sdk.Models; namespace Speckle.Converters.Common.Objects; public interface IToHostTopLevelConverter { - object Convert(Base target); + HostResult Convert(Base target); } diff --git a/Sdk/Speckle.Converters.Common/Objects/IToSpeckleTopLevelConverter.cs b/Sdk/Speckle.Converters.Common/Objects/IToSpeckleTopLevelConverter.cs index 827ae43cd..d9fba6afa 100644 --- a/Sdk/Speckle.Converters.Common/Objects/IToSpeckleTopLevelConverter.cs +++ b/Sdk/Speckle.Converters.Common/Objects/IToSpeckleTopLevelConverter.cs @@ -1,8 +1,8 @@ -using Speckle.Sdk.Models; +using Speckle.Converters.Common.Registration; namespace Speckle.Converters.Common.Objects; public interface IToSpeckleTopLevelConverter { - Base Convert(object target); + BaseResult Convert(object target); } diff --git a/Sdk/Speckle.Converters.Common/Registration/ConverterManager.cs b/Sdk/Speckle.Converters.Common/Registration/ConverterManager.cs index 49a1558fd..4a48d6ef0 100644 --- a/Sdk/Speckle.Converters.Common/Registration/ConverterManager.cs +++ b/Sdk/Speckle.Converters.Common/Registration/ConverterManager.cs @@ -1,15 +1,15 @@ using System.Collections.Concurrent; using Microsoft.Extensions.DependencyInjection; -using Speckle.Sdk.Common.Exceptions; namespace Speckle.Converters.Common.Registration; + public class ConverterManager(ConcurrentDictionary converterTypes, IServiceProvider serviceProvider) : IConverterManager { public string Name => typeof(T).Name; - public T ResolveConverter(Type type, bool recursive = true) + public ConverterResult ResolveConverter(Type type, bool recursive = true) { var currentType = type; while (true) @@ -23,16 +23,17 @@ public T ResolveConverter(Type type, bool recursive = true) if (currentType == null) { - throw new ConversionNotSupportedException($"No conversion found for {type.Name} or any of its base types"); + return new ConverterResult(ConversionStatus.NoConverter, Message: $"No conversion found for {type.Name} or any of its base types"); } } else if (converter is null) { - throw new ConversionNotSupportedException($"No conversion found for {type.Name}"); + return new ConverterResult(ConversionStatus.NoConverter, Message: $"No conversion found for {type.Name}"); } else { - return converter; + + return new ConverterResult(ConversionStatus.Success, converter); } } } diff --git a/Sdk/Speckle.Converters.Common/Registration/IConversionResult.cs b/Sdk/Speckle.Converters.Common/Registration/IConversionResult.cs new file mode 100644 index 000000000..5a41f064c --- /dev/null +++ b/Sdk/Speckle.Converters.Common/Registration/IConversionResult.cs @@ -0,0 +1,39 @@ +using Speckle.Sdk.Models; + +namespace Speckle.Converters.Common.Registration; + +public interface IConversionResult +{ + ConversionStatus ConversionStatus { get; } + string? Message { get; } + bool IsSuccess { get; } +} + +public enum ConversionStatus +{ + Success, + NoConverter, + NoConversion +} + +public readonly record struct ConverterResult(ConversionStatus ConversionStatus, T? Converter = default, string? Message = null) : IConversionResult +{ + public bool IsSuccess => ConversionStatus == ConversionStatus.Success; +} + +public readonly record struct BaseResult(ConversionStatus ConversionStatus, Base? Base = null, string? Message = null) : IConversionResult +{ + public bool IsSuccess => ConversionStatus == ConversionStatus.Success; + + public static BaseResult Success(Base baseObject) => new(ConversionStatus.Success, baseObject); + public static BaseResult NoConverter(string? message) => new(ConversionStatus.NoConverter, Message: message); +} + +public readonly record struct HostResult(ConversionStatus ConversionStatus, object? Host = null, string? Message = null) : IConversionResult +{ + public bool IsSuccess => ConversionStatus == ConversionStatus.Success; + + public static HostResult Success(object obj) => new(ConversionStatus.Success, obj); + public static HostResult NoConverter(string? message) => new(ConversionStatus.NoConverter, Message: message); + public static HostResult NoConversion(string? message) => new(ConversionStatus.NoConversion, Message: message); +} diff --git a/Sdk/Speckle.Converters.Common/Registration/IConverterManager.cs b/Sdk/Speckle.Converters.Common/Registration/IConverterManager.cs index 6be889288..4b4569925 100644 --- a/Sdk/Speckle.Converters.Common/Registration/IConverterManager.cs +++ b/Sdk/Speckle.Converters.Common/Registration/IConverterManager.cs @@ -3,5 +3,5 @@ namespace Speckle.Converters.Common.Registration; public interface IConverterManager { public string Name { get; } - public T ResolveConverter(Type type, bool recursive = true); + public ConverterResult ResolveConverter(Type type, bool recursive = true); } diff --git a/Sdk/Speckle.Converters.Common/RootToSpeckleConverter.cs b/Sdk/Speckle.Converters.Common/RootToSpeckleConverter.cs index b087e5f08..71f6a3894 100644 --- a/Sdk/Speckle.Converters.Common/RootToSpeckleConverter.cs +++ b/Sdk/Speckle.Converters.Common/RootToSpeckleConverter.cs @@ -1,28 +1,24 @@ using Speckle.Converters.Common.Objects; using Speckle.Converters.Common.Registration; using Speckle.InterfaceGenerator; -using Speckle.Sdk.Models; +using Speckle.Sdk.Common; namespace Speckle.Converters.Common; [GenerateAutoInterface] -public class RootToSpeckleConverter : IRootToSpeckleConverter +public class RootToSpeckleConverter(IConverterManager toSpeckle) : IRootToSpeckleConverter { - private readonly IConverterManager _toSpeckle; - - public RootToSpeckleConverter(IConverterManager toSpeckle) - { - _toSpeckle = toSpeckle; - } - - public Base Convert(object target) + public BaseResult Convert(object target) { Type type = target.GetType(); - var objectConverter = _toSpeckle.ResolveConverter(type); - - var convertedObject = objectConverter.Convert(target); + var result = toSpeckle.ResolveConverter(type); - return convertedObject; + if (result.IsSuccess) + { + var convertedObject = result.Converter.NotNull().Convert(target); + return convertedObject; + } + return BaseResult.NoConverter(result.Message); } } diff --git a/Sdk/Speckle.Converters.Common/ToHost/ConverterWithFallback.cs b/Sdk/Speckle.Converters.Common/ToHost/ConverterWithFallback.cs index f7730f598..94ee3dccd 100644 --- a/Sdk/Speckle.Converters.Common/ToHost/ConverterWithFallback.cs +++ b/Sdk/Speckle.Converters.Common/ToHost/ConverterWithFallback.cs @@ -1,55 +1,27 @@ using System.Collections; using Microsoft.Extensions.Logging; +using Speckle.Converters.Common.Registration; using Speckle.Sdk.Common.Exceptions; using Speckle.Sdk.Models; using Speckle.Sdk.Models.Extensions; namespace Speckle.Converters.Common.ToHost; -// POC: CNX-9394 Find a better home for this outside `DependencyInjection` project -/// -/// -///
-/// If no suitable converter conversion is found, and the target object has a displayValue property -/// a converter with strong name of is resolved for. -/// -/// -public sealed class ConverterWithFallback : IRootToHostConverter -{ - private readonly ILogger _logger; - private readonly ConverterWithoutFallback _baseConverter; - public ConverterWithFallback(ConverterWithoutFallback baseConverter, ILogger logger) - { - _logger = logger; - _baseConverter = baseConverter; - } - - /// - /// Converts a instance to a host object. - /// - /// The instance to convert. - /// The converted host object. - /// Fallbacks to display value if a direct conversion is not possible. - /// - /// The conversion is done in the following order of preference: - /// 1. Direct conversion using the . - /// 2. Fallback to display value using the method, if a direct conversion is not possible. - /// - /// If the direct conversion is not available and there is no displayValue, a is thrown. - /// - /// Thrown when no conversion is found for . - public object Convert(Base target) +public sealed class ConverterWithFallback(ConverterWithoutFallback baseConverter, ILogger logger) + : IRootToHostConverter +{ + public HostResult Convert(Base target) { Type type = target.GetType(); try { - return _baseConverter.Convert(target); + return baseConverter.Convert(target); } catch (ConversionNotSupportedException e) { - _logger.LogInformation(e, "Attempt to find conversion for type {type} failed", type); + logger.LogInformation(e, "Attempt to find conversion for type {type} failed", type); } // Fallback to display value if it exists. @@ -57,8 +29,7 @@ public object Convert(Base target) if (displayValue == null || (displayValue is IList && !displayValue.Any())) { - // TODO: I'm not sure if this should be a ConversionNotSupported instead, but it kinda mixes support + validation so I went for normal conversion exception - throw new ConversionException( + return HostResult.NoConversion( $"No direct conversion found for type {type} and it's fallback display value was null/empty" ); } @@ -66,15 +37,15 @@ public object Convert(Base target) return FallbackToDisplayValue(displayValue); // 1 - many mapping } - private object FallbackToDisplayValue(IReadOnlyList displayValue) + private HostResult FallbackToDisplayValue(IReadOnlyList displayValue) { var tempDisplayableObject = new DisplayableObject(displayValue); - var conversionResult = _baseConverter.Convert(tempDisplayableObject); + var conversionResult = baseConverter.Convert(tempDisplayableObject); // if the host app returns a list of objects as the result of the fallback conversion, we zip them together with the original base display value objects that generated them. - if (conversionResult is IEnumerable result) + if (conversionResult.Host is IEnumerable result) { - return result.Zip(displayValue, (a, b) => (a, b)); + return HostResult.Success(result.Zip(displayValue, (a, b) => (a, b))); } // if not, and the host app "merges" together somehow multiple display values into one entity, we return that. diff --git a/Sdk/Speckle.Converters.Common/ToHost/ConverterWithoutFallback.cs b/Sdk/Speckle.Converters.Common/ToHost/ConverterWithoutFallback.cs index 181eb8306..318b5e641 100644 --- a/Sdk/Speckle.Converters.Common/ToHost/ConverterWithoutFallback.cs +++ b/Sdk/Speckle.Converters.Common/ToHost/ConverterWithoutFallback.cs @@ -6,30 +6,17 @@ namespace Speckle.Converters.Common.ToHost; -// POC: CNX-9394 Find a better home for this outside `DependencyInjection` project -/// -/// Provides an implementation for -/// that resolves a via the injected -/// -/// -public sealed class ConverterWithoutFallback : IRootToHostConverter +public sealed class ConverterWithoutFallback( + IConverterManager converterResolver, + ILogger logger) + : IRootToHostConverter { - private readonly IConverterManager _toHost; - private readonly ILogger _logger; + private readonly ILogger _logger = logger; - public ConverterWithoutFallback( - IConverterManager converterResolver, - ILogger logger - ) + public HostResult Convert(Base target) { - _toHost = converterResolver; - _logger = logger; - } - - public object Convert(Base target) - { - var converter = _toHost.ResolveConverter(target.GetType()); - object result = converter.ConvertAndLog(target, _logger); + var converter = converterResolver.ResolveConverter(target.GetType()); + var result = converter.ConvertAndLog(target, _logger); return result; } }