restore ISerializer ability, "safely" #218
Conversation
src/RedisOutputCachingMiddleware/RedisOutputCachingMiddleware.csproj
Outdated
Show resolved
Hide resolved
test/RedisOutputCachingMiddlewareUnitTests/RedisOutputCachingMiddleware.UnitTests.csproj
Outdated
Show resolved
Hide resolved
|
@zachrybaker can you please agree to the CLA? |
@microsoft-github-policy-service agree |
|
@microsoft-github-policy-service agree |
|
@stanleysmall-microsoft if you can recap what you’re looking for me at this point, I can try to put some attention on it over the weekend or something. |
|
Yes, the intention is to note how "THIS" instance of the data in redis is
serialized, vs some other one. I have a totally different Blazor app also
syncing this data.
Zachry Baker
…On Fri, Jan 12, 2024 at 12:19 PM Philo ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/RedisSessionStateProvider/KeyGenerator.cs
<#218 (comment)>
:
>
private void GenerateKeys(string id, string app)
{
this.id = id;
- DataKey = $"{{{app}_{id}}}_SessionStateItemCollection";
+ DataKey = $"{{{app}_{id}}}_{SessionDataType}";
We should retain the "SessionStateItemCollection" portion of the key
unless we need to eliminate it. What's your intention for the new
SessionDataType? I assume to tag keys with a serializer identifier?
—
Reply to this email directly, view it on GitHub
<#218 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACDSMQGDQSAUNCC6N6YTYLYOF5CDAVCNFSM6AAAAABA2HOYEOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQMJYHAYDONBRGU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
@zachrybaker thanks for making this PR. I think your changes will improve the user experience of the package. I already made some changes to your branch which I think can be merged soon. The only questions I had were about |
|
Yes.
I added the publish feature so that another app/process can subscribe to
the update.
My use case is that I am incrementally rewriting a large web forms app in
Blazor and for most of that time period they will share session and profile
data. Taking the strangler fig approach where both apps will coexist and
the Blazor version will proxy unhandled requests to the old app. Since
Blazor does not really have the HTTP request/response model but is a
circuit - more of a current state machine, I needed a mechanism to
synchronize profile and session between the two and this publish fits the
bill perfectly.
…On Tue, Jan 16, 2024 at 7:30 AM Stanley Small ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/Shared/ProviderConfiguration.cs
<#218 (comment)>
:
> @@ -87,6 +90,8 @@ private ProviderConfiguration(NameValueCollection config)
// All below parameters are only fetched from web.config
DatabaseId = GetIntSettings(config, "databaseId", 0);
ApplicationName = GetStringSettings(config, "applicationName", null);
+ PublishSessionChanges = GetBoolSettings(config, "publishSessionChanges", false);
@zachrybaker <https://github.com/zachrybaker> can you briefly describe
the motivation behind adding the publishSessionChanges parameter? I don't
think we should add it in this PR, but I'm curious about what this does.
—
Reply to this email directly, view it on GitHub
<#218 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AACDSMXR7ZRXBKKJWIM7IZLYOZ6HTAVCNFSM6AAAAABA2HOYEOVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQMRTGM3TONZZHA>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
test/OutputCacheProviderFunctionalTests/RedisOutputCacheProvider.FunctionalTests.csproj
Outdated
Show resolved
Hide resolved
test/RedisSessionStateProviderFunctionalTests/RedisSessionStateProvider.FunctionalTests.csproj
Show resolved
Hide resolved
|
@stanleysmall-microsoft Can you comment on what is going on with this PR and branch? AFAICT I submitted a PR to restore the ability to serialize to whatever format necessary, providing the means to do safe serialization. I also provided an option to publish the session so that subscribers (for example replacement apps using the Microsoft.SystemWebAdapters package) could interop and share session out of process. This is important for the Blazor use case, as session support in Blazor server is not there yet (and may never be). It seems you guys repurposed my branch to do maintenance work but axed my features and the whole purpose of the branch. Can you explain? |
This will resolve issue 185