Skip to content

Add smoke test with Spring Session and Hazelcast #27793

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
pmverma opened this issue Aug 21, 2021 · 12 comments
Closed

Add smoke test with Spring Session and Hazelcast #27793

pmverma opened this issue Aug 21, 2021 · 12 comments
Labels
status: reserved-for-conference-event status: superseded An issue that has been superseded by another type: task A general task

Comments

@pmverma
Copy link

pmverma commented Aug 21, 2021

With SB 2.4.7 and spring (session+hazelcast) 2.5.2, I was expecting the sessions endpoint to work properly.
In trace log there is,

   SessionsEndpointAutoConfiguration#sessionEndpoint:
      Did not match:
         - @ConditionalOnBean (types: org.springframework.session.FindByIndexNameSessionRepository; SearchStrategy: all) did not find any beans of type org.springframework.session.FindByIndexNameSessionRepository (OnBeanCondition)

But Hazelcast4IndexedSessionRepository is already there implementing FindByIndexNameSessionRepository

The endpoints docs mention only the following which is already set up in my case.

Allows retrieval and deletion of user sessions from a Spring Session-backed session store. Requires a Servlet-based web application using Spring Session.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Aug 21, 2021
@snicoll
Copy link
Member

snicoll commented Aug 22, 2021

@pmverma can you please share a small minimal sample that showcases the problem you've described. You can do so by attaching a zip to this issue or pushing the project on a GitHub repository.

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Aug 22, 2021
@vpavic
Copy link
Contributor

vpavic commented Aug 22, 2021

@snicoll I was able to reproduce this using my Spring Session samples repo - use sample-httpsession-hazelcast subproject.

I think this is actually a bug in Spring Session that was introduced together with the support for Hazelcast 4, when the return type of @Bean factory method in HazelcastHttpSessionConfiguration was changed to return SessionRepository<?> (see here). I can look into this from Spring Session side tomorrow.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Aug 22, 2021
@snicoll
Copy link
Member

snicoll commented Aug 26, 2021

@vpavic Thanks. Should we move this one to Spring Session then?

@vpavic
Copy link
Contributor

vpavic commented Aug 27, 2021

Hey @snicoll, I've taken care of this from Spring Session side - see the timeline just above.

I thought this issue could be used to consider adding smoke tests to prevent something like this from happening again. I added some from the Spring Session side, but since Spring Boot is the side that owns the integration it seems reasonable to have the smoke tests in here.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Aug 27, 2021
@snicoll
Copy link
Member

snicoll commented Aug 27, 2021

see the timeline just above.

Thanks but it's far from obvious from the timeline that it has been fixed. Have I missed a reference to an issue in Spring Session?

Edit: looks like I did 🤦

Sure, we can reuse this issue to add a smoke test.

@snicoll snicoll added type: task A general task and removed status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided labels Aug 27, 2021
@snicoll snicoll added this to the 2.6.x milestone Aug 27, 2021
@snicoll snicoll changed the title SessionsEndpointAutoConfiguration does not work with spring-session+hazelcast4 Add smoke test with Spring Session and Hazelcast Aug 27, 2021
@vpavic
Copy link
Contributor

vpavic commented Aug 27, 2021

Sorry, I probably should have referenced the issue in my comment as that would make it more clear.

Anyway, I think it would be a good idea to do this for all the session stores that Spring Boot's auto-configuration supports as a regression like this one can happen anywhere.

Looking at the existing spring-boot-smoke-test-session, are the profiles defined in the build of this smoke test actually leveraged anywhere in the project's build?

sessionStores[project.findProperty("sessionStore") ?: "jdbc"].each { runtimeOnly it }

@scottfrederick scottfrederick added status: first-timers-only An issue that can only be worked on by brand new contributors and removed status: reserved-for-conference-event labels Sep 1, 2021
@wilkinsona
Copy link
Member

They are not. IIRC, the profiles are a fairly direct translation of what we used to have with Maven. They need to be enabled as part of a manual build.

@vpavic
Copy link
Contributor

vpavic commented Sep 7, 2021

I see, so in reality those profiles don't add much value having in mind these are smoke tests. But I understand that historically they've been samples (or at least were named as such) so in that context profiles would be of more use.

Either way, what's your take on ensuring that Actuator integration for Spring Session gets smoke tested across all the session stores Spring Boot provides auto-config for? Even though there are some tests on Spring Session side now, the integration is owned by Spring Boot so it makes sense that things primarily get smoke tested here.

@mbhave mbhave added status: reserved-for-conference-event and removed status: first-timers-only An issue that can only be worked on by brand new contributors labels Oct 1, 2021
@kandulsh
Copy link

kandulsh commented Oct 1, 2021

Would like to work on this issue.

@mbhave
Copy link
Contributor

mbhave commented Oct 5, 2021

Closing in favor of PR #28173.

@mbhave mbhave closed this as completed Oct 5, 2021
@mbhave mbhave removed this from the 2.6.x milestone Oct 5, 2021
@mbhave mbhave added the status: superseded An issue that has been superseded by another label Oct 5, 2021
@vpavic
Copy link
Contributor

vpavic commented Oct 8, 2021

I think this should be reopened (and a bit repurposed) because the my previous comment raised some concerns that weren't addressed. I really think it would be valuable to have smoke tests that verify actuator integration for all the supported session stores.

@mbhave
Copy link
Contributor

mbhave commented Oct 11, 2021

I've added a new issue for MongoDB and Redis.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: reserved-for-conference-event status: superseded An issue that has been superseded by another type: task A general task
Projects
None yet
Development

No branches or pull requests

9 participants