diff --git a/src/EtabSharp/Core/ETABSApplication.cs b/src/EtabSharp/Core/ETABSApplication.cs index d3242ce..73883e2 100644 --- a/src/EtabSharp/Core/ETABSApplication.cs +++ b/src/EtabSharp/Core/ETABSApplication.cs @@ -5,6 +5,7 @@ using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using System.Runtime.InteropServices; +using System.Runtime.Versioning; namespace EtabSharp.Core; @@ -176,9 +177,23 @@ public void Close(bool savePrompt = false) } /// - /// Disposes this wrapper. Calls ApplicationExit(false) — does NOT save. - /// For Mode A (attach) flows, do NOT dispose — release COM manually via ComCleanup. - /// For Mode B (hidden) flows, disposing is correct and will exit the hidden instance. + /// Disposes this wrapper by releasing the COM references it holds. It does not + /// call ApplicationExit and never shuts ETABS down. + /// + /// Attached / external session (, + /// ): disposing on its own is the correct + /// and complete cleanup. The user's ETABS stays running, which is the point — the + /// session was never ours to end. + /// + /// Caller-owned session being shut down (a hidden instance from + /// , or a handle wrapped with + /// ): request the authoritative + /// app.Application.ApplicationExit(false) and resolve the process exit first, + /// then dispose. Dispose is never a substitute for that exit — it releases references, + /// not the application — and CSI documents that the cSapModel reference should + /// be dropped after ApplicationExit. + /// + /// Safe to call more than once. /// public void Dispose() { @@ -187,27 +202,15 @@ public void Dispose() // Only attempt COM cleanup on Windows platforms where Marshal.ReleaseComObject is supported. // This prevents CA1416 diagnostics and avoids calling Windows-only runtime APIs on other platforms. - if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows)) + if (OperatingSystem.IsWindows()) { - try - { - // Release child managers first (reverse init order) - if (_model.IsValueCreated) - Marshal.ReleaseComObject(_model.Value); - - if (_application.IsValueCreated) - Marshal.ReleaseComObject(_application.Value); - - // Release raw COM refs last (parents after children) - Marshal.ReleaseComObject(_sapModel); - Marshal.ReleaseComObject(_api); - - _logger.LogInformation("COM references released"); - } - catch (Exception ex) - { - _logger.LogWarning(ex, "Error releasing COM objects: {Message}", ex.Message); - } + // Only _sapModel and _api are real COM references. _model and _application are + // ordinary managed wrappers around them, and Marshal.ReleaseComObject rejects a + // non-COM object — releasing them here used to throw and, because every release + // shared one try block, could skip the two releases that actually matter. + // Each real reference is now released independently. + ReleaseComReference(_sapModel, nameof(SapModel)); + ReleaseComReference(_api, "cOAPI"); } else { @@ -217,6 +220,20 @@ public void Dispose() GC.SuppressFinalize(this); } + [SupportedOSPlatform("windows")] + private void ReleaseComReference(object comReference, string name) + { + try + { + Marshal.ReleaseComObject(comReference); + _logger.LogInformation("COM reference released: {Reference}", name); + } + catch (Exception ex) + { + _logger.LogWarning(ex, "Error releasing COM reference {Reference}: {Message}", name, ex.Message); + } + } + #region Advanced / Raw Access /// diff --git a/src/EtabSharp/Core/ETABSWrapper.cs b/src/EtabSharp/Core/ETABSWrapper.cs index ed0999f..ee6fa91 100644 --- a/src/EtabSharp/Core/ETABSWrapper.cs +++ b/src/EtabSharp/Core/ETABSWrapper.cs @@ -9,9 +9,10 @@ namespace EtabSharp.Core; /// Factory for creating and connecting to ETABS v22+ instances. /// Returns ETABSApplication — the single entry point for all ETABS interaction. /// -/// Two usage patterns: -/// ETABSWrapper.Connect() — Mode A: attach to user's running ETABS -/// ETABSWrapper.CreateNew() — Mode B: start a new hidden instance +/// Three usage patterns: +/// ETABSWrapper.Connect() — Mode A: attach to the user's running ETABS +/// ETABSWrapper.CreateNew() — Mode B: start a new hidden instance +/// ETABSWrapper.WrapExisting() — Mode C: wrap a cOAPI the caller created and started itself /// public static class ETABSWrapper { @@ -21,6 +22,85 @@ public static class ETABSWrapper #region Public Factory Methods + /// + /// Mode C: wraps a the caller has already created and + /// already started, without performing any lifecycle action of its own. + /// + /// This is deliberately low level. It exists for callers that must own the raw + /// CSI lifecycle and the exact OS process identity themselves — creating the object + /// with cHelper.CreateObject(path), checking cOAPI.ApplicationStart(), + /// proving which process they own, and calling cSapModel.InitializeNewModel() — + /// and only then want EtabSharp's model and domain abstractions over that exact + /// instance. Callers that do not need that control should use + /// or instead. + /// + /// This method performs no lifecycle or discovery work: it does not create + /// an object, does not call ApplicationStart, does not attach through + /// GetObject/GetObjectProcess or the ROT, does not enumerate or select + /// ETABS processes, and never calls Hide, Unhide or ApplicationExit. + /// The only member it touches on is SapModel. + /// + /// Ownership: the caller owns the application lifecycle. Wrapping does not + /// transfer it, and disposing the returned wrapper releases COM references only — it + /// never exits ETABS. When the caller is shutting that session down, it must request + /// the authoritative ApplicationExit(false) and resolve the process exit itself, + /// and dispose afterwards. + /// + /// + /// An existing, already-started API object. The exact instance is preserved; nothing + /// is created or re-attached on the caller's behalf. + /// + /// ETABS major version of the running instance (22 or newer). + /// OAPI version reported by that instance; 0 if the caller could not read it. + /// Full ETABS version string of the running instance, e.g. "23.3.0". + /// Optional logger for diagnostics. + /// is null. + /// is null or blank. + /// + /// is below the minimum supported version, or + /// is negative. + /// + /// + /// exposes no SapModel. An object that was never started + /// is not usable, and this method will not start it. + /// + public static ETABSApplication WrapExisting( + ETABSv1.cOAPI api, + int majorVersion, + double apiVersion, + string fullVersion, + ILogger? logger = null) + { + ArgumentNullException.ThrowIfNull(api); + + if (string.IsNullOrWhiteSpace(fullVersion)) + { + throw new ArgumentException( + "Full version must be supplied by the caller; wrapping does not discover it.", + nameof(fullVersion)); + } + + if (majorVersion < MINIMUM_SUPPORTED_VERSION) + { + throw new ArgumentOutOfRangeException( + nameof(majorVersion), + majorVersion, + $"ETABS v{MINIMUM_SUPPORTED_VERSION} is the minimum supported version."); + } + + if (apiVersion < 0) + { + throw new ArgumentOutOfRangeException( + nameof(apiVersion), + apiVersion, + "OAPI version cannot be negative."); + } + + // Unlike Connect/CreateNew this returns non-null or throws: the caller already holds + // a handle it proved, so a silent null would discard the reason wrapping failed. + return new ETABSApplication(api, majorVersion, apiVersion, fullVersion, logger); + } + /// /// Mode A: Connects to the currently running ETABS instance (v22+). /// Does NOT call ApplicationStart or Hide — attaches to whatever ETABS the user has open. diff --git a/test/EtabSharp.Test/EtabSharp.Test.csproj b/test/EtabSharp.Test/EtabSharp.Test.csproj index 55fa735..1ba283c 100644 --- a/test/EtabSharp.Test/EtabSharp.Test.csproj +++ b/test/EtabSharp.Test/EtabSharp.Test.csproj @@ -5,7 +5,7 @@ enable Exe EtabSharp.Test - net10.0 + net8.0;net10.0 false true @@ -40,8 +40,19 @@ + - C:\Program Files\Computers and Structures\ETABS 22\ETABSv1.dll + $(MSBuildThisFileDirectory)..\..\lib\ETABSv1.dll + C:\Program Files\Computers and Structures\ETABS 24\ETABSv1.dll + C:\Program Files (x86)\Computers and Structures\ETABS 24\ETABSv1.dll + C:\Program Files\Computers and Structures\ETABS 23\ETABSv1.dll + C:\Program Files (x86)\Computers and Structures\ETABS 23\ETABSv1.dll + C:\Program Files\Computers and Structures\ETABS 22\ETABSv1.dll C:\Program Files (x86)\Computers and Structures\ETABS 22\ETABSv1.dll diff --git a/test/EtabSharp.Test/wrap_existing_test.cs b/test/EtabSharp.Test/wrap_existing_test.cs new file mode 100644 index 0000000..c80c091 --- /dev/null +++ b/test/EtabSharp.Test/wrap_existing_test.cs @@ -0,0 +1,273 @@ +using EtabSharp.Core; +using ETABSv1; +using Microsoft.Extensions.Logging; +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.Reflection; +using Xunit; + +namespace EtabSharp.Test; + +// ───────────────────────────────────────────────────────────── +// ETABSWrapper.WrapExisting — Mode C (pure wrapping) +// +// These tests are OFFLINE. They never start, attach to, or exit ETABS: the +// cOAPI passed in is a recording proxy, so every member the factory touches is +// observable. That is the point of the suite — the contract is as much about +// what WrapExisting must NOT do as what it returns. +// ───────────────────────────────────────────────────────────── + +[Trait("Category", "Offline")] +public class WrapExistingTests +{ + private const int MajorVersion = 23; + private const double ApiVersion = 1.0; + private const string FullVersion = "23.3.0"; + + private static readonly string[] LifecycleMembers = + [ + "ApplicationStart", + "ApplicationExit", + "Hide", + "Unhide", + "Visible", + "SetAsActiveObject", + "UnsetAsActiveObject", + "InternalExec", + "GetOAPIVersionNumber" + ]; + + [Fact] + public void WrapExisting_PreservesTheExactObjectsSupplied() + { + var (api, recorder, sapModel) = CreateRecordingApi(); + + var app = ETABSWrapper.WrapExisting(api, MajorVersion, ApiVersion, FullVersion); + + // Same cSapModel instance — not a re-read from some other application object. + Assert.Same(sapModel, app.SapModel); + Assert.Equal(MajorVersion, app.MajorVersion); + Assert.Equal(ApiVersion, app.ApiVersion); + Assert.Equal(FullVersion, app.FullVersion); + Assert.Single(recorder.Calls, call => call == "get_SapModel"); + } + + [Fact] + public void WrapExisting_TouchesNoLifecycleMemberAndStartsNoProcess() + { + var (api, recorder, _) = CreateRecordingApi(); + var etabsBefore = Process.GetProcessesByName("ETABS").Length; + + _ = ETABSWrapper.WrapExisting(api, MajorVersion, ApiVersion, FullVersion); + + // The only member the factory may touch is SapModel. + Assert.Equal(["get_SapModel"], recorder.Calls); + foreach (var member in LifecycleMembers) + { + Assert.DoesNotContain(recorder.Calls, call => call.Contains(member, StringComparison.Ordinal)); + } + + // No creation, no attach, no ROT, no process selection. + Assert.Equal(etabsBefore, Process.GetProcessesByName("ETABS").Length); + } + + [Fact] + public void WrapExisting_DisposeReleasesReferencesWithoutExitingEtabs() + { + var (api, recorder, _) = CreateRecordingApi(); + var app = ETABSWrapper.WrapExisting(api, MajorVersion, ApiVersion, FullVersion); + + app.Dispose(); + + // Dispose is COM cleanup only. It is never a substitute for an authoritative + // ApplicationExit(false), and must not issue one behind the caller's back. + Assert.DoesNotContain("ApplicationExit", recorder.Calls); + Assert.Equal(["get_SapModel"], recorder.Calls); + } + + // The managed wrappers are not COM RCWs. Releasing them used to throw inside the one + // shared try block and could skip the two references that actually are COM. + [Fact] + public void WrapExisting_DisposeWithMaterializedWrappersStillAttemptsBothComReleases() + { + var (api, recorder, _) = CreateRecordingApi(); + var log = new RecordingLogger(); + var app = ETABSWrapper.WrapExisting(api, MajorVersion, ApiVersion, FullVersion, log); + + // Materialize both lazy managed wrappers before disposing. + Assert.NotNull(app.Model); + Assert.NotNull(app.Application); + Assert.Equal(["get_SapModel"], recorder.Calls); + + app.Dispose(); + + // Each real COM reference is attempted independently: with a non-COM test double + // both releases fail, and both failures must be recorded — one skipping the other + // is exactly the defect being fixed. + var releaseEntries = log.Entries.FindAll(entry => entry.Contains("COM reference", StringComparison.Ordinal)); + Assert.Equal(2, releaseEntries.Count); + Assert.Contains(releaseEntries, entry => entry.Contains("SapModel", StringComparison.Ordinal)); + Assert.Contains(releaseEntries, entry => entry.Contains("cOAPI", StringComparison.Ordinal)); + + // Still no lifecycle call, even with both wrappers materialized. + Assert.Equal(["get_SapModel"], recorder.Calls); + foreach (var member in LifecycleMembers) + { + Assert.DoesNotContain(recorder.Calls, call => call.Contains(member, StringComparison.Ordinal)); + } + } + + [Fact] + public void WrapExisting_DoubleDisposeIsSafeAndDoesNothingTheSecondTime() + { + var (api, recorder, _) = CreateRecordingApi(); + var log = new RecordingLogger(); + var app = ETABSWrapper.WrapExisting(api, MajorVersion, ApiVersion, FullVersion, log); + _ = app.Model; + _ = app.Application; + + app.Dispose(); + var afterFirst = log.Entries.Count; + app.Dispose(); + + Assert.Equal(afterFirst, log.Entries.Count); + Assert.Equal(["get_SapModel"], recorder.Calls); + } + + [Fact] + public void WrapExisting_IsIdempotentAcrossCallsAndNeverSubstitutesAnotherInstance() + { + var (firstApi, _, firstSapModel) = CreateRecordingApi(); + var (secondApi, _, secondSapModel) = CreateRecordingApi(); + + var first = ETABSWrapper.WrapExisting(firstApi, MajorVersion, ApiVersion, FullVersion); + var second = ETABSWrapper.WrapExisting(secondApi, MajorVersion, ApiVersion, FullVersion); + + Assert.Same(firstSapModel, first.SapModel); + Assert.Same(secondSapModel, second.SapModel); + Assert.NotSame(first.SapModel, second.SapModel); + } + + [Fact] + public void WrapExisting_RejectsANullApi() + { + var error = Assert.Throws( + () => ETABSWrapper.WrapExisting(null!, MajorVersion, ApiVersion, FullVersion)); + + Assert.Equal("api", error.ParamName); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public void WrapExisting_RejectsMissingFullVersion(string? fullVersion) + { + var (api, recorder, _) = CreateRecordingApi(); + + var error = Assert.Throws( + () => ETABSWrapper.WrapExisting(api, MajorVersion, ApiVersion, fullVersion!)); + + Assert.Equal("fullVersion", error.ParamName); + // Rejected before anything was read from the caller's object. + Assert.Empty(recorder.Calls); + } + + [Theory] + [InlineData(0)] + [InlineData(21)] + public void WrapExisting_RejectsUnsupportedMajorVersion(int majorVersion) + { + var (api, recorder, _) = CreateRecordingApi(); + + var error = Assert.Throws( + () => ETABSWrapper.WrapExisting(api, majorVersion, ApiVersion, FullVersion)); + + Assert.Equal("majorVersion", error.ParamName); + Assert.Empty(recorder.Calls); + } + + [Fact] + public void WrapExisting_RejectsNegativeApiVersion() + { + var (api, recorder, _) = CreateRecordingApi(); + + var error = Assert.Throws( + () => ETABSWrapper.WrapExisting(api, MajorVersion, -1.0, FullVersion)); + + Assert.Equal("apiVersion", error.ParamName); + Assert.Empty(recorder.Calls); + } + + [Fact] + public void WrapExisting_RejectsAnApiWithNoSapModel() + { + var (api, recorder, _) = CreateRecordingApi(withSapModel: false); + + // An object that was never started has no usable model, and WrapExisting will not + // start it — that stays the caller's job. + Assert.Throws( + () => ETABSWrapper.WrapExisting(api, MajorVersion, ApiVersion, FullVersion)); + Assert.Equal(["get_SapModel"], recorder.Calls); + } + + private static (cOAPI Api, CallRecorder Recorder, cSapModel? SapModel) CreateRecordingApi( + bool withSapModel = true) + { + var sapModel = withSapModel ? CreateProxy() : null; + var api = CreateProxy(); + var recorder = ((RecordingProxy)(object)api).Recorder; + recorder.Returns["get_SapModel"] = sapModel; + return (api, recorder, sapModel); + } + + private static T CreateProxy() where T : class => + DispatchProxy.Create(); + + /// Captures log entries so per-reference release attempts are observable. + private sealed class RecordingLogger : ILogger + { + public List Entries { get; } = []; + + public IDisposable? BeginScope(TState state) where TState : notnull => null; + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log( + LogLevel logLevel, + EventId eventId, + TState state, + Exception? exception, + Func formatter) => + Entries.Add(formatter(state, exception)); + } + + /// Records every member invocation so "did not touch" is provable, not assumed. + public sealed class CallRecorder + { + public List Calls { get; } = []; + public Dictionary Returns { get; } = []; + } + + public class RecordingProxy : DispatchProxy + { + public CallRecorder Recorder { get; } = new(); + + protected override object? Invoke(MethodInfo? targetMethod, object?[]? args) + { + var name = targetMethod?.Name ?? ""; + Recorder.Calls.Add(name); + + if (Recorder.Returns.TryGetValue(name, out var configured)) + { + return configured; + } + + var returnType = targetMethod?.ReturnType; + return returnType is null || returnType == typeof(void) || !returnType.IsValueType + ? null + : Activator.CreateInstance(returnType); + } + } +}