Implement IDisposable in RedisCacheWrapper

[hishamco]

No description provided.

[sebastienros]

I think it's important to explain the intent of such changes, at least sharing what the scopes of the service is and why this change is better. So I dug a little...

The wrapper is a singleton, at first I didn't see why calling Dispose is better in any way. Being a singleton everything will be release once the application stops.

However I looked into the history of this "Wrapper". And it seems that it was added to prevent the internal redis cache service to be "DIsposed" when a tenant is restarted, or the IConnectionMultiplexer that is shared across tenants would be disposed too. Hence the goal of this wrapper is to NOT dispose the redis cache.

Please revert it an add a comment on this class explain what its purpose is and why the redis cache should not be disposed. IF there were other goals behind this change please explain them.

[hishamco]

@sebastienros yes the intention is not to dispose of the cache because it's shared across tenants, I don't think there's wrong with disposing of the cache once the Wrapper is disposed

For instance, we disposed of the list of shells once the ShellHost is disposed which is make sense to me

[sebastienros]

I don't think there's wrong with disposing of the cache once the Wrapper is disposed

I literally explained why it's wrong in the comment I made

[hishamco]

I will revert the commit, but at least we should mark this class as sealed, right?

[sebastienros]

I will revert the commit, but at least we should mark this class as sealed, right?

Yes this is fine, in your someone will also add the comments on the class to explain why it should not dispose the redis cache.