From 4174b0662169c1315f228c5211f2adb0badc4f67 Mon Sep 17 00:00:00 2001 From: Steve Ballantine Date: Fri, 13 Jan 2023 11:06:41 +0000 Subject: [PATCH 1/3] Merged PR 8055: BUG: Fix client-side features for .NET framework - BUG: ExampleFrameworkWebsite did not include several dependencies that were required. - BUG: PipelineModule does not complete the request after intercepting requests for json and js data. This means that the static file handler runs later in the process, can't find the files and returns a 404. #5336 Related work items: #5336 --- .../AspNet Framework MVC Example.csproj | 21 +++++++++++++++++++ .../ExampleFrameworkWebsite/packages.config | 7 +++++++ .../PipelineModule.cs | 2 ++ 3 files changed, 30 insertions(+) diff --git a/Web Integration/Examples/ExampleFrameworkWebsite/AspNet Framework MVC Example.csproj b/Web Integration/Examples/ExampleFrameworkWebsite/AspNet Framework MVC Example.csproj index 3ca2bfb9..784473d3 100644 --- a/Web Integration/Examples/ExampleFrameworkWebsite/AspNet Framework MVC Example.csproj +++ b/Web Integration/Examples/ExampleFrameworkWebsite/AspNet Framework MVC Example.csproj @@ -46,14 +46,35 @@ 4 + + ..\..\..\packages\FiftyOne.Caching.4.4.1\lib\netstandard2.0\FiftyOne.Caching.dll + + + ..\..\..\packages\FiftyOne.Common.4.4.1\lib\netstandard2.0\FiftyOne.Common.dll + ..\..\..\packages\Newtonsoft.Json.13.0.1\lib\net45\Newtonsoft.Json.dll + + ..\..\..\packages\NUglify.1.5.12\lib\net40\NUglify.dll + + + ..\..\..\packages\Stubble.Core.1.7.2\lib\net45\Stubble.Core.dll + + + ..\..\..\packages\System.Collections.Immutable.1.5.0\lib\netstandard2.0\System.Collections.Immutable.dll + + + ..\..\..\packages\System.Runtime.CompilerServices.Unsafe.4.5.0\lib\netstandard2.0\System.Runtime.CompilerServices.Unsafe.dll + + + ..\..\..\packages\System.Threading.Tasks.Extensions.4.5.1\lib\netstandard2.0\System.Threading.Tasks.Extensions.dll + diff --git a/Web Integration/Examples/ExampleFrameworkWebsite/packages.config b/Web Integration/Examples/ExampleFrameworkWebsite/packages.config index 358e0f36..56348e08 100644 --- a/Web Integration/Examples/ExampleFrameworkWebsite/packages.config +++ b/Web Integration/Examples/ExampleFrameworkWebsite/packages.config @@ -2,6 +2,8 @@ + + @@ -14,6 +16,11 @@ + + + + + \ No newline at end of file diff --git a/Web Integration/FiftyOne.Pipeline.Web.Framework/PipelineModule.cs b/Web Integration/FiftyOne.Pipeline.Web.Framework/PipelineModule.cs index 75ec1757..a3551996 100644 --- a/Web Integration/FiftyOne.Pipeline.Web.Framework/PipelineModule.cs +++ b/Web Integration/FiftyOne.Pipeline.Web.Framework/PipelineModule.cs @@ -81,11 +81,13 @@ private void OnBeginRequestJavascript(object sender, EventArgs e) StringComparison.OrdinalIgnoreCase)) { FiftyOneJsProvider.GetInstance().ServeJavascript(context); + HttpContext.Current.ApplicationInstance.CompleteRequest(); } if (context.Request.Path.EndsWith("51dpipeline/json", StringComparison.OrdinalIgnoreCase)) { FiftyOneJsProvider.GetInstance().ServeJson(context); + HttpContext.Current.ApplicationInstance.CompleteRequest(); } } } From 47179f48292baed3d819914e09275299bea1a6b0 Mon Sep 17 00:00:00 2001 From: Ben Shillito Date: Fri, 13 Jan 2023 14:32:25 +0000 Subject: [PATCH 2/3] Merged PR 8063: FEAT: Return the correct upgrade message if the property was not found in the data file, but there is an explicit property getter for it in the data for the engine. #4197 Related work items: #4197 --- .../GenericEngine.cs | 43 +++++++++++ .../Services/MissingPropertyService.cs | 77 ++++++++++++++++++- .../Services/MissingPropertyServiceTests.cs | 72 +++++++++++++++++ 3 files changed, 191 insertions(+), 1 deletion(-) create mode 100644 FiftyOne.Pipeline.Engines.TestHelpers/GenericEngine.cs diff --git a/FiftyOne.Pipeline.Engines.TestHelpers/GenericEngine.cs b/FiftyOne.Pipeline.Engines.TestHelpers/GenericEngine.cs new file mode 100644 index 00000000..f25dc443 --- /dev/null +++ b/FiftyOne.Pipeline.Engines.TestHelpers/GenericEngine.cs @@ -0,0 +1,43 @@ +using FiftyOne.Pipeline.Core.Data; +using FiftyOne.Pipeline.Core.FlowElements; +using FiftyOne.Pipeline.Engines.Data; +using FiftyOne.Pipeline.Engines.FlowElements; +using Microsoft.Extensions.Logging; +using System; +using System.Collections.Generic; +using System.Text; + +namespace FiftyOne.Pipeline.Engines.TestHelpers +{ + public class GenericEngine : AspectEngineBase + where T : IAspectData + { + public override string DataSourceTier { get; } + + public override string ElementDataKey { get; } + + public override IEvidenceKeyFilter EvidenceKeyFilter => new EvidenceKeyFilterWhitelist(new List()); + + public override IList Properties { get; } + + public GenericEngine( + ILogger> logger, + string tier, + string dataKey, + IList properties) : + base(logger, (p, e) => default(T)) + { + DataSourceTier = tier; + ElementDataKey = dataKey; + Properties = properties; + } + + protected override void ProcessEngine(IFlowData data, T aspectData) + { + } + + protected override void UnmanagedResourcesCleanup() + { + } + } +} diff --git a/FiftyOne.Pipeline.Engines/Services/MissingPropertyService.cs b/FiftyOne.Pipeline.Engines/Services/MissingPropertyService.cs index 9519b028..cae9a5e8 100644 --- a/FiftyOne.Pipeline.Engines/Services/MissingPropertyService.cs +++ b/FiftyOne.Pipeline.Engines/Services/MissingPropertyService.cs @@ -40,6 +40,14 @@ public class MissingPropertyService : IMissingPropertyService private static IMissingPropertyService _instance; private static object _lock = new object(); + /// + /// Used to store the results of looking up whether a property is available or not. + /// + /// The key is the engine type. The inner dictionary is keyed on property name. + /// + private static Dictionary> _propertyAvailable = + new Dictionary>(); + /// /// Get the singleton instance of this service. /// @@ -149,6 +157,14 @@ public MissingPropertyResult GetMissingPropertyReason(string propertyName, IAspe reason = MissingPropertyReason.PropertyNotAccessibleWithResourceKey; } } + else if (reason == MissingPropertyReason.Unknown && + EngineDataContainsPropertyGetter(propertyName, engine)) + { + // If the property meta data is not available, but the engine + // data class defines a getter, it's safe to assume that the data + // file needs upgrading. + reason = MissingPropertyReason.DataFileUpgradeRequired; + } // Build the message string to return to the caller. StringBuilder message = new StringBuilder(); @@ -163,7 +179,9 @@ public MissingPropertyResult GetMissingPropertyReason(string propertyName, IAspe message.Append( string.Format(CultureInfo.InvariantCulture, Messages.MissingPropertyMessageDataUpgradeRequired, - string.Join(",", property.DataTiersWherePresent), + property == null ? + "Unknown" : + string.Join(",", property.DataTiersWherePresent), engine.GetType().Name)); break; case MissingPropertyReason.PropertyExcludedFromEngineConfiguration: @@ -193,5 +211,62 @@ public MissingPropertyResult GetMissingPropertyReason(string propertyName, IAspe result.Reason = reason; return result; } + + /// + /// Return true if there is an explicit property getter for the name provided + /// in the data type returned by the engine. + /// + /// + /// + /// + private bool EngineDataContainsPropertyGetter(string propertyName, IAspectEngine engine) + { + // Get the property dictionary for this engine + Dictionary engineProperties; + if(_propertyAvailable.TryGetValue(engine.GetType(), out engineProperties) == false) + { + lock (_propertyAvailable) + { + if (_propertyAvailable.TryGetValue(engine.GetType(), out engineProperties) == false) + { + engineProperties = new Dictionary(StringComparer.OrdinalIgnoreCase); + _propertyAvailable.Add(engine.GetType(), engineProperties); + } + } + } + + // If we don't have a stored result in the dictionary for this property then use + // reflection to figure it out. + if (engineProperties.TryGetValue(propertyName, out var result) == false) + { + result = false; + + foreach (var dataType in engine.GetType().GetInterfaces().SelectMany(i => i.GetGenericArguments()) + .Where(i => typeof(IAspectData).IsAssignableFrom(i))) + { + if (dataType != null && result == false) + { + foreach (var property in dataType.GetProperties()) + { + if (property.Name.Equals(propertyName, StringComparison.InvariantCultureIgnoreCase)) + { + result = true; + break; + } + } + } + } + + // Add the result to the property dictionary. + lock (engineProperties) + { + if (engineProperties.ContainsKey(propertyName) == false) + { + engineProperties.Add(propertyName, result); + } + } + } + return result; + } } } diff --git a/Tests/FiftyOne.Pipeline.Engines.Tests/Services/MissingPropertyServiceTests.cs b/Tests/FiftyOne.Pipeline.Engines.Tests/Services/MissingPropertyServiceTests.cs index d7e2ddd3..b2bba410 100644 --- a/Tests/FiftyOne.Pipeline.Engines.Tests/Services/MissingPropertyServiceTests.cs +++ b/Tests/FiftyOne.Pipeline.Engines.Tests/Services/MissingPropertyServiceTests.cs @@ -20,9 +20,16 @@ * such notice(s) shall fulfill the requirements of that article. * ********************************************************************* */ +using FiftyOne.Pipeline.Core.Data; +using FiftyOne.Pipeline.Core.FlowElements; +using FiftyOne.Pipeline.Core.TypedMap; +using FiftyOne.Pipeline.Engines.Caching; +using FiftyOne.Pipeline.Engines.Configuration; using FiftyOne.Pipeline.Engines.Data; using FiftyOne.Pipeline.Engines.FlowElements; using FiftyOne.Pipeline.Engines.Services; +using FiftyOne.Pipeline.Engines.TestHelpers; +using Microsoft.Extensions.Logging; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; using System; @@ -62,6 +69,71 @@ public void MissingPropertyService_GetReason_Upgrade() Assert.AreEqual(MissingPropertyReason.DataFileUpgradeRequired, result.Reason); } + public interface ITestData : IAspectData + { + public string TestProperty { get; } + } + + /// + /// Check that when the property meta data is not present, so the data tier + /// cannot be checked, that an upgrade message is returned correctly. + /// This will be the case if the property is not present in the data file, + /// but there is an explicit getter for it in the data class. + /// + [TestMethod] + public void MissingPropertyService_GetReason_UnknownUpgrade() + { + // Arrange + GenericEngine engine = new GenericEngine( + new Mock>>().Object, + "lite", + "test", + new List()); + + // Act + var result = _service.GetMissingPropertyReason("testProperty", engine); + + // Assert + Assert.AreEqual(MissingPropertyReason.DataFileUpgradeRequired, result.Reason); + } + + public class InheritedTestEngine : GenericEngine + { + public InheritedTestEngine( + ILogger> logger, + string tier, + string dataKey, + IList properties) + : base(logger, tier, dataKey, properties) + { + } + } + + /// + /// Check that when the property meta data is not present, so the data tier + /// cannot be checked, that an upgrade message is returned correctly. + /// This will be the case if the property is not present in the data file, + /// but there is an explicit getter for it in the data class. + /// This test uses a further inherited class to check that the data type + /// can still be worked out if the generic data type is burried. + /// + [TestMethod] + public void MissingPropertyService_GetReason_UnknownUpgrade_Inherited() + { + // Arrange + InheritedTestEngine engine = new InheritedTestEngine( + new Mock>().Object, + "lite", + "test", + new List()); + + // Act + var result = _service.GetMissingPropertyReason("testProperty", engine); + + // Assert + Assert.AreEqual(MissingPropertyReason.DataFileUpgradeRequired, result.Reason); + } + /// /// Check that the missing property service works as expected when /// the property has been excluded from the result set From cde169d6e80d3906b102d42cd8c5c80809ad81f4 Mon Sep 17 00:00:00 2001 From: CIUser Date: Fri, 13 Jan 2023 16:25:06 +0000 Subject: [PATCH 3/3] REF: Update submodules references. --- ci/common-ci | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/common-ci b/ci/common-ci index bfffd7bd..f1f5f3c6 160000 --- a/ci/common-ci +++ b/ci/common-ci @@ -1 +1 @@ -Subproject commit bfffd7bde49abc58ef661e99d72d1c01ecdf9697 +Subproject commit f1f5f3c6216e2cbf845d1e2df4b75de20922461f