From bdc35d66545f1e76a7b915410b6cc0984bc95ed5 Mon Sep 17 00:00:00 2001 From: pabloFuente Date: Fri, 4 Oct 2019 12:51:56 +0200 Subject: [PATCH] openvidu-server: SessionManager collections initialization refactoring --- .../openvidu/server/core/SessionManager.java | 47 ++++++++--------- .../server/rest/SessionRestController.java | 52 +++++++++---------- 2 files changed, 46 insertions(+), 53 deletions(-) diff --git a/openvidu-server/src/main/java/io/openvidu/server/core/SessionManager.java b/openvidu-server/src/main/java/io/openvidu/server/core/SessionManager.java index 902ea139..4a7273d3 100644 --- a/openvidu-server/src/main/java/io/openvidu/server/core/SessionManager.java +++ b/openvidu-server/src/main/java/io/openvidu/server/core/SessionManager.java @@ -173,6 +173,10 @@ public abstract class SessionManager { return this.sessionsNotActive.get(sessionId); } + public Session getSessionWithNotActive(String sessionId) { + return (sessions.get(sessionId) != null ? sessions.get(sessionId) : sessionsNotActive.get(sessionId)); + } + public Collection getSessionsWithNotActive() { Collection allSessions = new HashSet<>(); allSessions.addAll(this.sessionsNotActive.values().stream() @@ -262,24 +266,13 @@ public abstract class SessionManager { public Session storeSessionNotActive(String sessionId, SessionProperties sessionProperties) { Session sessionNotActive = new Session(sessionId, sessionProperties, openviduConfig, recordingManager); - this.sessionsNotActive.put(sessionId, sessionNotActive); - this.sessionidParticipantpublicidParticipant.putIfAbsent(sessionId, new ConcurrentHashMap<>()); - this.sessionidFinalUsers.putIfAbsent(sessionId, new ConcurrentHashMap<>()); - if (this.openviduConfig.isRecordingModuleEnabled()) { - this.sessionidAccumulatedRecordings.putIfAbsent(sessionId, new ConcurrentLinkedQueue<>()); - } - showTokens(); - return sessionNotActive; + return this.storeSessionNotActive(sessionNotActive); } public Session storeSessionNotActive(Session sessionNotActive) { final String sessionId = sessionNotActive.getSessionId(); this.sessionsNotActive.put(sessionId, sessionNotActive); - this.sessionidParticipantpublicidParticipant.putIfAbsent(sessionId, new ConcurrentHashMap<>()); - this.sessionidFinalUsers.putIfAbsent(sessionId, new ConcurrentHashMap<>()); - if (this.openviduConfig.isRecordingModuleEnabled()) { - this.sessionidAccumulatedRecordings.putIfAbsent(sessionId, new ConcurrentLinkedQueue<>()); - } + this.initializeCollections(sessionId); showTokens(); return sessionNotActive; } @@ -287,9 +280,9 @@ public abstract class SessionManager { public String newToken(String sessionId, OpenViduRole role, String serverMetadata, KurentoTokenOptions kurentoTokenOptions) throws OpenViduException { - ConcurrentHashMap map = this.sessionidTokenTokenobj.putIfAbsent(sessionId, - new ConcurrentHashMap<>()); - if (map != null) { + Map tokenMap = this.sessionidTokenTokenobj.get(sessionId); + + if (tokenMap != null) { if (!formatChecker.isServerMetadataFormatCorrect(serverMetadata)) { log.error("Data invalid format"); @@ -297,14 +290,12 @@ public abstract class SessionManager { } Token token = tokenGenerator.generateToken(sessionId, role, serverMetadata, kurentoTokenOptions); - - map.putIfAbsent(token.getToken(), token); + tokenMap.putIfAbsent(token.getToken(), token); showTokens(); return token.getToken(); } else { - this.sessionidTokenTokenobj.remove(sessionId); - log.error("sessionId [" + sessionId + "] was not found"); + log.error("sessionId [" + sessionId + "] was not found (race condition error)"); throw new OpenViduException(Code.ROOM_NOT_FOUND_ERROR_CODE, "sessionId [" + sessionId + "] not found"); } } @@ -317,12 +308,7 @@ public abstract class SessionManager { return false; } } else { - this.sessionidParticipantpublicidParticipant.putIfAbsent(sessionId, new ConcurrentHashMap<>()); - this.sessionidFinalUsers.putIfAbsent(sessionId, new ConcurrentHashMap<>()); - if (this.openviduConfig.isRecordingModuleEnabled()) { - this.sessionidAccumulatedRecordings.putIfAbsent(sessionId, new ConcurrentLinkedQueue<>()); - } - this.sessionidTokenTokenobj.putIfAbsent(sessionId, new ConcurrentHashMap<>()); + this.initializeCollections(sessionId); this.sessionidTokenTokenobj.get(sessionId).putIfAbsent(token, new Token(token, OpenViduRole.PUBLISHER, "", this.coturnCredentialsService.isCoturnAvailable() @@ -528,4 +514,13 @@ public abstract class SessionManager { sessionidTokenTokenobj.remove(sessionId); } + private void initializeCollections(String sessionId) { + this.sessionidParticipantpublicidParticipant.putIfAbsent(sessionId, new ConcurrentHashMap<>()); + this.sessionidFinalUsers.putIfAbsent(sessionId, new ConcurrentHashMap<>()); + this.sessionidTokenTokenobj.putIfAbsent(sessionId, new ConcurrentHashMap<>()); + if (this.openviduConfig.isRecordingModuleEnabled()) { + this.sessionidAccumulatedRecordings.putIfAbsent(sessionId, new ConcurrentLinkedQueue<>()); + } + } + } diff --git a/openvidu-server/src/main/java/io/openvidu/server/rest/SessionRestController.java b/openvidu-server/src/main/java/io/openvidu/server/rest/SessionRestController.java index c886bc94..63c7586f 100644 --- a/openvidu-server/src/main/java/io/openvidu/server/rest/SessionRestController.java +++ b/openvidu-server/src/main/java/io/openvidu/server/rest/SessionRestController.java @@ -153,13 +153,12 @@ public class SessionRestController { String sessionId; if (customSessionId != null && !customSessionId.isEmpty()) { - if (sessionManager.sessionidTokenTokenobj.putIfAbsent(customSessionId, new ConcurrentHashMap<>()) != null) { + if (sessionManager.getSessionWithNotActive(customSessionId) != null) { return new ResponseEntity<>(HttpStatus.CONFLICT); } sessionId = customSessionId; } else { sessionId = RandomStringUtils.randomAlphanumeric(16).toLowerCase(); - sessionManager.sessionidTokenTokenobj.putIfAbsent(sessionId, new ConcurrentHashMap<>()); } Session sessionNotActive = sessionManager.storeSessionNotActive(sessionId, sessionProperties); @@ -236,21 +235,18 @@ public class SessionRestController { log.info("REST API: DELETE /api/sessions/{}/connection/{}", sessionId, participantPublicId); - Session session = this.sessionManager.getSession(sessionId); - if (session != null) { - Participant participant = session.getParticipantByPublicId(participantPublicId); - if (participant != null) { - this.sessionManager.evictParticipant(participant, null, null, EndReason.forceDisconnectByServer); - return new ResponseEntity<>(HttpStatus.NO_CONTENT); - } else { - return new ResponseEntity<>(HttpStatus.NOT_FOUND); - } - } else { - if (this.sessionManager.getSessionNotActive(sessionId) != null) { - return new ResponseEntity<>(HttpStatus.NOT_FOUND); - } + Session session = this.sessionManager.getSessionWithNotActive(sessionId); + if (session == null) { return new ResponseEntity<>(HttpStatus.BAD_REQUEST); } + + Participant participant = session.getParticipantByPublicId(participantPublicId); + if (participant != null) { + this.sessionManager.evictParticipant(participant, null, null, EndReason.forceDisconnectByServer); + return new ResponseEntity<>(HttpStatus.NO_CONTENT); + } else { + return new ResponseEntity<>(HttpStatus.NOT_FOUND); + } } @RequestMapping(value = "/sessions/{sessionId}/stream/{streamId}", method = RequestMethod.DELETE) @@ -259,7 +255,12 @@ public class SessionRestController { log.info("REST API: DELETE /api/sessions/{}/stream/{}", sessionId, streamId); - Session session = this.sessionManager.getSession(sessionId); + Session session = this.sessionManager.getSessionWithNotActive(sessionId); + if (session == null) { + return new ResponseEntity<>(HttpStatus.BAD_REQUEST); + } + + session = this.sessionManager.getSession(sessionId); if (session != null) { if (this.sessionManager.unpublishStream(session, streamId, null, null, EndReason.forceUnpublishByServer)) { return new ResponseEntity<>(HttpStatus.NO_CONTENT); @@ -267,10 +268,7 @@ public class SessionRestController { return new ResponseEntity<>(HttpStatus.NOT_FOUND); } } else { - if (this.sessionManager.getSessionNotActive(sessionId) != null) { - return new ResponseEntity<>(HttpStatus.NOT_FOUND); - } - return new ResponseEntity<>(HttpStatus.BAD_REQUEST); + return new ResponseEntity<>(HttpStatus.NOT_FOUND); } } @@ -300,6 +298,11 @@ public class SessionRestController { HttpStatus.BAD_REQUEST); } + if (this.sessionManager.getSessionWithNotActive(sessionId) == null) { + return this.generateErrorResponse("Session " + sessionId + " not found", "/api/tokens", + HttpStatus.NOT_FOUND); + } + JsonObject kurentoOptions = null; if (params.get("kurentoOptions") != null) { @@ -335,13 +338,8 @@ public class SessionRestController { metadata = (metadata != null) ? metadata : ""; - String token; - try { - token = sessionManager.newToken(sessionId, role, metadata, kurentoTokenOptions); - } catch (OpenViduException e) { - // Session was not found - return this.generateErrorResponse(e.getMessage(), "/api/tokens", HttpStatus.NOT_FOUND); - } + String token = sessionManager.newToken(sessionId, role, metadata, kurentoTokenOptions); + JsonObject responseJson = new JsonObject(); responseJson.addProperty("id", token); responseJson.addProperty("session", sessionId);