From be1abf4ce2730d6cd70fb9356a58aed1c1c2d062 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20B=C5=82a=C5=BCejewicz=20=28Peter=20Blazejewicz=29?= Date: Thu, 24 Oct 2019 21:18:44 +0200 Subject: [PATCH] :sparkles: Update JSInterop details to use direct code evaluation As mentioned in the comment in the `protectedBrowserStorage.js` (being removed), that is now possible to access some native features of the JavaScript engine without a need of the custom wrapper. This change removes such access wrapper and simplifies implemenation, which results in cleaner, quicker to grasp code. - implemenation detail changes - removed JavaScript code - tests updates Thanks! --- .../Pages/_Host.cshtml | 1 - .../src/ProtectedBrowserStorage.cs | 11 ++--- .../src/wwwroot/protectedBrowserStorage.js | 10 ---- .../test/ProtectedBrowserStorageTest.cs | 47 ++++++++++--------- 4 files changed, 27 insertions(+), 42 deletions(-) delete mode 100644 src/ProtectedBrowserStorage/src/wwwroot/protectedBrowserStorage.js diff --git a/src/ProtectedBrowserStorage/sample/ProtectedBrowserStorageSample/Pages/_Host.cshtml b/src/ProtectedBrowserStorage/sample/ProtectedBrowserStorageSample/Pages/_Host.cshtml index 8a30d159a..7a1a03056 100644 --- a/src/ProtectedBrowserStorage/sample/ProtectedBrowserStorageSample/Pages/_Host.cshtml +++ b/src/ProtectedBrowserStorage/sample/ProtectedBrowserStorageSample/Pages/_Host.cshtml @@ -16,6 +16,5 @@ - diff --git a/src/ProtectedBrowserStorage/src/ProtectedBrowserStorage.cs b/src/ProtectedBrowserStorage/src/ProtectedBrowserStorage.cs index fb4f660aa..1fbffb2a7 100644 --- a/src/ProtectedBrowserStorage/src/ProtectedBrowserStorage.cs +++ b/src/ProtectedBrowserStorage/src/ProtectedBrowserStorage.cs @@ -25,8 +25,6 @@ namespace Microsoft.AspNetCore.ProtectedBrowserStorage /// public abstract class ProtectedBrowserStorage { - private const string JsFunctionsPrefix = "protectedBrowserStorage"; - private readonly string _storeName; private readonly IJSRuntime _jsRuntime; private readonly IDataProtectionProvider _dataProtectionProvider; @@ -95,8 +93,7 @@ public ValueTask SetAsync(string purpose, string key, object value) var protector = GetOrCreateCachedProtector(purpose); var protectedJson = protector.Protect(json); return _jsRuntime.InvokeVoidAsync( - $"{JsFunctionsPrefix}.set", - _storeName, + $"{_storeName}.setItem", key, protectedJson); } @@ -123,8 +120,7 @@ public ValueTask GetAsync(string key) public async ValueTask GetAsync(string purpose, string key) { var protectedJson = await _jsRuntime.InvokeAsync( - $"{JsFunctionsPrefix}.get", - _storeName, + $"{_storeName}.getItem", key); // We should consider having both TryGetAsync and GetValueOrDefaultAsync @@ -150,8 +146,7 @@ public async ValueTask GetAsync(string purpose, string key) public ValueTask DeleteAsync(string key) { return _jsRuntime.InvokeVoidAsync( - $"{JsFunctionsPrefix}.delete", - _storeName, + $"{_storeName}.removeItem", key); } diff --git a/src/ProtectedBrowserStorage/src/wwwroot/protectedBrowserStorage.js b/src/ProtectedBrowserStorage/src/wwwroot/protectedBrowserStorage.js deleted file mode 100644 index c5388331c..000000000 --- a/src/ProtectedBrowserStorage/src/wwwroot/protectedBrowserStorage.js +++ /dev/null @@ -1,10 +0,0 @@ -(function () { - // We may be able to eliminate this .js file entirely as of preview 8, - // because it will be possible to invoke things like localStorage.setValue - // directly from .NET without needing a JS wrapper - window.protectedBrowserStorage = { - get: (storeName, key) => window[storeName][key], - set: (storeName, key, value) => { window[storeName][key] = value; }, - delete: (storeName, key) => { delete window[storeName][key]; } - }; -})(); diff --git a/src/ProtectedBrowserStorage/test/ProtectedBrowserStorageTest.cs b/src/ProtectedBrowserStorage/test/ProtectedBrowserStorageTest.cs index 96e6b8064..468e5e68d 100644 --- a/src/ProtectedBrowserStorage/test/ProtectedBrowserStorageTest.cs +++ b/src/ProtectedBrowserStorage/test/ProtectedBrowserStorageTest.cs @@ -48,13 +48,14 @@ public void SetAsync_ProtectsAndInvokesJS(string customPurpose) { // Arrange var jsRuntime = new TestJSRuntime(); + var storeName = "test store"; var dataProtectionProvider = new TestDataProtectionProvider(); - var protectedBrowserStorage = new TestProtectedBrowserStorage("test store", jsRuntime, dataProtectionProvider); + var protectedBrowserStorage = new TestProtectedBrowserStorage(storeName, jsRuntime, dataProtectionProvider); var jsResultTask = new ValueTask((object)null); var data = new TestModel { StringProperty = "Hello", IntProperty = 123 }; var keyName = "test key"; var expectedPurpose = customPurpose == null - ? $"{typeof(TestProtectedBrowserStorage).FullName}:test store:{keyName}" + ? $"{typeof(TestProtectedBrowserStorage).FullName}:{storeName}:{keyName}" : customPurpose; // Act @@ -65,9 +66,8 @@ public void SetAsync_ProtectsAndInvokesJS(string customPurpose) // Assert var invocation = jsRuntime.Invocations.Single(); - Assert.Equal("protectedBrowserStorage.set", invocation.Identifier); + Assert.Equal($"{storeName}.setItem", invocation.Identifier); Assert.Collection(invocation.Args, - arg => Assert.Equal("test store", arg), arg => Assert.Equal(keyName, arg), arg => Assert.Equal( "{\"StringProperty\":\"Hello\",\"IntProperty\":123}", @@ -79,10 +79,11 @@ public void SetAsync_ProtectsAndInvokesJS_NullValue() { // Arrange var jsRuntime = new TestJSRuntime(); + var storeName = "test store"; var dataProtectionProvider = new TestDataProtectionProvider(); - var protectedBrowserStorage = new TestProtectedBrowserStorage("test store", jsRuntime, dataProtectionProvider); + var protectedBrowserStorage = new TestProtectedBrowserStorage(storeName, jsRuntime, dataProtectionProvider); var jsResultTask = new ValueTask((object)null); - var expectedPurpose = $"{typeof(TestProtectedBrowserStorage).FullName}:test store:test key"; + var expectedPurpose = $"{typeof(TestProtectedBrowserStorage).FullName}:{storeName}:test key"; // Act jsRuntime.NextInvocationResult = jsResultTask; @@ -90,9 +91,8 @@ public void SetAsync_ProtectsAndInvokesJS_NullValue() // Assert var invocation = jsRuntime.Invocations.Single(); - Assert.Equal("protectedBrowserStorage.set", invocation.Identifier); + Assert.Equal($"{storeName}.setItem", invocation.Identifier); Assert.Collection(invocation.Args, - arg => Assert.Equal("test store", arg), arg => Assert.Equal("test key", arg), arg => Assert.Equal( "null", @@ -107,11 +107,12 @@ public async Task GetAsync_InvokesJSAndUnprotects_ValidData(string customPurpose // Arrange var jsRuntime = new TestJSRuntime(); var dataProtectionProvider = new TestDataProtectionProvider(); - var protectedBrowserStorage = new TestProtectedBrowserStorage("test store", jsRuntime, dataProtectionProvider); + var storeName = "test store"; + var protectedBrowserStorage = new TestProtectedBrowserStorage(storeName, jsRuntime, dataProtectionProvider); var data = new TestModel { StringProperty = "Hello", IntProperty = 123 }; var keyName = "test key"; var expectedPurpose = customPurpose == null - ? $"{typeof(TestProtectedBrowserStorage).FullName}:test store:{keyName}" + ? $"{typeof(TestProtectedBrowserStorage).FullName}:{storeName}:{keyName}" : customPurpose; var storedJson = "{\"StringProperty\":\"Hello\",\"IntProperty\":123}"; jsRuntime.NextInvocationResult = new ValueTask( @@ -127,9 +128,8 @@ public async Task GetAsync_InvokesJSAndUnprotects_ValidData(string customPurpose Assert.Equal(123, result.IntProperty); var invocation = jsRuntime.Invocations.Single(); - Assert.Equal("protectedBrowserStorage.get", invocation.Identifier); + Assert.Equal($"{storeName}.getItem", invocation.Identifier); Assert.Collection(invocation.Args, - arg => Assert.Equal("test store", arg), arg => Assert.Equal(keyName, arg)); } @@ -217,8 +217,9 @@ public void DeleteAsync_InvokesJS() { // Arrange var jsRuntime = new TestJSRuntime(); + var storeName = "test store"; var dataProtectionProvider = new TestDataProtectionProvider(); - var protectedBrowserStorage = new TestProtectedBrowserStorage("test store", jsRuntime, dataProtectionProvider); + var protectedBrowserStorage = new TestProtectedBrowserStorage(storeName, jsRuntime, dataProtectionProvider); var nextTask = new ValueTask((object)null); jsRuntime.NextInvocationResult = nextTask; @@ -227,9 +228,8 @@ public void DeleteAsync_InvokesJS() // Assert var invocation = jsRuntime.Invocations.Single(); - Assert.Equal("protectedBrowserStorage.delete", invocation.Identifier); + Assert.Equal($"{storeName}.removeItem", invocation.Identifier); Assert.Collection(invocation.Args, - arg => Assert.Equal("test store", arg), arg => Assert.Equal("test key", arg)); } @@ -239,8 +239,9 @@ public async Task ReusesCachedProtectorsByPurpose() // Arrange var jsRuntime = new TestJSRuntime(); jsRuntime.NextInvocationResult = new ValueTask((object)null); + var storeName = "test store"; var dataProtectionProvider = new TestDataProtectionProvider(); - var protectedBrowserStorage = new TestProtectedBrowserStorage("test store", jsRuntime, dataProtectionProvider); + var protectedBrowserStorage = new TestProtectedBrowserStorage(storeName, jsRuntime, dataProtectionProvider); // Act await protectedBrowserStorage.SetAsync("key 1", null); @@ -252,17 +253,17 @@ public async Task ReusesCachedProtectorsByPurpose() var typeName = typeof(TestProtectedBrowserStorage).FullName; var expectedPurposes = new[] { - $"{typeName}:test store:key 1", - $"{typeName}:test store:key 2", - $"{typeName}:test store:key 3" + $"{typeName}:{storeName}:key 1", + $"{typeName}:{storeName}:key 2", + $"{typeName}:{storeName}:key 3" }; Assert.Equal(expectedPurposes, dataProtectionProvider.ProtectorsCreated.ToArray()); Assert.Collection(jsRuntime.Invocations, - invocation => Assert.Equal(TestDataProtectionProvider.Protect(expectedPurposes[0], "null"), invocation.Args[2]), - invocation => Assert.Equal(TestDataProtectionProvider.Protect(expectedPurposes[1], "null"), invocation.Args[2]), - invocation => Assert.Equal(TestDataProtectionProvider.Protect(expectedPurposes[0], "null"), invocation.Args[2]), - invocation => Assert.Equal(TestDataProtectionProvider.Protect(expectedPurposes[2], "null"), invocation.Args[2])); + invocation => Assert.Equal(TestDataProtectionProvider.Protect(expectedPurposes[0], "null"), invocation.Args[1]), + invocation => Assert.Equal(TestDataProtectionProvider.Protect(expectedPurposes[1], "null"), invocation.Args[1]), + invocation => Assert.Equal(TestDataProtectionProvider.Protect(expectedPurposes[0], "null"), invocation.Args[1]), + invocation => Assert.Equal(TestDataProtectionProvider.Protect(expectedPurposes[2], "null"), invocation.Args[1])); } class TestModel