LTI-369: quick refactoring to the way settings are fetched from broker (

#314) * [LTI-369] quick refactoring to the way settings are fetched from broker * LTI-369: renamed function tenant_settings for fetching data * LTI-369: scoped cache for bbb_helper and used variable cache expiration * LTI-369: adjusted rspec mockups * LTI-369: rubcops on adjusted rspec mockups
bigbluebutton · May 9, 2024 · 5d12848 · 5d12848
1 parent dc65e20
commit 5d12848
Show file tree

Hide file tree

Showing 10 changed files with 50 additions and 59 deletions.
diff --git a/app/controllers/concerns/bbb_helper.rb b/app/controllers/concerns/bbb_helper.rb
@@ -115,7 +115,7 @@ def meeting_running?
 
   # Fetches all recordings for a room.
   def recordings
-    res = Rails.cache.fetch("#{@chosen_room.handler}/#{RECORDINGS_KEY}", expires_in: 30.minutes) if Rails.configuration.cache_enabled
+    res = Rails.cache.fetch("rooms/#{@chosen_room.handler}/#{RECORDINGS_KEY}", expires_in: Rails.configuration.cache_expires_in_minutes.minutes) if Rails.configuration.cache_enabled
     res ||= bbb.get_recordings(meetingID: @chosen_room.handler)
     recordings_formatted(res)
   end
@@ -152,25 +152,25 @@ def server_running?
 
   # Deletes a recording.
   def delete_recording(record_id)
-    Rails.cache.delete("#{@chosen_room.handler}/#{RECORDINGS_KEY}") if Rails.configuration.cache_enabled
+    Rails.cache.delete("rooms/#{@chosen_room.handler}/#{RECORDINGS_KEY}") if Rails.configuration.cache_enabled
     bbb.delete_recordings(record_id)
   end
 
   # Publishes a recording.
   def publish_recording(record_id)
-    Rails.cache.delete("#{@chosen_room.handler}/#{RECORDINGS_KEY}") if Rails.configuration.cache_enabled
+    Rails.cache.delete("rooms/#{@chosen_room.handler}/#{RECORDINGS_KEY}") if Rails.configuration.cache_enabled
     bbb.publish_recordings(record_id, true)
   end
 
   # Unpublishes a recording.
   def unpublish_recording(record_id)
-    Rails.cache.delete("#{@chosen_room.handler}/#{RECORDINGS_KEY}") if Rails.configuration.cache_enabled
+    Rails.cache.delete("rooms/#{@chosen_room.handler}/#{RECORDINGS_KEY}") if Rails.configuration.cache_enabled
     bbb.publish_recordings(record_id, false)
   end
 
   # Updates a recording.
   def update_recording(record_id, meta)
-    Rails.cache.delete("#{@chosen_room.handler}/#{RECORDINGS_KEY}") if Rails.configuration.cache_enabled
+    Rails.cache.delete("rooms/#{@chosen_room.handler}/#{RECORDINGS_KEY}") if Rails.configuration.cache_enabled
     meta[:recordID] = record_id
     bbb.send_api_request('updateRecordings', meta)
   end
@@ -196,7 +196,7 @@ def meeting_start_time
   end
 
   def bigbluebutton_moderator_roles
-    roles_params = bbb_moderator_roles_params(@room.tenant)
+    roles_params = tenant_setting(@room.tenant, 'bigbluebutton_moderator_roles')&.split(',')
     roles_params.presence || Rails.configuration.bigbluebutton_moderator_roles.split(',')
   end
 

diff --git a/app/controllers/concerns/broker_helper.rb b/app/controllers/concerns/broker_helper.rb
@@ -18,39 +18,22 @@ module BrokerHelper
 
   include OmniauthHelper
 
-  # Fetch tenant settings from the broker
-  def tenant_settings(options = {})
-    tenant = options[:tenant] || @room&.tenant || ''
-    bbbltibroker_url = omniauth_bbbltibroker_url("/api/v1/tenants/#{tenant}")
-    get_response = RestClient.get(bbbltibroker_url, 'Authorization' => "Bearer #{omniauth_client_token(omniauth_bbbltibroker_url)}")
-
-    JSON.parse(get_response)
+  # Fetch tenant object from the broker
+  def broker_tenant_info(tenant)
+    # tenant ||= @room&.tenant || ''
+    Rails.cache.fetch("rooms/tenant/#{tenant}", expires_in: Rails.configuration.cache_expires_in_minutes.minutes) do
+      bbbltibroker_url = omniauth_bbbltibroker_url("/api/v1/tenants/#{tenant}")
+      get_response = RestClient.get(bbbltibroker_url, 'Authorization' => "Bearer #{omniauth_client_token(omniauth_bbbltibroker_url)}")
+
+      JSON.parse(get_response)
+    end
   rescue StandardError => e
     Rails.logger.error("Could not fetch tenant credentials from broker. Error message: #{e}")
     nil
   end
 
-  # Fetch the params to use when creating the room handler
-  def handler_params(tenant)
-    tenant_settings(tenant: tenant)&.[]('settings')&.[]('handler_params')&.split(',')
-  end
-
-  # See whether shared rooms have been enabled in tenant settings. They are disabled by default.
-  def shared_rooms_enabled(tenant)
-    Rails.cache.fetch("rooms/tenant_settings/shared_rooms_enabled/#{tenant}", expires_in: 1.hour) do
-      tenant_settings(tenant: tenant)&.[]('settings')&.[]('enable_shared_rooms') == 'true' || false
-    end
-  end
-
-  def hide_build_tag(tenant)
-    tenant_settings(tenant: tenant)&.[]('settings')&.[]('hide_build_tag') == 'true' || false
-  end
-
-  def bbb_moderator_roles_params(tenant)
-    tenant_settings(tenant: tenant)&.[]('settings')&.[]('bigbluebutton_moderator_roles')&.split(',')
-  end
-
-  def room_setting_defaults(tenant)
-    tenant_settings(tenant: tenant)&.dig('settings', 'room_setting_defaults')
+  def tenant_setting(tenant, setting)
+    tenant_settings = broker_tenant_info(tenant)&.[]('settings')
+    tenant_settings&.[](setting)
   end
 end
diff --git a/app/controllers/rooms_controller.rb b/app/controllers/rooms_controller.rb
@@ -251,7 +251,7 @@ def individual_recording
   end
 
   helper_method :recording_date, :recording_length, :meeting_running?, :bigbluebutton_moderator_roles,
-                :bigbluebutton_recording_public_formats, :meeting_info, :bigbluebutton_recording_enabled, :server_running?, :shared_rooms_enabled, :hide_build_tag
+                :bigbluebutton_recording_public_formats, :meeting_info, :bigbluebutton_recording_enabled, :server_running?
 
   private
 
@@ -293,7 +293,8 @@ def set_room
 
   # If the room is using a shared code, then use the shared room's recordings and bbb link
   def set_chosen_room
-    @shared_rooms_enabled = shared_rooms_enabled(@room&.tenant)
+    # See whether shared rooms have been enabled in tenant settings. They are disabled by default.
+    @shared_rooms_enabled = tenant_setting(@room&.tenant, 'enable_shared_rooms') == 'true'
     @shared_room = Room.find_by(code: @room.shared_code, tenant: @room.tenant) if @shared_rooms_enabled && @room&.use_shared_code
 
     use_shared_room = @shared_rooms_enabled && @room&.use_shared_code && Room.where(code: @room.shared_code, tenant: @room.tenant).exists?
@@ -497,7 +498,7 @@ def room_handler(launch_params, tenant)
     input = "rooms#{tenant}"
 
     # use resource_link_id as the default param if nothing was specified in the broker settings
-    room_handler_params = handler_params(tenant).presence || ['resource_link_id']
+    room_handler_params = tenant_setting(tenant, 'handler_params')&.split(',').presence || ['resource_link_id']
 
     room_handler_params.each do |param|
       param_val = launch_params[param]

diff --git a/app/helpers/rooms_helper.rb b/app/helpers/rooms_helper.rb
@@ -16,6 +16,8 @@
 #  You should have received a copy of the GNU Lesser General Public License along
 #  with BigBlueButton; if not, see <http://www.gnu.org/licenses/>.
 module RoomsHelper
+  include BrokerHelper
+
   def autoclose_url
     'javascript:window.close();'
   end
@@ -41,4 +43,8 @@ def add_zero_maybe(num)
 
     num
   end
+
+  def hide_build_tag?
+    tenant_setting(@room.tenant, 'hide_build_tag') == 'true'
+  end
 end
diff --git a/app/models/room.rb b/app/models/room.rb
@@ -86,7 +86,7 @@ def delete_settings
 
   def initialize_setting_defaults
     # get the key value pair from the broker using the room_setting_defaults function
-    room_settings = room_setting_defaults(tenant)
+    room_settings = tenant_setting(tenant, 'room_setting_defaults')
 
     # Define default values
     defaults = {

diff --git a/app/views/shared/_footer.html.erb b/app/views/shared/_footer.html.erb
@@ -14,7 +14,7 @@
 %>
 
 <div id="footer">
-    <% unless hide_build_tag(@room.tenant) %>
+    <% unless hide_build_tag? %>
       <div class="navbar fixed-bottom">
         <div class="text-center">
           <p class>Build number <%= Rails.configuration.build_number || 'v1' %>.</p>

diff --git a/config/application.rb b/config/application.rb
@@ -59,6 +59,7 @@ class Application < Rails::Application
 
     # Settings for external services.
     config.cache_enabled = ENV.fetch('CACHE_ENABLED', 'false').casecmp?('true')
+    config.cache_expires_in_minutes = ENV.fetch('CACHE_EXPIRES_IN_MINUTES', '10').to_i
     config.external_multitenant_endpoint = ENV['EXTERNAL_MULTITENANT_ENDPOINT']
     config.external_multitenant_secret = ENV['EXTERNAL_MULTITENANT_SECRET']
     config.tenant_credentials = ENV['TENANT_CREDENTIALS'] || '{}'

diff --git a/lib/bbb/credentials.rb b/lib/bbb/credentials.rb
@@ -64,13 +64,13 @@ def tenant_info(tenant, key)
     def formatted_tenant_info(tenant)
       if @cache_enabled
         Rails.logger.debug('Cache enabled, attempt to fetch credentials from cache...')
-        cached_tenant = @cache.fetch("#{tenant}/tenantInfo", expires_in: 10.minutes)
+        cached_tenant = @cache.fetch("rooms/#{tenant}/tenantInfo", expires_in: Rails.configuration.cache_expires_in_minutes.minutes)
         return cached_tenant unless cached_tenant.nil?
       end
 
       # Get tenant info from broker
       Rails.logger.debug('No cache. Attempt to fetch credentials from broker...')
-      tenant_info = tenant_settings(tenant: tenant)
+      tenant_info = broker_tenant_info(tenant)
 
       # Get tenant credentials from TENANT_CREDENTIALS environment variable
       tenant_credentials = JSON.parse(Rails.configuration.tenant_credentials)[tenant]
@@ -109,7 +109,7 @@ def formatted_tenant_info(tenant)
         response['settings'] = tenant_settings
       end
 
-      @cache.fetch("#{tenant}/tenantInfo", expires_in: 10.minutes) do
+      @cache.fetch("rooms/#{tenant}/tenantInfo", expires_in: Rails.configuration.cache_expires_in_minutes.minutes) do
         response || { 'apiURL' => api_url, 'secret' => secret, 'settings' => tenant_settings }
       end
     end

diff --git a/spec/concerns/bbb_helper_spec.rb b/spec/concerns/bbb_helper_spec.rb
@@ -28,14 +28,14 @@
   before do
     @room = @chosen_room = create(:room)
     allow_any_instance_of(BbbHelper).to(receive(:bbb).and_return(bbb_api))
-    allow_any_instance_of(BrokerHelper).to(receive(:tenant_settings).and_return({
-                                                                                  'handler_params' => 'context_id',
-                                                                                  'hide_build_tag' => 'false',
-                                                                                  'bigbluebutton_url' => 'https://example-bbb-server.com/bigbluebutton/api',
-                                                                                  'bigbluebutton_secret' => 'supersecretsecret',
-                                                                                  'enable_shared_rooms' => 'true',
-                                                                                  'bigbluebutton_moderator_roles' => 'administrator,teacher',
-                                                                                }))
+    allow_any_instance_of(BrokerHelper).to(receive(:broker_tenant_info).and_return({
+                                                                                     'handler_params' => 'context_id',
+                                                                                     'hide_build_tag' => 'false',
+                                                                                     'bigbluebutton_url' => 'https://example-bbb-server.com/bigbluebutton/api',
+                                                                                     'bigbluebutton_secret' => 'supersecretsecret',
+                                                                                     'enable_shared_rooms' => 'true',
+                                                                                     'bigbluebutton_moderator_roles' => 'administrator,teacher',
+                                                                                   }))
   end
 
   describe 'meeting' do

diff --git a/spec/controllers/rooms_spec.rb b/spec/controllers/rooms_spec.rb
@@ -10,14 +10,14 @@
     allow_any_instance_of(RoomsController).to(receive(:authenticate_user!).and_return(:success))
     allow_any_instance_of(RoomsController).to(receive(:bbb).and_return(bbb_api))
     allow_any_instance_of(NotifyMeetingWatcherJob).to(receive(:bbb).and_return(bbb_api)) # stub actioncable processes
-    allow_any_instance_of(BrokerHelper).to(receive(:tenant_settings).and_return({
-                                                                                  'handler_params' => 'context_id',
-                                                                                  'hide_build_tag' => 'false',
-                                                                                  'bigbluebutton_url' => 'https://example-bbb-server.com/bigbluebutton/api',
-                                                                                  'bigbluebutton_secret' => 'supersecretsecret',
-                                                                                  'enable_shared_rooms' => 'true',
-                                                                                  'bigbluebutton_moderator_roles' => 'administrator,teacher',
-                                                                                }))
+    allow_any_instance_of(BrokerHelper).to(receive(:broker_tenant_info).and_return({
+                                                                                     'handler_params' => 'context_id',
+                                                                                     'hide_build_tag' => 'false',
+                                                                                     'bigbluebutton_url' => 'https://example-bbb-server.com/bigbluebutton/api',
+                                                                                     'bigbluebutton_secret' => 'supersecretsecret',
+                                                                                     'enable_shared_rooms' => 'true',
+                                                                                     'bigbluebutton_moderator_roles' => 'administrator,teacher',
+                                                                                   }))
 
     @request.session['handler'] = {
       user_params: {