Merge branch 'release-3.2.x' into refactor-old-code

CHANGELOG.md

@@ -1,5 +1,19 @@
 # Change Log
 
+## [3.2.0] - 2021-06-15
+* [#198] Fix deletion of meetings with recordings.
+  When deleting a meeting with recording,
+  sometimes errors occurred and the recording was kept.
+  Now these errors are rescued and both are deleted.
+* [#197] Now the meetings controller can be overriden on apps if necessary.
+* [#196] Fix race condition of workers and improve performance.
+  Now workers will run sync for each rooms, not for each server, and 
+  will run 16 times for about 24h after a meeting end, 
+  applications can easily customize the intervals between each job
+* Fix migration.rb:
+    - Add missing `BigbluebuttonRecording#state` attribute
+
+
 ## [3.1.2] - 2021-05-15
 
 * [#190] Increase the size of `BigbluebuttonMeeting#title` from 80 to 255 characters. It was a column
@@ -379,6 +393,7 @@ https://github.com/mconf/bigbluebutton_rails/wiki/Migrate-to-1.3.0
 * Controller to access servers and rooms
 * rooms_controller interacts with a BBB server using bigbluebutton-api-ruby
 
+[3.2.0]: https://github.com/mconf/bigbluebutton_rails/compare/v3.1.2...v3.2.0
 [3.1.2]: https://github.com/mconf/bigbluebutton_rails/compare/v3.1.1...v3.1.2
 [3.1.1]: https://github.com/mconf/bigbluebutton_rails/compare/v3.1.0...v3.1.1
 [3.1.0]: https://github.com/mconf/bigbluebutton_rails/compare/v3.0.1...v3.1.0

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    bigbluebutton_rails (3.1.2)
+    bigbluebutton_rails (3.2.0)
       activerecord-import (~> 1.0)
       bigbluebutton-api-ruby (~> 1.6)
       browser (~> 0.8.0)

app/controllers/bigbluebutton/meetings_controller.rb

@@ -24,7 +24,7 @@ def destroy
           }
         end
       else
-        flash[:error] = t('bigbluebutton_rails.meetings.notice.destroy.error_destroy')
+        flash[:error] = t('bigbluebutton_rails.meetings.notice.destroy.error')
         redirect_to_using_params_or_back(request.referer)
       end
     else

app/controllers/bigbluebutton/recordings_controller.rb

@@ -45,24 +45,14 @@ def update
   end
 
   def destroy
-    error = false
-    begin
-      @recording.destroy
-      message = t('bigbluebutton_rails.recordings.notice.destroy.success')
-    rescue BigBlueButton::BigBlueButtonException => e
-      error = true
-      message = t('bigbluebutton_rails.recordings.notice.destroy.success_with_bbb_error')
+    if @recording.destroy
+      flash[:success] = t('bigbluebutton_rails.recordings.notice.destroy.success')
+    else
+      flash[:error] = t('bigbluebutton_rails.recordings.notice.destroy.error')
     end
 
     respond_with do |format|
-      format.html {
-        if error
-          flash[:error] = message
-          redirect_to_using_params bigbluebutton_recordings_url
-        else
-          redirect_to_using_params bigbluebutton_recordings_url, :notice => message
-        end
-      }
+      format.html { redirect_to_using_params bigbluebutton_recordings_url }
     end
   end
 

app/models/bigbluebutton_recording.rb

@@ -72,7 +72,12 @@ def default_playback_format
   # Remove this recording from the server
   def delete_from_server!
     if self.server.present?
-      self.server.send_delete_recordings(self.recordid)
+      begin
+        self.server.send_delete_recordings(self.recordid)
+      rescue BigBlueButton::BigBlueButtonException => e
+        logger.error "Could not delete the recording #{self.id} from the server. API error: #{e}"
+        return false
+      end
     else
       false
     end
@@ -106,7 +111,7 @@ def self.recording_changed?(recording, data)
       # the attributes that are considered in the comparison
       keys = [ # rawSize is not stored at the moment
         :end_time, :meetingid,  :metadata, :playback, :published,
-        :recordid, :size, :start_time
+        :recordid, :size, :start_time, :state, :name
       ]
       keys_formats = [ # :size, :processingTime are not stored at the moment
         :length, :type, :url
@@ -153,53 +158,71 @@ def self.recording_changed?(recording, data)
   # Will add new recordings that are not in the db yet and update the ones that
   # already are (matching by 'recordid'). Will NOT delete recordings from the db
   # if they are not in the array but instead mark them as unavailable.
-  # 'server' is the BigbluebuttonServer object from which the recordings
-  # were fetched.
+  #
+  # server:: The BigbluebuttonServer from which the recordings were fetched.
+  # recordings:: The response from getRecordings.
+  # sync_scope:: The scope to which these recordings are part of. If we fetched all recordings
+  #   in a server, the scope is all recordings in the server; if we fetched only recordings
+  #   for a room, the scope is the recordings of this room. This is used to set `available`
+  #   in the recordings that might not be in the server anymore.
+  # sync_started_at:: Moment when the getRecordings call that returned the `recordings`
+  #   was made. Used so we don't set `available` on recordings created during the
+  #   synchronization process.
   #
   # TODO: catch exceptions on creating/updating recordings
-  def self.sync(server, recordings, full_sync=false)
+  def self.sync(server, recordings, sync_scope=nil, sync_started_at=nil)
+    logger.info "Sync recordings: starting a sync for server=#{server.url};#{server.secret} sync_scope=\"#{sync_scope&.to_sql}\""
+
     recordings.each do |rec|
-      rec_obj = BigbluebuttonRecording.find_by_recordid(rec[:recordID])
+      rec_obj = BigbluebuttonRecording.find_by(recordid: rec[:recordID])
       rec_data = adapt_recording_hash(rec)
-      changed = !rec_obj.present? ||
-                self.recording_changed?(rec_obj, rec_data)
+      changed = !rec_obj.present? || self.recording_changed?(rec_obj, rec_data)
 
       if changed
+        logger.info "Sync recordings: detected that the recording changed #{rec[:recordID]}"
         BigbluebuttonRecording.transaction do
           if rec_obj
-            logger.info "Sync recordings: updating recording #{rec_obj.inspect}"
-            logger.debug "Sync recordings: recording data #{rec_data.inspect}"
+            logger.info "Sync recordings: updating recording #{rec[:recordID]}"
+            logger.debug "Sync recordings: updating recording with data #{rec_data.inspect}"
             self.update_recording(server, rec_obj, rec_data)
           else
-            logger.info "Sync recordings: creating recording"
-            logger.debug "Sync recordings: recording data #{rec_data.inspect}"
+            logger.info "Sync recordings: creating recording #{rec[:recordID]}"
+            logger.debug "Sync recordings: creating recording with data #{rec_data.inspect}"
             self.create_recording(server, rec_data)
           end
         end
       end
     end
     cleanup_playback_types
 
-    # set as unavailable the recordings that are not in 'recordings', but
-    # only in a full synchronization process, which means that the recordings
-    # in `recordings` are *all* available in `server`, not a subset.
-    if full_sync
+    # Set as unavailable the recordings that are not in the list returned by getRecordings, but
+    # only if there is a scope set, otherwise we don't know how the call to getRecordings was filtered.
+    # Uses the scope passed to figure out which recordings should be marked as unavailable.
+    if sync_scope.present?
+      sync_started_at = DateTime.now if sync_started_at.nil?
+
       recordIDs = recordings.map{ |rec| rec[:recordID] }
-      if recordIDs.length <= 0 # empty response
-        BigbluebuttonRecording.
-          where(available: true, server: server).
+      if recordIDs.length <= 0
+        # empty response, all recordings in the scope are unavailable
+        sync_scope.
+          where(available: true).
+          where("created_at <= ?", sync_started_at).
           update_all(available: false)
       else
-        BigbluebuttonRecording.
-          where(available: true, server: server).
-          where.not(recordid: recordIDs).
+        # non empty response, mark as unavailable all recordings in the scope that
+        # were not returned by getRecording
+        sync_scope.
+          where(available: true).where.not(recordid: recordIDs).
+          where("created_at <= ?", sync_started_at).
           update_all(available: false)
-        BigbluebuttonRecording.
-          where(available: false, server: server).
-          where(recordid: recordIDs).
+        sync_scope.
+          where(available: false, recordid: recordIDs).
+          where("created_at <= ?", sync_started_at).
           update_all(available: true)
       end
     end
+
+    logger.info "Sync recordings: finished a sync for server=#{server.url};#{server.secret} sync_scope=\"#{sync_scope&.to_sql}\""
   end
 
   protected

app/models/bigbluebutton_room.rb

@@ -393,11 +393,9 @@ def finish_meetings
 
     if to_be_finished.count > 0
       # start trying to get the recording for this room
-      # since we don't have a way to know exactly when a recording is done, we
-      # have to keep polling the server for them
-      # 3 times so it tries at: 4, 9, 14 and 19
-      # no point trying more since there is a global synchronization process
-      Resque.enqueue_in(1.minutes, ::BigbluebuttonRecordingsForRoomWorker, self.id, 10)
+      intervals  = BigbluebuttonRails.configuration.recording_sync_for_room_intervals
+      tries = intervals.length - 1
+      Resque.enqueue_in(intervals[0], ::BigbluebuttonRecordingsForRoomWorker, self.id, tries)
     end
   end
 
@@ -425,10 +423,14 @@ def self.generate_dial_number(pattern=nil)
     end
   end
 
-  def fetch_recordings(filter={})
+  # Synchronizes all the recordings for this room. Will only get recordings with the
+  # default states (won't get recordings with the state 'deleted', for instance).
+  def fetch_recordings
     server = BigbluebuttonRails.configuration.select_server.call(self, :get_recordings)
     if server.present?
-      server.fetch_recordings(filter.merge({ meetingID: self.meetingid }))
+      states = BigbluebuttonRecording::STATES.values
+      scope = BigbluebuttonRecording.where(room: self, state: states)
+      server.fetch_recordings({ meetingID: self.meetingid, state: states }, scope)
       true
     else
       false

app/models/bigbluebutton_server.rb

@@ -133,12 +133,20 @@ def send_delete_recordings(ids)
   #          metadata values.
   #
   # Triggers API call: <tt>getRecordings</tt>.
-  def fetch_recordings(filter=nil, full_sync=false)
+  def fetch_recordings(filter=nil, sync_scope=nil)
     filter ||= {}
-    logger.info "Fetching recordings for the server #{self.inspect} with filter: #{filter.inspect}"
+    logger.info "Fetching recordings on #{self.url} with filter: #{filter.inspect}"
+
+    sync_started_at = DateTime.now
     recordings = self.api.get_recordings(filter)
+
     if recordings and recordings[:recordings]
-      BigbluebuttonRecording.sync(self, recordings[:recordings], full_sync)
+
+      # if no scope is set and there are no filters, set the scope to all recordings
+      # in this server
+      sync_scope = BigbluebuttonRecording.where(server: self) if filter.blank? && sync_scope.nil?
+
+      BigbluebuttonRecording.sync(self, recordings[:recordings], sync_scope, sync_started_at)
     end
   end
 

app/workers/bigbluebutton_recordings_for_room_worker.rb

@@ -12,16 +12,26 @@ class BigbluebuttonRecordingsForRoomWorker
   @queue = :bigbluebutton_rails
 
   def self.perform(room_id, tries_left=0)
-    Rails.logger.info "BigbluebuttonRecordingsForRoomWorker worker running"
+    return if tries_left <= 0
+
+    Rails.logger.info "BigbluebuttonRecordingsForRoomWorker worker running " \
+                      "room_id=#{room_id} tries_left=#{tries_left}"
 
     room = BigbluebuttonRoom.find(room_id)
     if room.present?
-      Rails.logger.info "BigbluebuttonRecordingsForRoomWorker getting recordings for #{room.inspect}"
-      room.fetch_recordings( state: BigbluebuttonRecording::STATES.values)
+      Rails.logger.info "BigbluebuttonRecordingsForRoomWorker getting recordings for meetingid=#{room.meetingid}"
+
+      room.fetch_recordings
 
-      if tries_left > 0
-        Resque.enqueue_in(5.minutes, ::BigbluebuttonRecordingsForRoomWorker, room_id, tries_left - 1)
-      end
+      intervals = BigbluebuttonRails.configuration.recording_sync_for_room_intervals
+      idx = intervals.length - tries_left
+      wait = intervals[idx]
+      wait = intervals[intervals.length - 1] if wait.nil?
+
+      Resque.enqueue_in(wait, ::BigbluebuttonRecordingsForRoomWorker, room_id, tries_left - 1)
     end
+
+    Rails.logger.info "BigbluebuttonRecordingsForRoomWorker worker ended " \
+                      "room_id=#{room_id} tries_left=#{tries_left}"
   end
 end

app/workers/bigbluebutton_update_recordings_worker.rb

@@ -6,6 +6,10 @@ class BigbluebuttonUpdateRecordingsWorker
 
   def self.perform(server_id=nil)
     Rails.logger.info "BigbluebuttonUpdateRecordingsWorker worker running"
-    BigbluebuttonRails::BackgroundTasks.update_recordings(server_id)
+
+    query = BigbluebuttonRails.configuration.rooms_for_full_recording_sync.call
+    BigbluebuttonRails::BackgroundTasks.update_recordings_by_room(query)
+
+    Rails.logger.info "BigbluebuttonUpdateRecordingsWorker worker ended"
   end
 end

config/locales/en.yml

@@ -93,7 +93,7 @@ en:
           failure: "It was not possible to update your meeting record."
         destroy:
           success_with_bbb_error: "The record was successfully destroyed but it wasn't deleted from the videoconference server (\"%{error}\")"
-          error_destroy: "This record could not be deleted"
+          error: "This record could not be deleted"
           running:
             not_ended: "This record can not be deleted because the meeting is still running"
     metadata:
@@ -126,6 +126,7 @@ en:
         destroy:
           success: "Recording destroyed!"
           success_with_bbb_error: "The recording was successfully destroyed but it wasn't deleted from the webconference server."
+          error: "This recording could not be deleted."
         publish:
           success: "Recording published!"
         unpublish:
@@ -155,6 +156,7 @@ en:
         destroy:
           success: "Room destroyed."
           success_with_bbb_error: "The room was successfully destroyed but the meeting wasn't ended in the webconference server."
+          error: "This room could not be deleted."
         end:
           not_running: "The meeting could not be ended because it is not running."
           success: "The meeting was successfully ended."

config/locales/pt-br.yml

@@ -76,7 +76,7 @@ pt-br:
           failure: "Não foi possível atualizar seu registro de reunião."
         destroy:
           success_with_bbb_error: "O registro foi destruído com sucesso mas não foi removido do servidor de videoconferência (\"%{error}\")"
-          error_destroy: "Não foi possível deletar este registro."
+          error: "Não foi possível deletar este registro."
           running:
             not_ended: "Este registro não pode ser deletado pois a reunião ainda está acontecendo."
     metadata:
@@ -109,6 +109,7 @@ pt-br:
         destroy:
           success: "Gravação destruida!"
           success_with_bbb_error: "A gravação foi destruída com sucesso mas não foi removida do servidor de webconferência."
+          error: "Não foi possível deletar esta gravação."
         publish:
           success: "Gravação publicada!"
         unpublish:
@@ -138,6 +139,7 @@ pt-br:
         destroy:
           success: "Reunião destruída!"
           success_with_bbb_error: "A sala foi destruída com sucesso mas a reunião não foi finalizada no servidor de webconferência."
+          error: "Não foi possível deletar esta sala."
         end:
           not_running: "A reunião não pôde ser finalizada pois não está em andamento."
           success: "A reunião foi finalizada com sucesso."

config/resque/workers_schedule.yml

@@ -5,7 +5,6 @@ finish_meetings:
   description: "Checks for meetings that finished and mark as finished. Same as 'rake bigbluebutton_rails:meetings:finish'."
 
 update_recordings:
-  every:
-    - "30m"
+  cron: "0 0,12 * * *" # twice a day
   class: BigbluebuttonUpdateRecordingsWorker
   description: "Gets the recordings in the server to populate the db. Same as 'rake bigbluebutton_rails:recordings:update'."