Merge pull request #200 from mconf/refactor-old-code

Remove running attribute from BigbluebuttonRoom model

app/controllers/bigbluebutton/api/rooms_controller.rb

@@ -45,7 +45,7 @@ def index
   end
 
   def running
-    check_is_running
+    @running = check_is_running
   end
 
   def join
@@ -89,12 +89,11 @@ def join_user_params
 
   def check_is_running
     begin
-      @room.fetch_is_running?
+      @room.is_running?
     rescue StandardError => e
       @errors = [BigbluebuttonRails::APIError.new(e.to_s, 500)]
-      render 'bigbluebutton/api/error'
+      render 'bigbluebutton/api/error' and return false
     end
-    @room.is_running?
   end
 
   def set_content_type

app/controllers/bigbluebutton/rooms_controller.rb

@@ -79,7 +79,6 @@ def update
   def destroy
     error = false
     begin
-      @room.fetch_is_running?
       @room.send_end if @room.is_running?
       message = t('bigbluebutton_rails.rooms.notice.destroy.success')
     rescue BigBlueButton::BigBlueButtonException => e
@@ -120,19 +119,18 @@ def invite
 
   def running
     begin
-      @room.fetch_is_running?
+      running = @room.is_running?
       info = @room.fetch_meeting_info
     rescue BigBlueButton::BigBlueButtonException => e
       render :json => { :running => "false", :error => "#{api_error_msg(e)}" }
     else
-      render :json => { :running => "#{@room.is_running?}", :meeting_info => info}
+      render :json => { :running => "#{running}", :meeting_info => info}
     end
   end
 
   def end
     error = false
     begin
-      @room.fetch_is_running?
       if @room.is_running?
         @room.send_end
         message = t('bigbluebutton_rails.rooms.notice.end.success')
@@ -273,7 +271,7 @@ def join_user_params
   # Aborts and redirects to an error if the user can't create a meeting in
   # the room and it needs to be created.
   def join_check_can_create
-    unless @room.fetch_is_running?
+    unless @room.is_running?
       unless bigbluebutton_can_create?(@room, @user_role)
         flash[:error] = t('bigbluebutton_rails.rooms.errors.join.cannot_create')
         redirect_to_on_join_error
@@ -318,7 +316,7 @@ def redirect_to_on_join_error
   def join_internal(username, role, id)
     begin
       # first check if we have to create the room and if the user can do it
-      unless @room.fetch_is_running?
+      unless @room.is_running?
         if bigbluebutton_can_create?(@room, role)
           if @room.create_meeting(bigbluebutton_user, request)
             logger.info "Meeting created: id: #{@room.meetingid}, name: #{@room.name}, created_by: #{username}, time: #{Time.now.iso8601}"

app/models/bigbluebutton_meeting.rb

@@ -50,7 +50,7 @@ def self.create_meeting_record_from_room(room, response, server, user, user_opts
           title: room.name,
           recorded: room.record_meeting,
           create_time: room.create_time,
-          running: room.running,
+          running: response[:running],
           ended: false,
           internal_meeting_id: response[:internalMeetingID]
         }

app/models/bigbluebutton_room.rb

@@ -51,7 +51,7 @@ class BigbluebuttonRoom < ActiveRecord::Base
   validates :moderator_key, :presence => true, :if => :private?
 
   # Note: these params need to be fetched from the server before being accessed
-  attr_accessor :running, :participant_count, :moderator_count, :current_attendees,
+  attr_accessor :participant_count, :moderator_count, :current_attendees,
                 :has_been_forcibly_ended, :end_time
 
   after_initialize :init
@@ -95,7 +95,8 @@ class BigbluebuttonRoom < ActiveRecord::Base
 
   # Convenience method to access the attribute <tt>running</tt>
   def is_running?
-    @running
+    # TODO: cache this maybe?
+    fetch_is_running?
   end
 
   # Fetches info from BBB about this room.
@@ -107,7 +108,6 @@ def is_running?
   # The attributes changed are:
   # * <tt>participant_count</tt>
   # * <tt>moderator_count</tt>
-  # * <tt>running</tt>
   # * <tt>has_been_forcibly_ended</tt>
   # * <tt>create_time</tt>
   # * <tt>end_time</tt>
@@ -121,7 +121,6 @@ def fetch_meeting_info
 
       @participant_count = response[:participantCount]
       @moderator_count = response[:moderatorCount]
-      @running = response[:running]
       @has_been_forcibly_ended = response[:hasBeenForciblyEnded]
       @end_time = response[:endTime]
       @current_attendees = []
@@ -135,8 +134,8 @@ def fetch_meeting_info
 
       # a 'shortcut' to update meetings since we have all information we need
       # if we got here, it means the meeting is still in the server, so it's not ended
-      self.update_attributes(create_time: response[:createTime]) unless self.new_record?
-      self.update_current_meeting_record(response[:metadata], true)
+      update_attributes(create_time: response[:createTime]) unless self.new_record?
+      update_current_meeting_record(response, true)
 
     rescue BigBlueButton::BigBlueButtonException => e
       # note: we could catch only the 'notFound' error, but there are complications, so
@@ -156,7 +155,7 @@ def fetch_meeting_info
   # Triggers API call: <tt>isMeetingRunning</tt>.
   def fetch_is_running?
     server = BigbluebuttonRails.configuration.select_server.call(self, :is_meeting_running)
-    @running = server.api.is_meeting_running?(self.meetingid)
+    server.api.is_meeting_running?(self.meetingid)
   end
 
   # Sends a call to the BBB server to end the meeting.
