ISPN-9819 Listeners correction in spring session

[karesti]

https://issues.jboss.org/browse/ISPN-9819

[wburns]

To completely finalize this I think we still need to add a test checking that a session attribute is available to a registered listener.

Also I would say we should probably verify that embedded works in a clustered environment and confirm that the event is propagated to even non owner nodes.

[karesti]

@wburns Ok for the tests you mentioned

[karesti]

I will squash all the commits in the end

[karesti]

@wburns I've been working on the tests, and it's too much effort today to spend more time testing from spring that the listeners are indeed clustered in embedded. Same applies to checking a value inside the httpsession. I added a test to check that the events are raised and that contain a non null http session inside. Which is the original problem that was fixed from spring session and now from our side as well.

[karesti]

@wburns concerning integration tests, they are missing, globally speaking, on spring/spring-boot. But I would add those tests in the spring-boot starter, where we do provide nice support for spring-boot, instead of infinispan repository. I have already an issue for that. Spring session integration tests are missing there, and others too

[karesti]

@gustavonalle @wburns failed tests unrelated

[danberindei]

@karesti I just noticed @wburns's comment, and he's right that we should check a session attribute in the test to see that it's a full session and not a dummy one.

Please also rename callEviction to processExpiration, and configure the cache managers with a ControlledTimeService so that the test doesn't have to sleep for 1 second.

[karesti]

@danberindei I added a test to check that we have a session that it's not null. If the underlying session object exists - not null - it is not a dummy session but the session. This is how spring session is working now

[karesti]

@danberindei as I said before, these kind of tests are good to test with spring boot and all the spring boot mechanisms. The starter is the good place to put these tests, instead of the spring integration tests in infinispan code base (older tests, before the starter existed)

[danberindei]

@karesti I didn't mean to add a new test, but testEventBridge already checks that the number of emitted events is correct, so I was thinking it could be amended to set an attribute in the session then check that it's there in the event as well.

I prefer putting tests as close to the code they're testing as possible. If we only had tests in the starter we could make a breaking change in core, then we'd modify the spring integration to compile and push to master, and we'd only see a failure in the starter test suite much later.

[karesti]

@danberindei hi, tell me what you think, before merge I have to squash

[danberindei]

I missed this... we should at least log the error if something went wrong

[danberindei]

Actually logging is probably best left to ClientListenerInvocation... here we should return null if buffer.remaining() == 0, but otherwise we should assume the buffer contains a full element.

[danberindei]

I pushed a commit to check the size to your branch @karesti
Test suite looks good locally but I'll wait for CI to confirm

[karesti]

Test failure is unrelated.
Backport PR is here #6535

[danberindei]

Integrated, thanks Katia!