From f0a2d2156e0ad6061327c6c8cf9f5ed8c0308647 Mon Sep 17 00:00:00 2001 From: Jan Kessler Date: Mon, 4 Mar 2024 10:43:54 +0100 Subject: [PATCH 1/5] in create call, now check whether meeting is already running before finding available servers --- app/controllers/bigbluebutton_api_controller.rb | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/app/controllers/bigbluebutton_api_controller.rb b/app/controllers/bigbluebutton_api_controller.rb index d140739f..847be475 100644 --- a/app/controllers/bigbluebutton_api_controller.rb +++ b/app/controllers/bigbluebutton_api_controller.rb @@ -160,9 +160,17 @@ def create params.require(:meetingID) begin - server = Server.find_available(params[:'meta_server-tag']) - rescue ApplicationRedisRecord::RecordNotFound => e - raise InternalError, e.message + # Check if meeting is already running + meeting = Meeting.find(params[:meetingID], @tenant&.id) + server = meeting.server + logger.debug("Found existing meeting #{params[:meetingID]} on BigBlueButton server #{server.id}.") + rescue ApplicationRedisRecord::RecordNotFound + # Find available server to create meeting on + begin + server = Server.find_available(params[:'meta_server-tag']) + rescue ApplicationRedisRecord::RecordNotFound => e + raise InternalError, e.message + end end # Create meeting in database From b1e49cd3b14c80f1f7408a0e2b6e45529f5b966b Mon Sep 17 00:00:00 2001 From: Jan Kessler Date: Mon, 4 Mar 2024 11:43:07 +0100 Subject: [PATCH 2/5] even more streamlining of the create call handling --- .../bigbluebutton_api_controller.rb | 47 +++++++++---------- 1 file changed, 22 insertions(+), 25 deletions(-) diff --git a/app/controllers/bigbluebutton_api_controller.rb b/app/controllers/bigbluebutton_api_controller.rb index 847be475..79bcd3d7 100644 --- a/app/controllers/bigbluebutton_api_controller.rb +++ b/app/controllers/bigbluebutton_api_controller.rb @@ -165,36 +165,33 @@ def create server = meeting.server logger.debug("Found existing meeting #{params[:meetingID]} on BigBlueButton server #{server.id}.") rescue ApplicationRedisRecord::RecordNotFound - # Find available server to create meeting on begin + # Find available server and create meeting on it server = Server.find_available(params[:'meta_server-tag']) + + # Create meeting in database + logger.debug("Creating meeting #{params[:meetingID]} in database.") + moderator_pwd = params[:moderatorPW].presence || SecureRandom.alphanumeric(8) + meeting = Meeting.find_or_create_with_server!( + params[:meetingID], + server, + moderator_pwd, + params[:voiceBridge], + @tenant&.id + ) + + # Update server if meeting (unexpectedly) already existed on a different server + server = meeting.server + + logger.debug("Incrementing server #{server.id} load by 1") + server.increment_load(1) rescue ApplicationRedisRecord::RecordNotFound => e raise InternalError, e.message end end - # Create meeting in database - logger.debug("Creating meeting #{params[:meetingID]} in database.") - - moderator_pwd = params[:moderatorPW].presence || SecureRandom.alphanumeric(8) - params[:moderatorPW] = moderator_pwd - - meeting = Meeting.find_or_create_with_server!( - params[:meetingID], - server, - moderator_pwd, - params[:voiceBridge], - @tenant&.id - ) - - # Update with old server if meeting already existed in database - server = meeting.server - - logger.debug("Incrementing server #{server.id} load by 1") - server.increment_load(1) - - duration = params[:duration].to_i - + params[:moderatorPW] = meeting.moderator_pw + params[:voiceBridge] = meeting.voice_bridge params[:'meta_tenant-id'] = @tenant.id if @tenant.present? if server.tag.present? params[:'meta_server-tag'] = server.tag @@ -202,6 +199,8 @@ def create params.delete(:'meta_server-tag') end + duration = params[:duration].to_i + # Set/Overite duration if MAX_MEETING_DURATION is set and it's greater than params[:duration] (if passed) if !Rails.configuration.x.max_meeting_duration.zero? && (duration.zero? || duration > Rails.configuration.x.max_meeting_duration) @@ -209,8 +208,6 @@ def create params[:duration] = Rails.configuration.x.max_meeting_duration end - params[:voiceBridge] = meeting.voice_bridge - if @tenant&.lrs_endpoint.present? lrs_payload = LrsPayloadService.new(tenant: @tenant, secret: server.secret).call params[:'meta_secret-lrs-payload'] = lrs_payload if lrs_payload.present? From f4cfa388cfa4b4d0887bec3959ca3de9fe0d836f Mon Sep 17 00:00:00 2001 From: Jan Kessler Date: Mon, 4 Mar 2024 13:09:41 +0100 Subject: [PATCH 3/5] now preliminarily increment server load in join call (too) --- app/controllers/bigbluebutton_api_controller.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/controllers/bigbluebutton_api_controller.rb b/app/controllers/bigbluebutton_api_controller.rb index 79bcd3d7..46837c6a 100644 --- a/app/controllers/bigbluebutton_api_controller.rb +++ b/app/controllers/bigbluebutton_api_controller.rb @@ -299,6 +299,9 @@ def join logger.info("The requested meeting #{params[:meetingID]} does not exist") raise MeetingNotFoundError end + logger.debug("Incrementing server #{server.id} load by 1") + server.increment_load(1) + # Get list of params that should not be modified by join API call excluded_params = Rails.configuration.x.join_exclude_params From 681d686a143053634e68ab7b52b4251d6beb6ab7 Mon Sep 17 00:00:00 2001 From: Jan Kessler Date: Tue, 5 Mar 2024 09:21:46 +0100 Subject: [PATCH 4/5] add test for load increment on join call --- .../bigbluebutton_api_controller_test.rb | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/test/controllers/bigbluebutton_api_controller_test.rb b/test/controllers/bigbluebutton_api_controller_test.rb index e6fdc6a0..599b0311 100644 --- a/test/controllers/bigbluebutton_api_controller_test.rb +++ b/test/controllers/bigbluebutton_api_controller_test.rb @@ -1191,6 +1191,26 @@ class BigBlueButtonApiControllerTest < ActionDispatch::IntegrationTest assert_redirected_to encode_bbb_uri('join', server1.url, server1.secret, params).to_s end + test 'join increments the server load by the value of load_multiplier' do + server1 = Server.create(url: 'https://test-1.example.com/bigbluebutton/api/', + secret: 'test-1-secret', enabled: true, online: true, load: 0, load_multiplier: 7.0) + meeting = Meeting.find_or_create_with_server('test-meeting-1', server1, 'mp') + + # Reload 1 + server1 = Server.find(server1.id) + load_before_join = server1.load + + params = { meetingID: meeting.id, moderatorPW: 'mp', fullName: 'test-name' } + BigBlueButtonApiController.stub_any_instance(:verify_checksum, nil) do + get bigbluebutton_api_join_url, params: params + end + + # Reload 2 + server1 = Server.find(server1.id) + expected_load = load_before_join + 7.0 + assert_equal expected_load, server1.load + end + test 'join redirects user to the current join url with only permitted params for join' do server1 = Server.create(url: 'https://test-1.example.com/bigbluebutton/api/', secret: 'test-1-secret', enabled: true, load: 0, online: true) From b20c9f89a6af28326f7933ceb48812c69cb44235 Mon Sep 17 00:00:00 2001 From: Jan Kessler Date: Fri, 8 Mar 2024 12:34:38 +0100 Subject: [PATCH 5/5] add RSpec version of test for load increment on join --- .../bigbluebutton_api_controller_spec.rb | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/spec/requests/bigbluebutton_api_controller_spec.rb b/spec/requests/bigbluebutton_api_controller_spec.rb index bca9f8c7..bc10214e 100644 --- a/spec/requests/bigbluebutton_api_controller_spec.rb +++ b/spec/requests/bigbluebutton_api_controller_spec.rb @@ -1224,6 +1224,25 @@ expect(response).to redirect_to(encode_bbb_uri("join", server.url, server.secret, params).to_s) end + it "increments the server load by the value of load_multiplier" do + server.load_multiplier = 7.0 + server.save! + meeting = create(:meeting, server: server) + + # Reload 1 + new_server = Server.find(server.id) + load_before_join = new_server.load + + # Join + params = { meetingID: meeting.id, password: "test-password", fullName: "test-name" } + get bigbluebutton_api_join_url, params: params + + # Reload 2 + new_server = Server.find(server.id) + expected_load = load_before_join + 7.0 + expect(new_server.load).to eq(expected_load) + end + it "redirects user to the current join url with only permitted params for join" do meeting = create(:meeting, server: server) params = { meetingID: meeting.id, password: "test-password", fullName: "test-name", test1: "", test2: "" }