It seems DeleteOpenIDConnectSession is never called?

[mitar]

Describe the bug

I see that DeleteOpenIDConnectSession is defined and implemented for in-memory storage, but it is never called. This leads me to think that those session entries are never removed from the storage?

[aeneasr]

That could be possible!

[mitar]

What to do about this?

[aeneasr]

It's probably being used in Ory Hydra

[mitar]

I searched in Hydra and I could not find it being used except in tests. :-)

[aeneasr]

Ah then I guess we can omit it here :)

[mitar]

But does this mean sessions are never cleared? :-)

[aeneasr]

Looks like it! Not sure if it makes sense to delete the rows though as it contains info required for refreshing the oidc session

[mitar]

So should we just delete this from the interface (and I can remove it from Hydra tests, too) and then if at a later time see we need some cleanup, we add things at that time, whatever is needed?

[aeneasr]

Hm, since it’s a breaking change and there’s no downside in keeping it I’m not sure if it’s worth to remove it.

[mitar]

How it is a breaking change? If we remove it from the interface, all existing implementations will still be valid implementations of the interface. Only that code might have around an extra function nobody will ever call.

I can leave it in Hydra if you want. But removing it from the interface here would make it easier for developers of Fosite stores to know that to (or not to) implement.

[aeneasr]

It’s possible that someone (e.g. Hydra) relies on the interface method being available (https://github.com/ory/hydra/search?q=DeleteOpenIDConnectSession&type=) so removing it is a breaking change!

[mitar]

Hm, only if you are passing the interface and not concrete type, sure. But otherwise code still keeps the implementation.

Hydra does not rely on it.

[mitar]

Maybe we can just then in documentation mentioned, that it is not really used and is deprecated?

[aeneasr]

SGTM