Skip to content

Commit

Permalink
Handle future transport/persistence settings without crashing (#3792)
Browse files Browse the repository at this point in the history
* Return Unknown Transport when TransportInfo can't be found

* Return Unknown Persistence when persistence can't be found

* More explicit with string matching
  • Loading branch information
DavidBoike authored Nov 10, 2023
1 parent 7d4b956 commit d655832
Show file tree
Hide file tree
Showing 20 changed files with 75 additions and 65 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
{
using System;
using System.IO;
using System.Linq;
using System.Text.Json;
using NUnit.Framework;
using Particular.Approvals;
Expand Down Expand Up @@ -34,7 +33,7 @@ public void Should_write_expected_config_file()
var logPath = Path.Combine(Path.GetTempPath(), TestContext.CurrentContext.Test.ID, "log");

newInstance.InstallPath = installPath;
newInstance.TransportPackage = ServiceControlCoreTransports.All.Single(t => t.Name == TransportNames.MSMQ);
newInstance.TransportPackage = ServiceControlCoreTransports.Find(TransportNames.MSMQ);

newInstance.DBPath = dbPath;
newInstance.LogPath = logPath;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
namespace ServiceControl.Config.UI.InstanceAdd;

using System.Collections.Generic;
using System.Linq;
using System.Windows.Input;
using Framework.Rx;
using PropertyChanged;
Expand All @@ -12,7 +11,7 @@ public class ServiceControlEditorViewModel : RxProgressScreen
{
public ServiceControlEditorViewModel()
{
Transports = ServiceControlCoreTransports.All.Where(t => t.AvailableInSCMU);
Transports = ServiceControlCoreTransports.GetSupportedTransports();
ServiceControl = new ServiceControlInformation(this);
ServiceControlAudit = new ServiceControlAuditInformation(this);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,12 @@ public string Persister
return "InMemory";
}

return "RavenDB 3.5";
if (ServiceInstance.Version.Major <= 4)
{
return "RavenDB 3.5";
}

return "Unknown";
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,19 @@ namespace ServiceControl.Config.UI.SharedInstanceEditor
{
using System;
using System.Collections.Generic;
using System.Linq;
using System.Windows.Input;
using Framework.Rx;
using PropertyChanged;
using ServiceControl.Config.Extensions;
using ServiceControlInstaller.Engine.Instances;
using Validation;
using ServiceControl.Config.Extensions;
using Validations = Validation.Validations;

public class SharedMonitoringEditorViewModel : RxProgressScreen
{
public SharedMonitoringEditorViewModel()
{
Transports = ServiceControlCoreTransports.All.Where(t => t.AvailableInSCMU);
Transports = ServiceControlCoreTransports.GetSupportedTransports();
}

[DoNotNotify]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ protected override void ProcessRecord()
newAuditInstance.ForwardAuditMessages = ForwardAuditMessages.ToBool();
newAuditInstance.AuditRetentionPeriod = AuditRetentionPeriod;
newAuditInstance.ConnectionString = ConnectionString;
newAuditInstance.TransportPackage = ServiceControlCoreTransports.All.First(t => t.Matches(Transport));
newAuditInstance.TransportPackage = ServiceControlCoreTransports.Find(Transport);
newAuditInstance.SkipQueueCreation = SkipQueueCreation;
newAuditInstance.ServiceControlQueueAddress = ServiceControlQueueAddress;
newAuditInstance.EnableFullTextSearchOnBodies = EnableFullTextSearchOnBodies;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ protected override void ProcessRecord()
Port = Port,
ErrorQueue = ErrorQueue,
ConnectionString = ConnectionString,
TransportPackage = ServiceControlCoreTransports.All.First(t => t.Matches(Transport)),
TransportPackage = ServiceControlCoreTransports.Find(Transport),
SkipQueueCreation = SkipQueueCreation
};
var details = monitoringNewInstance;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ protected override void ProcessRecord()
ForwardErrorMessages = ForwardErrorMessages.ToBool(),
ErrorRetentionPeriod = ErrorRetentionPeriod,
ConnectionString = ConnectionString,
TransportPackage = ServiceControlCoreTransports.All.First(t => t.Matches(Transport)),
TransportPackage = ServiceControlCoreTransports.Find(Transport),
SkipQueueCreation = SkipQueueCreation,
EnableFullTextSearchOnBodies = EnableFullTextSearchOnBodies,
};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
namespace ServiceControl.Management.PowerShell
{
using System.Linq;
using System.Management.Automation;
using ServiceControlInstaller.Engine.Instances;

Expand All @@ -9,7 +8,7 @@ public class GetServiceControlTransportTypes : Cmdlet
{
protected override void ProcessRecord()
{
WriteObject(ServiceControlCoreTransports.All.Select(PsTransportInfo.FromTransport), true);
WriteObject(ServiceControlCoreTransports.Select(PsTransportInfo.FromTransport), true);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,10 @@
{
using System;
using System.IO;
using System.Linq;
using System.ServiceProcess;
using System.Xml;
using NUnit.Framework;
using ServiceControlInstaller.Engine.Configuration.ServiceControl;
using ServiceControlInstaller.Engine.FileSystem;
using ServiceControlInstaller.Engine.Instances;
using ServiceControlInstaller.Engine.Services;

Expand All @@ -20,7 +18,7 @@ public void Should_default_to_raven35_when_no_config_entry_exists()
var newInstance = ServiceControlAuditNewInstance.CreateWithPersistence(ZipFileFolder.FullName, "RavenDB35");

newInstance.InstallPath = InstallPath;
newInstance.TransportPackage = ServiceControlCoreTransports.All.Single(t => t.Name == TransportNames.MSMQ);
newInstance.TransportPackage = ServiceControlCoreTransports.Find(TransportNames.MSMQ);
newInstance.DBPath = DbPath;
newInstance.LogPath = LogPath;
newInstance.HostName = "localhost";
Expand Down Expand Up @@ -56,7 +54,7 @@ public void Should_update_existing_persister()
var newInstance = ServiceControlAuditNewInstance.CreateWithPersistence(ZipFileFolder.FullName, "RavenDB5");

newInstance.InstallPath = InstallPath;
newInstance.TransportPackage = ServiceControlCoreTransports.All.Single(t => t.Name == TransportNames.MSMQ);
newInstance.TransportPackage = ServiceControlCoreTransports.Find(TransportNames.MSMQ);

newInstance.DBPath = DbPath;
newInstance.LogPath = LogPath;
Expand Down Expand Up @@ -86,7 +84,7 @@ public void Should_remove_log_and_db_folders_on_uninstall()
var newInstance = ServiceControlAuditNewInstance.CreateWithPersistence(ZipFileFolder.FullName, "RavenDB35");

newInstance.InstallPath = InstallPath;
newInstance.TransportPackage = ServiceControlCoreTransports.All.Single(t => t.Name == TransportNames.MSMQ);
newInstance.TransportPackage = ServiceControlCoreTransports.Find(TransportNames.MSMQ);
newInstance.DBPath = DbPath;
newInstance.LogPath = LogPath;
newInstance.HostName = "localhost";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ public void Should_install_raven5_for_new_instances()
var newInstance = ServiceControlAuditNewInstance.CreateWithDefaultPersistence(ZipFileFolder.FullName);

newInstance.InstallPath = InstallPath;
newInstance.TransportPackage = ServiceControlCoreTransports.All.Single(t => t.Name == TransportNames.MSMQ);
newInstance.TransportPackage = ServiceControlCoreTransports.Find(TransportNames.MSMQ);

newInstance.DBPath = DbPath;
newInstance.LogPath = LogPath;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ public async Task CreateInstanceMSMQ()
AuditRetentionPeriod = TimeSpan.FromDays(SettingConstants.AuditRetentionPeriodDefaultInDaysForUI),
ErrorRetentionPeriod = TimeSpan.FromDays(SettingConstants.ErrorRetentionPeriodDefaultInDaysForUI),
ErrorQueue = "testerror",
TransportPackage = ServiceControlCoreTransports.All.First(t => t.Name == TransportNames.MSMQ),
TransportPackage = ServiceControlCoreTransports.Find(TransportNames.MSMQ),
ReportCard = new ReportCard(),
// but this fails for unit tests as the deploymentCache path is not used
// constructer of ServiceControlInstanceMetadata extracts version from zip
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ public class QueueValidationTests
public void Init()
{
var instanceA = new Mock<IServiceControlInstance>();
instanceA.SetupGet(p => p.TransportPackage).Returns(ServiceControlCoreTransports.All.First(t => t.Name == TransportNames.MSMQ));
instanceA.SetupGet(p => p.TransportPackage).Returns(ServiceControlCoreTransports.Find(TransportNames.MSMQ));
instanceA.SetupGet(p => p.ErrorQueue).Returns(@"error");
instanceA.SetupGet(p => p.ErrorLogQueue).Returns(@"errorlog");
instanceA.SetupGet(p => p.ForwardErrorMessages).Returns(true);

var instanceB = new Mock<IServiceControlInstance>();
instanceB.SetupGet(p => p.TransportPackage).Returns(ServiceControlCoreTransports.All.First(t => t.Name == TransportNames.RabbitMQClassicConventionalRoutingTopology || t.Name == TransportNames.RabbitMQQuorumConventionalRoutingTopology));
instanceB.SetupGet(p => p.TransportPackage).Returns(ServiceControlCoreTransports.Find(TransportNames.RabbitMQClassicConventionalRoutingTopology));
instanceB.SetupGet(p => p.ErrorQueue).Returns(@"RMQerror");
instanceB.SetupGet(p => p.ErrorLogQueue).Returns(@"RMQerrorlog");
instanceB.SetupGet(p => p.ForwardErrorMessages).Returns(true);
Expand All @@ -40,7 +40,7 @@ public void CheckQueueNamesAreUniqueShouldSucceed()
{
var newInstance = new ServiceControlNewInstance
{
TransportPackage = ServiceControlCoreTransports.All.First(t => t.Name == TransportNames.MSMQ),
TransportPackage = ServiceControlCoreTransports.Find(TransportNames.MSMQ),
ErrorLogQueue = "errorlog",
ErrorQueue = "error"
};
Expand All @@ -56,12 +56,12 @@ public void CheckQueueNamesAreUniqueShouldSucceed()
public void CheckChainingOfAuditQueues_ShouldSucceed()
{
var existingAudit = new Mock<IServiceControlAuditInstance>();
existingAudit.SetupGet(p => p.TransportPackage).Returns(ServiceControlCoreTransports.All.First(t => t.Name == TransportNames.MSMQ));
existingAudit.SetupGet(p => p.TransportPackage).Returns(ServiceControlCoreTransports.Find(TransportNames.MSMQ));
existingAudit.SetupGet(p => p.AuditQueue).Returns(@"audit");

var newInstance = ServiceControlAuditNewInstance.CreateWithDefaultPersistence(GetZipFolder().FullName);

newInstance.TransportPackage = ServiceControlCoreTransports.All.First(t => t.Name == TransportNames.MSMQ);
newInstance.TransportPackage = ServiceControlCoreTransports.Find(TransportNames.MSMQ);
newInstance.AuditQueue = "audit";

var validator = new QueueNameValidator(newInstance)
Expand All @@ -77,7 +77,7 @@ public void CheckQueueNamesAreUniqueShouldThrow()
{
var newInstance = new ServiceControlNewInstance
{
TransportPackage = ServiceControlCoreTransports.All.First(t => t.Name == TransportNames.MSMQ),
TransportPackage = ServiceControlCoreTransports.Find(TransportNames.MSMQ),
ErrorLogQueue = "error",
ErrorQueue = "error",
ForwardErrorMessages = true
Expand All @@ -97,7 +97,7 @@ public void CheckQueueNamesAreNotTakenByAnotherInstance_ShouldSucceed()
{
var newInstance = new ServiceControlNewInstance
{
TransportPackage = ServiceControlCoreTransports.All.First(t => t.Name == TransportNames.MSMQ),
TransportPackage = ServiceControlCoreTransports.Find(TransportNames.MSMQ),
ErrorLogQueue = "errorlog2",
ErrorQueue = "error2"
};
Expand All @@ -115,7 +115,7 @@ public void CheckQueueNamesAreNotTakenByAnotherInstance_ShouldThrow()
var expectedError = "Some queue names specified are already assigned to another ServiceControl instance - Correct the values for ErrorLogQueue, ErrorQueue";
var newInstance = new ServiceControlNewInstance
{
TransportPackage = ServiceControlCoreTransports.All.First(t => t.Name == TransportNames.MSMQ),
TransportPackage = ServiceControlCoreTransports.Find(TransportNames.MSMQ),
ErrorLogQueue = "errorlog",
ErrorQueue = "error",
ForwardErrorMessages = true
Expand Down Expand Up @@ -152,7 +152,7 @@ public void DuplicateQueueNamesAreAllowedOnDifferentTransports_ShouldNotThrow()

var newInstance = new ServiceControlNewInstance
{
TransportPackage = ServiceControlCoreTransports.All.First(t => t.Name == TransportNames.RabbitMQQuorumConventionalRoutingTopology),
TransportPackage = ServiceControlCoreTransports.Find(TransportNames.RabbitMQQuorumConventionalRoutingTopology),
ErrorLogQueue = "errorlog",
ErrorQueue = "error",
ForwardErrorMessages = true
Expand Down Expand Up @@ -186,7 +186,7 @@ public void EnsureDuplicateQueueNamesAreAllowedOnSameTransportWithDifferentConne
{
var newInstance = new ServiceControlNewInstance
{
TransportPackage = ServiceControlCoreTransports.All.First(t => t.Name == TransportNames.RabbitMQQuorumConventionalRoutingTopology),
TransportPackage = ServiceControlCoreTransports.Find(TransportNames.RabbitMQQuorumConventionalRoutingTopology),
ErrorQueue = "RMQerror",
ErrorLogQueue = "RMQerrorlog",
ConnectionString = "afakeconnectionstring",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,14 +97,9 @@ string ReadConnectionString()

TransportInfo DetermineTransportPackage()
{
var transportAppSetting = AppConfig.Read(SettingsList.TransportType, ServiceControlCoreTransports.All.Single(t => t.Default).TypeName).Trim();
var transport = ServiceControlCoreTransports.All.FirstOrDefault(p => p.Matches(transportAppSetting));
if (transport != null)
{
return transport;
}

return ServiceControlCoreTransports.All.First(p => p.Default);
var transportAppSetting = AppConfig.Read<string>(SettingsList.TransportType, null)?.Trim();
var transport = ServiceControlCoreTransports.Find(transportAppSetting);
return transport;
}

public async Task ValidateChanges()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
namespace ServiceControlInstaller.Engine.Instances
{
using System;
using System.Collections.Generic;

public class PersistenceManifest
Expand All @@ -24,5 +25,8 @@ public class Setting
public string DefaultValue { get; set; }
public bool Mandatory { get; set; }
}

public bool Matches(string name) => Name.Equals(name, StringComparison.OrdinalIgnoreCase)
|| TypeName.Equals(name, StringComparison.Ordinal);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ protected override AppConfig CreateAppConfig()

protected override string GetTransportTypeSetting()
{
return AppConfig.Read(AuditInstanceSettingsList.TransportType, ServiceControlCoreTransports.All.Single(t => t.Default).TypeName).Trim();
return AppConfig.Read<string>(AuditInstanceSettingsList.TransportType, null)?.Trim();
}

protected override string BaseServiceName => "ServiceControl.Audit";
Expand All @@ -82,18 +82,8 @@ public override void Reload()

var zipInfo = ServiceControlAuditZipInfo.Find(deploymentCachePath);

var manifests = ServiceControlAuditPersisters.LoadAllManifests(zipInfo.FilePath);

var persistenceType = AppConfig.Read<string>(AuditInstanceSettingsList.PersistenceType, null);

if (string.IsNullOrEmpty(persistenceType))
{
PersistenceManifest = manifests.Single(m => m.Name == "RavenDB35");
}
else
{
PersistenceManifest = manifests.Single(m => m.TypeName == persistenceType);
}
PersistenceManifest = ServiceControlAuditPersisters.GetPersistence(zipInfo.FilePath, persistenceType);

TransportPackage = DetermineTransportPackage();
ConnectionString = ReadConnectionString();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
{
using System;
using System.IO;
using System.Linq;
using System.Reflection;
using System.Xml;
using System.Xml.Serialization;
Expand All @@ -27,8 +26,7 @@ public static ServiceControlAuditNewInstance CreateWithDefaultPersistence(string
public static ServiceControlAuditNewInstance CreateWithPersistence(string deploymentCachePath, string persistence)
{
var zipInfo = ServiceControlAuditZipInfo.Find(deploymentCachePath);
var persistenceManifest = ServiceControlAuditPersisters.LoadAllManifests(zipInfo.FilePath)
.Single(manifest => manifest.Name == persistence);
var persistenceManifest = ServiceControlAuditPersisters.GetPersistence(zipInfo.FilePath, persistence);

return new ServiceControlAuditNewInstance(zipInfo.Version, persistenceManifest);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
using System.Linq;
using Newtonsoft.Json;

public class ServiceControlAuditPersisters
public static class ServiceControlAuditPersisters
{
public static IReadOnlyList<PersistenceManifest> LoadAllManifests(string zipFilePath)
internal static IReadOnlyList<PersistenceManifest> LoadAllManifests(string zipFilePath)
{
using (var zipArchive = ZipFile.OpenRead(zipFilePath))
{
Expand All @@ -31,5 +31,22 @@ public static IReadOnlyList<PersistenceManifest> LoadAllManifests(string zipFile
return manifests;
}
}

public static PersistenceManifest GetPersistence(string zipFilePath, string name)
{
var manifests = LoadAllManifests(zipFilePath);

if (string.IsNullOrEmpty(name))
{
// Must always remain RavenDB35 so that SCMU understands that an instance with no configured value is an old Raven 3.5 instance
return manifests.Single(m => m.Name == "RavenDB35");
}

return manifests.FirstOrDefault(m => m.Matches(name)) ?? new PersistenceManifest
{
Name = $"Unknown Persistence: {name}",
Description = $"Unknown Persistence {name} may be from a future version of ServiceControl"
};
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -145,13 +145,8 @@ protected string ReadConnectionString()
protected TransportInfo DetermineTransportPackage()
{
var transportAppSetting = GetTransportTypeSetting();
var transport = ServiceControlCoreTransports.All.FirstOrDefault(p => p.Matches(transportAppSetting));
if (transport != null)
{
return transport;
}

return ServiceControlCoreTransports.All.First(p => p.Default);
var transport = ServiceControlCoreTransports.Find(transportAppSetting);
return transport;
}

protected void RecreateUrlAcl(ServiceControlBaseService oldSettings)
Expand Down
Loading

0 comments on commit d655832

Please sign in to comment.