@@ -288,7 +287,7 @@ def user_role(params)
   # if they are have all equal values.
   # From: http://alicebobandmallory.com/articles/2009/11/02/comparing-instance-variables-in-ruby
   def instance_variables_compare(o)
-    vars = [ :@running, :@participant_count, :@moderator_count, :@current_attendees,
+    vars = [ :@participant_count, :@moderator_count, :@current_attendees,
              :@has_been_forcibly_ended, :@end_time ]
     Hash[*vars.map { |v|
            self.instance_variable_get(v)!=o.instance_variable_get(v) ?
@@ -311,7 +310,6 @@ def to_param
   # Will create the meeting in this room unless it is already running.
   # Returns true if the meeting was created.
   def create_meeting(user=nil, request=nil)
-    fetch_is_running?
     unless is_running?
       add_domain_to_logout_url(request.protocol, request.host_with_port) unless request.nil?
       send_create(user)
@@ -353,16 +351,13 @@ def get_current_meeting
   end
 
   # Updates the current meeting associated with this room
-  def update_current_meeting_record(metadata=nil, force_not_ended=false)
-    unless self.create_time.nil?
-      attrs = {
-        :running => self.running,
-        :create_time => self.create_time
-      }
-      # note: it's important to update the 'ended' attr so the meeting is
-      # reopened in case it was mistakenly considered as ended
-      attrs[:ended] = false if force_not_ended
+  def update_current_meeting_record(response, force_not_ended=false)
+    attrs = {}
+    unless response.nil?
+      attrs[:running] = response[:running]
+      attrs[:create_time] = response[:createTime]
 
+      metadata = response[:metadata]
       unless metadata.nil?
         begin
           attrs[:creator_id] = metadata[BigbluebuttonRails.configuration.metadata_user_id].to_i
@@ -372,10 +367,13 @@ def update_current_meeting_record(metadata=nil, force_not_ended=false)
           attrs[:creator_name] = nil
         end
       end
-
-      meeting = self.get_current_meeting
-      meeting.update_attributes(attrs) if meeting.present?
     end
+    # note: it's important to update the 'ended' attr so the meeting is
+    # reopened in case it was mistakenly considered as ended
+    attrs[:ended] = false if force_not_ended
+
+    meeting = self.get_current_meeting
+    meeting.update_attributes(attrs) if meeting.present?
   end
 
   # Sets all meetings related to this room as not running
@@ -464,7 +462,6 @@ def init
     # fetched attributes
     @participant_count = 0
     @moderator_count = 0
-    @running = false
     @has_been_forcibly_ended = false
     @end_time = nil
     @current_attendees = []

app/models/bigbluebutton_server.rb

@@ -93,8 +93,7 @@ def fetch_meetings
         room.update_attributes(attendee_api_password: attr[:attendeePW],
                                moderator_api_password: attr[:moderatorPW])
       end
-      room.running = attr[:running]
-      room.update_current_meeting_record
+      room.update_current_meeting_record(attr)
 
       @meetings << room
     end

app/views/bigbluebutton/api/rooms/running.rabl

@@ -5,6 +5,6 @@ child(@room => :data) {
   node(:id) { |obj| obj.to_param }
 
   node :attributes do |room|
-    { :running => room.is_running? }
+    { :running => @running }
   end
 }

spec/controllers/bigbluebutton/servers_controller_spec.rb

@@ -282,6 +282,8 @@
         server.should_receive(:meetings).at_least(:once).and_return([room1, room2])
         room1.should_receive(:fetch_meeting_info)
         room2.should_receive(:fetch_meeting_info)
+        room1.should_receive(:fetch_is_running?)
+        room2.should_receive(:fetch_is_running?)
       end
 
       context do
@@ -318,6 +320,8 @@
           server.should_receive(:fetch_meetings).and_return({ })
           server.should_receive(:meetings).at_least(:once).and_return([room1, room2])
           room1.should_receive(:fetch_meeting_info) { raise bbb_error }
+          room1.should_receive(:fetch_is_running?)
+          room2.should_receive(:fetch_is_running?)
         end
         before(:each) { get :activity, :id => server.to_param }
         it { should set_the_flash.to(api_error_msg(bbb_error)) }

spec/models/bigbluebutton_meeting_spec.rb

@@ -114,16 +114,16 @@
       }
       before {
         room.create_time = Time.now.utc
-        room.running = !room.running # to change its default value
         room.record_meeting = !room.record_meeting # to change its default value
         room.create_time = Time.at(Time.now.to_i - 123)  # to change its default value
       }
 
       context "if there's no meeting associated yet creates one" do
         context "and there's no metadata in the response" do
+          let(:running) { false }
           before(:each) {
             expect {
-              BigbluebuttonMeeting.create_meeting_record_from_room(room, {internalMeetingID: 'fake-id'}, server, nil, {})
+              BigbluebuttonMeeting.create_meeting_record_from_room(room, {internalMeetingID: 'fake-id', running: running}, server, nil, {})
             }.to change{ BigbluebuttonMeeting.count }.by(1)
           }
           subject { BigbluebuttonMeeting.last }
@@ -133,7 +133,9 @@
           it("sets meetingid") { subject.meetingid.should eq(room.meetingid) }
           it("sets name") { subject.name.should eq(room.name) }
           it("sets recorded") { subject.recorded.should eq(room.record_meeting) }
-          it("sets running") { subject.running.should eq(room.running) }
+          it("sets running") {
+            room.should_receive(:fetch_is_running?).and_return(running)
+            subject.running.should eq(room.is_running?) }
           it("sets create_time") { subject.create_time.should eq(room.create_time.to_i) }
           it("doesn't set creator_id") { subject.creator_id.should be_nil }
           it("doesn't set creator_name") { subject.creator_name.should be_nil }