diff --git a/c-sharp-tests/DummyPapiClient.cs b/c-sharp-tests/DummyPapiClient.cs index 18d65acf83..e2f44c641b 100644 --- a/c-sharp-tests/DummyPapiClient.cs +++ b/c-sharp-tests/DummyPapiClient.cs @@ -4,6 +4,7 @@ using Paranext.DataProvider.MessageHandlers; using Paranext.DataProvider.Messages; using Paranext.DataProvider.MessageTransports; +using Paranext.DataProvider.Projects; namespace TestParanextDataProvider { @@ -11,6 +12,7 @@ namespace TestParanextDataProvider internal class DummyPapiClient : PapiClient { private readonly Dictionary> _eventHandlers = new(); + private readonly Dictionary> _validSettings = new(); public Stack EventMessages { get; } = new(); @@ -30,6 +32,16 @@ public IEnumerable FakeMessageFromServer(Message message) return handler.HandleMessage(message); } + public void AddSettingValueToTreatAsValid(string pbSettingName, string newValue, string oldValue) + { + if (!_validSettings.TryGetValue(pbSettingName, out var values)) + { + _validSettings[pbSettingName] = values = + new List<(string newValue, string oldValue)>(); + } + values.Add((newValue, oldValue)); + } + #region Overrides of PapiClient public override Task ConnectAsync() { @@ -50,7 +62,14 @@ public override Task RegisterRequestHandler( { var responder = (MessageHandlerRequestByRequestType) _messageHandlersByMessageType[MessageType.REQUEST]; - responder.SetHandlerForRequestType(requestType, requestHandler); + try + { + responder.SetHandlerForRequestType(requestType, requestHandler); + } + catch (ArgumentException) + { + return Task.FromResult(false); + } return Task.FromResult(true); } @@ -71,6 +90,98 @@ public override void SendEvent(MessageEvent message) Message? result = handler(message); EventMessages.Push(result); } + + public override void SendRequest(string requestType, object requestContents, + Action responseCallback) + { + Task.Delay(1).ContinueWith(async _ => + { + bool success = false; + object? result = null; + + if ((requestType == "object:ProjectSettingsService.function") && + (requestContents is string[] details) && + (details.Length > 2)) + { + // If this is a setting known to both Platform.Bible and Paratext, do the + // translation from the Paratext settings key to the PB setting id. + // If it's a custom settings key, then perhaps it's one we've registered to + // handle, and no translation is needed. + var ourSettingName = details[1]; + var pbSettingName = ProjectSettings + .GetPlatformBibleSettingNameFromParatextSettingName(ourSettingName); + + if (ourSettingName != null) + { + switch (details[0]) + { + case "isValid": + if (details.Length == 6 && details[4] == ProjectType.Paratext) + { + success = true; + // Might be a setting we've registered to handle. + var isValidRequestContents = new [] + {details[2], details[3], details[5]}; + if (TryValidationUsingRegisteredHandler(isValidRequestContents, + ourSettingName, out var isValid)) + { + result = isValid; + } + else if (pbSettingName == null) + { + // Per comment in isValid (in + // project-settings.service-host.ts), if there is no + // validator just let the change go through + result = true; + } + else + { + result = _validSettings.TryGetValue(pbSettingName, + out var validValues) && validValues.Any(vv => + vv.newValue == details[2] && + vv.oldValue == details[3]); + } + } + break; + case "getDefault": + if (details.Length == 3 && + details[2] == ProjectType.Paratext && + pbSettingName != null) + { + success = true; + result = $"default value for {pbSettingName}"; + } + break; + default: + break; + } + } + } + + await Task.Run(() => responseCallback(success, + JsonSerializer.SerializeToElement(result))); + }); + } + + private bool TryValidationUsingRegisteredHandler(string[] requestContents, string settingName, + out bool isValid) + { + isValid = true; + if (!_messageHandlersByMessageType.TryGetValue( + MessageType.REQUEST, out var responder)) + return false; + + var msgRequest = new MessageRequest(ProjectSettingsService.GetValidatorKey(settingName), + GetRequestId(), requestContents); + var responseMsg = responder.HandleMessage(msgRequest).OfType() + .FirstOrDefault(); + + if (responseMsg == null) + return false; + + isValid = responseMsg.Success; + return true; + } #endregion } } diff --git a/c-sharp-tests/ProjectSettingsServicesTests.cs b/c-sharp-tests/ProjectSettingsServicesTests.cs new file mode 100644 index 0000000000..c6cacb616d --- /dev/null +++ b/c-sharp-tests/ProjectSettingsServicesTests.cs @@ -0,0 +1,142 @@ +using System.Diagnostics.CodeAnalysis; +using Newtonsoft.Json; +using TestParanextDataProvider; + +namespace Paranext.DataProvider.Projects.Tests +{ + [ExcludeFromCodeCoverage] + public class ProjectSettingsServicesTests + { + [Test] + public void IsValid_ValidLanguage_ReturnsTrue() + { + DummyPapiClient papiClient = new DummyPapiClient(); + var newValueJson = JsonConvert.SerializeObject("Spanish"); + var currentValueJson = JsonConvert.SerializeObject("German"); + papiClient.AddSettingValueToTreatAsValid(ProjectSettings.PB_LANGUAGE, + newValueJson, currentValueJson); + var result = ProjectSettingsService.IsValid(papiClient, newValueJson, + currentValueJson, ProjectSettings.PT_LANGUAGE, "", + ProjectType.Paratext); + + Assert.That(result, Is.True); + } + + [Test] + public void IsValid_InvalidSetting_ReturnsFalse() + { + DummyPapiClient papiClient = new DummyPapiClient(); + var newValueJson = JsonConvert.SerializeObject("Spanish"); + var currentValueJson = JsonConvert.SerializeObject("German"); + var result = ProjectSettingsService.IsValid(papiClient, newValueJson, + currentValueJson, ProjectSettings.PT_LANGUAGE, "", + ProjectType.Paratext); + + Assert.That(result, Is.False); + } + + [Test] + public void IsValid_UnexpectedProjectType_ReturnsFalse() + { + DummyPapiClient papiClient = new DummyPapiClient(); + var newValueJson = JsonConvert.SerializeObject("Spanish"); + var currentValueJson = JsonConvert.SerializeObject("German"); + papiClient.AddSettingValueToTreatAsValid(ProjectSettings.PB_LANGUAGE, + newValueJson, currentValueJson); + var result = ProjectSettingsService.IsValid(papiClient, newValueJson, + currentValueJson, ProjectSettings.PT_LANGUAGE, "", + "SomethingElse"); + + Assert.That(result, Is.False); + } + + [Test] + public void GetDefault_KnownProperty_ReturnsDefaultValue() + { + DummyPapiClient papiClient = new DummyPapiClient(); + var result = ProjectSettingsService.GetDefault(papiClient, ProjectSettings.PT_LANGUAGE, + ProjectType.Paratext); + + Assert.That(result, Is.EqualTo($"default value for {ProjectSettings.PB_LANGUAGE}")); + } + + [Test] + public void GetDefault_UnknownProperty_ReturnsNull() + { + DummyPapiClient papiClient = new DummyPapiClient(); + var result = ProjectSettingsService.GetDefault(papiClient, "wonky.setting", + ProjectType.Paratext); + + Assert.That(result, Is.Null); + } + + [Test] + public void RegisterValidator_NewProperty_ReturnsTrue() + { + const string settingKey = "testScripture.MonkeyCount"; + (bool result, string? error) MonkeyCountValidator((string newValueJson, string currentValueJson, + string allChangesJson, string projectType) data) + { + try + { + var value = JsonConvert.DeserializeObject(data.newValueJson); + var result = true; + string? error = null; + if (value == null) + { + result = false; + error = "New value must be an integer. It cannot be null"; + } + else if ((long)value <= 4) + { + result = false; + error = "Mama called the doctor, and the doctor said, you must have at" + + "least 4 monkeys jumping on the bed!"; + } + return (result, error); + } + catch (Exception ex) + { + return (false, ex.Message); + } + } + + DummyPapiClient papiClient = new DummyPapiClient(); + + // Before registering the validator, IsValid should return true. + Assert.That(ProjectSettingsService.IsValid(papiClient, JsonConvert.SerializeObject(2), + JsonConvert.SerializeObject(5), settingKey, "", ProjectType.Paratext), Is.True); + + var result = ProjectSettingsService.RegisterValidator(papiClient, + settingKey, MonkeyCountValidator); + + Assert.That(result, Is.EqualTo(true)); + + Assert.That(ProjectSettingsService.IsValid(papiClient, JsonConvert.SerializeObject(1), + JsonConvert.SerializeObject(5), settingKey, "", ProjectType.Paratext), Is.False); + Assert.That(ProjectSettingsService.IsValid(papiClient, JsonConvert.SerializeObject(6), + JsonConvert.SerializeObject(5), settingKey, "", ProjectType.Paratext), Is.True); + } + + [Test] + public void RegisterValidator_ExistingProperty_ReturnsFalse() + { + const string settingKey = "testScripture.Oops"; + (bool result, string? error) OopsValidator((string newValueJson, string currentValueJson, + string allChangesJson, string projectType) data) + { + return (false, "The Oops property has no valid values. Ha Ha!"); + } + + DummyPapiClient papiClient = new DummyPapiClient(); + + var result = ProjectSettingsService.RegisterValidator(papiClient, + settingKey, OopsValidator); + + Assert.That(result, Is.EqualTo(true)); + + Assert.That(ProjectSettingsService.RegisterValidator(papiClient, + settingKey, OopsValidator), Is.EqualTo(false)); + } + } +} diff --git a/c-sharp-tests/Projects/ParatextDataProviderTests.cs b/c-sharp-tests/Projects/ParatextDataProviderTests.cs index 7263260238..7602c1b86d 100644 --- a/c-sharp-tests/Projects/ParatextDataProviderTests.cs +++ b/c-sharp-tests/Projects/ParatextDataProviderTests.cs @@ -1,11 +1,14 @@ using System.Diagnostics.CodeAnalysis; using System.Text.Json; using System.Text.Json.Nodes; +using Newtonsoft.Json; using Paranext.DataProvider.MessageHandlers; using Paranext.DataProvider.Messages; using Paranext.DataProvider.Projects; using Paratext.Data; +using Paratext.Data.ProjectSettingsAccess; using SIL.Scripture; +using static Paranext.DataProvider.Projects.ParatextProjectDataProvider; namespace TestParanextDataProvider.Projects { @@ -246,14 +249,14 @@ string expectedResult null, requestType, requesterId, - ParatextProjectDataProvider.AllScriptureDataTypes + AllScriptureDataTypes ); // Verify an update event was sent out properly Assert.That(updateEvents.Count, Is.EqualTo(1)); Assert.That( updateEvents[0].Event, - Is.EqualTo(ParatextProjectDataProvider.AllScriptureDataTypes) + Is.EqualTo(AllScriptureDataTypes) ); // Verify the new text was saved to disk @@ -340,14 +343,14 @@ string expectedResult null, requestType, requesterId, - ParatextProjectDataProvider.AllScriptureDataTypes + AllScriptureDataTypes ); // Verify an update event was sent out properly Assert.That(updateEvents.Count, Is.EqualTo(1)); Assert.That( updateEvents[0].Event, - Is.EqualTo(ParatextProjectDataProvider.AllScriptureDataTypes) + Is.EqualTo(AllScriptureDataTypes) ); // Verify the new text was saved to disk @@ -473,5 +476,43 @@ public async Task SetAndGetExtensionData_SavesAndGetsData() VerifyResponse(result2, null, requestType, requesterId, "Random file contents"); } + + /// + /// Tests that the ParatextProjectDataProvider has successfully registered a validator for + /// the Validity property and that the validator is called to determine that the new value + /// for that property is indeed valid. + /// + [Test] + public async Task SetProjectSetting_ValidVisibility_Succeeds() + { + DummyParatextProjectDataProvider provider = + new(PdpName, Client, _projectDetails, ParatextProjects); + await provider.RegisterDataProvider(); + + var result = provider.SetProjectSetting( + JsonConvert.SerializeObject(VisibilitySettingName), + JsonConvert.SerializeObject(ProjectVisibility.Public.ToString())); + + Assert.That(result.Success, Is.True); + } + + /// + /// Tests that the ParatextProjectDataProvider has successfully registered a validator for + /// the Validity property and that the validator is called to determine that the new value + /// for that property is indeed invalid. + /// + [Test] + public async Task SetProjectSetting_InvalidVisibility_DoesNotSucceed() + { + DummyParatextProjectDataProvider provider = + new(PdpName, Client, _projectDetails, ParatextProjects); + await provider.RegisterDataProvider(); + + var result = provider.SetProjectSetting( + JsonConvert.SerializeObject(VisibilitySettingName), + JsonConvert.SerializeObject(89)); + + Assert.That(result.Success, Is.False); + } } } diff --git a/c-sharp/MessageHandlers/MessageHandlerRequestByRequestType.cs b/c-sharp/MessageHandlers/MessageHandlerRequestByRequestType.cs index b3a8ab338d..aaf1a30221 100644 --- a/c-sharp/MessageHandlers/MessageHandlerRequestByRequestType.cs +++ b/c-sharp/MessageHandlers/MessageHandlerRequestByRequestType.cs @@ -13,9 +13,21 @@ internal class MessageHandlerRequestByRequestType : IMessageHandler { private readonly ConcurrentDictionary _handlersByRequestType = new(); + /// + /// Sets the handler to be called in response to the given request type + /// + /// Attempt to set handler for a request type that already + /// has a handler. + /// This is intended to be called only by PapiClient.RegisterRequestHandler. In + /// production, the argument exception should therefore be impossible, since the server should + /// be checking to prevent duplicates. public void SetHandlerForRequestType(string requestType, RequestHandler handler) { - _handlersByRequestType[requestType] = handler; + if (!_handlersByRequestType.TryAdd(requestType, handler)) + throw new ArgumentException( + "Attempt to set handler for a request type that already has a handler.", + nameof(requestType) + ); } public IEnumerable HandleMessage(Message message) diff --git a/c-sharp/MessageTransports/PapiClient.cs b/c-sharp/MessageTransports/PapiClient.cs index a9affcb992..2fe1a755af 100644 --- a/c-sharp/MessageTransports/PapiClient.cs +++ b/c-sharp/MessageTransports/PapiClient.cs @@ -203,7 +203,7 @@ public virtual void SendRequest( { ObjectDisposedException.ThrowIf(_isDisposed, this); - int requestId = Interlocked.Increment(ref _nextRequestId); + int requestId = GetRequestId(); var requestMessage = (requestContents is JsonElement jse) ? new MessageRequest(requestType, requestId, jse) @@ -319,6 +319,10 @@ public virtual void SendEvent(MessageEvent message) #endregion + #region Protected Methods + protected int GetRequestId() => Interlocked.Increment(ref _nextRequestId); + #endregion + #region Private helper methods for any thread to call /// diff --git a/c-sharp/Projects/ParatextProjectDataProvider.cs b/c-sharp/Projects/ParatextProjectDataProvider.cs index ff8e5d8e88..6e5be5f581 100644 --- a/c-sharp/Projects/ParatextProjectDataProvider.cs +++ b/c-sharp/Projects/ParatextProjectDataProvider.cs @@ -9,6 +9,7 @@ using Paranext.DataProvider.MessageHandlers; using Paranext.DataProvider.MessageTransports; using Paratext.Data; +using Paratext.Data.ProjectSettingsAccess; using SIL.Scripture; namespace Paranext.DataProvider.Projects; @@ -52,6 +53,8 @@ LocalParatextProjects paratextProjects Getters.Add("getSetting", GetProjectSetting); Setters.Add("setSetting", SetProjectSetting); + + RegisterSettingsValidators(); } #endregion @@ -175,6 +178,39 @@ protected virtual IProjectStreamManager CreateStreamManager(ProjectDetails proje #endregion #region Settings + public static string VisibilitySettingName => Setting.Visibility.ToString(); + private void RegisterSettingsValidators() + { + (bool result, string? error) VisibilityValidator((string newValueJson, string currentValueJson, + string allChangesJson, string projectType) data) + { + try + { + var value = JsonConvert.DeserializeObject(data.newValueJson); + var result = true; + string? error = null; + if (value == null) + { + result = false; + error = $"New {VisibilitySettingName} value cannot be null."; + } + else if (value is not ProjectVisibility && (value is not string valueStr || + !Enum.TryParse(valueStr, out var visibility))) + { + result = false; + error = $"New {VisibilitySettingName} value is not valid."; + } + return (result, error); + } + catch (Exception ex) + { + return (false, ex.Message); + } + } + + ProjectSettingsService.RegisterValidator(PapiClient, VisibilitySettingName, + VisibilityValidator); + } public ResponseToRequest GetProjectSetting(string jsonKey) { diff --git a/c-sharp/Projects/ProjectSettingsService.cs b/c-sharp/Projects/ProjectSettingsService.cs index 07c18746ac..fec15691b8 100644 --- a/c-sharp/Projects/ProjectSettingsService.cs +++ b/c-sharp/Projects/ProjectSettingsService.cs @@ -1,4 +1,5 @@ using System.Text.Json; +using Paranext.DataProvider.MessageHandlers; using Paranext.DataProvider.MessageTransports; namespace Paranext.DataProvider.Projects @@ -6,7 +7,29 @@ namespace Paranext.DataProvider.Projects internal static class ProjectSettingsService { private const string PROJECT_SETTINGS_SERVICE = "object:ProjectSettingsService.function"; + private const string PROJECT_SETTING_VALIDATOR = "extensionProjectSettingValidator:"; + internal static string GetValidatorKey(string settingKey) => + PROJECT_SETTING_VALIDATOR + settingKey; + + /// + /// Calls registered project settings validators to determine whether or not a (Paratext) + /// project setting change is valid. + /// + /// The proxy to facilitate communication to the Paranext server + /// via the PAPI + /// The new value requested to set the project setting value to, + /// serialized as a JSON string + /// The current project setting value, serialized as a JSON + /// string + /// The (Paratext) project setting key being set (this will probably + /// always be different from the corresponding key known to Platform.Bible) + /// A JSON object containing the keys and values of + /// all the settings being changed as part of an atomic operation (i.e., a single batch of + /// changes) + /// The `projectType` to get default setting value for (probably + /// always "ParatextStandard") + /// true if change is valid, false otherwise public static bool IsValid( PapiClient papiClient, string newValueJson, @@ -57,6 +80,21 @@ string projectType return requestSucceeded; } + /// + /// Gets the default value for a project setting + /// + /// The proxy to facilitate communication to the Paranext server + /// via the PAPI + /// The (Paratext) project setting key for which to get the default value + /// (this will probably always be different from the corresponding key known to + /// Platform.Bible) + /// + /// The `projectType` to get default setting value for (probably + /// always "ParatextStandard") + /// Every Project Data Provider **must** run this function when it receives a + /// request to get a project setting if the project does not have a value for the project + /// setting requested. It should return the response from this function directly, either + /// the returned default value or throw. public static string? GetDefault(PapiClient papiClient, string key, string projectType) { string? defaultValue = null; @@ -90,5 +128,69 @@ string projectType getDefaultTask.Wait(cts.Token); return defaultValue; } + + /// + /// Registers a function that validates whether a new setting value is allowed to be set. + /// + /// The proxy to facilitate communication to the Paranext server + /// via the PAPI + /// The id of the setting in Platform.Bible for which the validator is to + /// be registered + /// Function to call to validate the new setting value. See + /// for a complete + /// explanation of the input parameters to this function. + /// + /// true if the validator was successfully registered, false + /// otherwise + public static bool RegisterValidator( + PapiClient papiClient, + string key, + Func< + ( + string newValueJson, + string currentValueJson, + string allChangesJson, + string projectType + ), + (bool result, string? error) + > validatorCallback + ) + { + ResponseToRequest requestHandler(JsonElement jsonElement) + { + // Check if the JsonElement is an array + if ( + jsonElement.ValueKind != JsonValueKind.Array + || jsonElement.GetArrayLength() != 3 + ) + { + return ResponseToRequest.Failed( + $"Validator for {key} expected a JSON array with 3 items: newValueJson" + + "currentValueJson, allChangesJson." + ); + } + + string newValueJson = jsonElement[0].GetString() ?? ""; + string currentValueJson = jsonElement[1].GetString() ?? ""; + string allChangesJson = jsonElement[2].GetString() ?? ""; + + var validationResponse = validatorCallback( + (newValueJson, currentValueJson, allChangesJson, ProjectType.Paratext) + ); + return validationResponse.result + ? ResponseToRequest.Succeeded() + : ResponseToRequest.Failed( + validationResponse.error + ?? "Parameter validation failed for {key}. Error response provided no details." + ); + } + ; + + var t = papiClient.RegisterRequestHandler(GetValidatorKey(key), requestHandler); + + using var cts = new CancellationTokenSource(); + t.Wait(cts.Token); + return t.Result; + } } }