Add actuator endpoint for finding and deleting sessions #8342

vpavic · 2017-02-20T22:26:47Z

This PR is an alternative approach to implementing sessions endpoint to one proposed in #5730.

Key differences:

unlike Add session endpoints #5730, this implementation doesn't provide Endpoint implementation i.e. it only provides MVC and JMX endpoints that simply delegate to Spring Session's FindByIndexNameSessionRepository (this is analogous to audit events endpoints - see Add actuator endpoint to retrieve audit events #6579)
JMX endpoint is based on support for JmxEndpoint implementations without a backing Endpoint (introduced in Add actuator endpoint to retrieve audit events #6579)
Actuator docs

vpavic · 2017-09-18T06:02:34Z

I've adapted this PR to the new Actuator endpoint infrastructure. Hands-on developer experience with the new stuff is just great!

snicoll

Thanks for the PR @vpavic!

I've made a few suggestions (which might lead us to add another extension point in the Endpoint API).

Also, I wonder if a sample wouldn't be in order for this? This can happen in a separate PR for sure.

snicoll · 2017-09-19T13:25:14Z

...tor/src/main/java/org/springframework/boot/actuate/session/SessionsWebEndpointExtension.java

+	@ReadOperation
+	public WebEndpointResponse<SessionsReport> sessionsForUsername(String username) {
+		if (username == null) {
+			return new WebEndpointResponse<>(HttpStatus.BAD_REQUEST.value());


You can't use HttpStatus here as this class may not be available when running with Jersey. That's why that object is taking the raw int code.

Web framework neutrality completely slipped my mind when using HttpStatus, I'll address that shortly.

Note that I used the same pattern in AuditEventsWebEndpointExtension that was introduced in #10322. Quick search also revealed another usage in HeapDumpWebEndpoint.

Thanks for the feedback. I've created #10348

snicoll · 2017-09-19T13:35:26Z

...t-actuator/src/test/java/org/springframework/boot/actuate/session/SessionsEndpointTests.java

+
+	@Test
+	public void deleteSession() {
+		this.endpoint.deleteSession(session.getId());


That test does nothing. Verify that the repository was called with the right argument would be in order:

verify(this.repository).deleteById(session.getId());

snicoll · 2017-09-19T13:35:52Z

spring-boot-docs/src/main/asciidoc/production-ready-features.adoc

@@ -106,6 +106,10 @@ The following technology agnostic endpoints are available:
 |`mappings`
 |Displays a collated list of all `@RequestMapping` paths.

+|`sessions`
+|Allows retrieving user's sessions from Spring Session backed session store, and deleting
+them.


I'd add that to the first part of the sentence (see above for an example).

vpavic · 2017-09-19T17:22:26Z

Thanks for the review @snicoll, I've addressed your concerns.

Regarding the sample remark, would it be OK to reuse the existing spring-boot-sample-session-redis to showcase this functionality?

snicoll · 2017-09-19T17:47:37Z

Yes. Perhaps we could first rename it and use something else. jdbc comes to mind even I know you guys don't want to promote it too much?

That sample looks outdated as it requires a redis server.

vpavic · 2017-09-19T17:58:32Z

Can we use the similar approach to one in spring-boot-sample-cache and have multiple build profiles?

snicoll · 2017-09-19T18:18:15Z

Maybe. Not sure that's an immediate goal but I am happy to review a PR

vpavic · 2017-09-19T19:35:41Z

@snicoll I've opened #10351 to improve the sample.

vpavic · 2017-09-20T12:46:44Z

I've updated the PR to adopt to changes from #10350 and enable the new endpoint in spring-boot-sample-session.

vpavic · 2017-09-20T13:12:54Z

Transient errors involving WebFluxEndpointIntegrationTests like the one below are quite often on Travis CI lately. It's been basically 50-50 with my PR updates over the past few days.

[INFO] Results:
[INFO] 
[ERROR] Errors: 
[ERROR]   WebFluxEndpointIntegrationTests>AbstractWebEndpointIntegrationTests.writeOperation:165->AbstractWebEndpointIntegrationTests.load:312->AbstractWebEndpointIntegrationTests.load:300->AbstractWebEndpointIntegrationTests.lambda$load$24:313->AbstractWebEndpointIntegrationTests.lambda$writeOperation$11:170 » IllegalState
[INFO] 
[ERROR] Tests run: 440, Failures: 0, Errors: 1, Skipped: 1

wilkinsona · 2017-09-20T14:02:49Z

We see those on Bamboo too. @bclozel has been investigating. My suspicion is that it's a Linux-specific problem in Netty or Reactor Netty.

vpavic · 2017-09-20T14:08:19Z

Thanks for the feedback. FWIW I didn't hit those problems on Ubuntu 17.04.

See gh-8342

* pr/8342: Polish "Add actuator endpoint for finding and deleting sessions" Add actuator endpoint for finding and deleting sessions

manderson23 · 2018-02-26T14:06:42Z

@vpavic was there a reason that the endpoint doesn't have the option of returning the IDs of all sessions within the repository?

wilkinsona · 2018-02-26T15:03:59Z

There's no support in Spring Session for getting the IDs of all sessions known to the repository.

spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 20, 2017

philwebb added this to the 2.0.0.M3 milestone Apr 27, 2017

philwebb added priority: normal type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Apr 27, 2017

vpavic mentioned this pull request Apr 28, 2017

Actuator Endpoint to Delete Sessions spring-projects/spring-session#407

Closed

vpavic force-pushed the sessions-endpoint branch 4 times, most recently from b82bfd9 to 8e9ce60 Compare June 26, 2017 16:28

vpavic force-pushed the sessions-endpoint branch from 8e9ce60 to 297e675 Compare June 27, 2017 09:50

snicoll modified the milestones: 2.0.0.M3, 2.0.0.M4 Jul 13, 2017

vpavic force-pushed the sessions-endpoint branch from 297e675 to c8599f7 Compare July 27, 2017 21:02

wilkinsona modified the milestones: 2.0.0.M4, 2.0.0.M5 Jul 28, 2017

vpavic mentioned this pull request Aug 17, 2017

Add support for delete operation to the actuator endpoint API #10023

Closed

vpavic force-pushed the sessions-endpoint branch 4 times, most recently from d0e19a7 to ff4d0f6 Compare September 17, 2017 17:51

snicoll reviewed Sep 19, 2017

View reviewed changes

snicoll added the status: waiting-for-feedback We need additional information before we can continue label Sep 19, 2017

vpavic force-pushed the sessions-endpoint branch from ff4d0f6 to d007994 Compare September 19, 2017 17:19

vpavic force-pushed the sessions-endpoint branch from d007994 to 07b72a6 Compare September 19, 2017 17:24

vpavic force-pushed the sessions-endpoint branch from 07b72a6 to 49348ef Compare September 20, 2017 07:03

Add actuator endpoint for finding and deleting sessions

661f9ca

vpavic force-pushed the sessions-endpoint branch from 49348ef to 661f9ca Compare September 20, 2017 12:43

snicoll removed the status: waiting-for-feedback We need additional information before we can continue label Sep 20, 2017

snicoll self-assigned this Sep 20, 2017

snicoll pushed a commit that referenced this pull request Sep 20, 2017

Add actuator endpoint for finding and deleting sessions

cf151b1

See gh-8342

snicoll closed this in 27f2222 Sep 20, 2017

snicoll added a commit that referenced this pull request Sep 20, 2017

Merge pull request #8342 from vpavic:sessions-endpoint

56d82eb

* pr/8342: Polish "Add actuator endpoint for finding and deleting sessions" Add actuator endpoint for finding and deleting sessions

snicoll mentioned this pull request Sep 20, 2017

Add session endpoints #5730

Closed

1 task

vpavic deleted the sessions-endpoint branch September 20, 2017 17:36

Add actuator endpoint for finding and deleting sessions #8342

Add actuator endpoint for finding and deleting sessions #8342

Uh oh!

Conversation

vpavic commented Feb 20, 2017

Uh oh!

vpavic commented Sep 18, 2017

Uh oh!

snicoll left a comment

Choose a reason for hiding this comment

Uh oh!

snicoll Sep 19, 2017 • edited by wilkinsona Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vpavic Sep 19, 2017

Choose a reason for hiding this comment

Uh oh!

snicoll Sep 19, 2017

Choose a reason for hiding this comment

Uh oh!

snicoll Sep 19, 2017

Choose a reason for hiding this comment

Uh oh!

snicoll Sep 19, 2017

Choose a reason for hiding this comment

Uh oh!

vpavic commented Sep 19, 2017

Uh oh!

snicoll commented Sep 19, 2017

Uh oh!

vpavic commented Sep 19, 2017

Uh oh!

snicoll commented Sep 19, 2017

Uh oh!

vpavic commented Sep 19, 2017

Uh oh!

vpavic commented Sep 20, 2017

Uh oh!

vpavic commented Sep 20, 2017

Uh oh!

wilkinsona commented Sep 20, 2017

Uh oh!

vpavic commented Sep 20, 2017

Uh oh!

manderson23 commented Feb 26, 2018

Uh oh!

wilkinsona commented Feb 26, 2018

Uh oh!

Uh oh!

snicoll Sep 19, 2017 •

edited by wilkinsona

Loading