diff --git a/app/controllers/bigbluebutton/api/rooms_controller.rb b/app/controllers/bigbluebutton/api/rooms_controller.rb index 0e7f4d14..8cbbee51 100644 --- a/app/controllers/bigbluebutton/api/rooms_controller.rb +++ b/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 diff --git a/app/controllers/bigbluebutton/rooms_controller.rb b/app/controllers/bigbluebutton/rooms_controller.rb index 70639b46..5a8d0f51 100644 --- a/app/controllers/bigbluebutton/rooms_controller.rb +++ b/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}" diff --git a/app/models/bigbluebutton_meeting.rb b/app/models/bigbluebutton_meeting.rb index d26f08c9..0b877525 100644 --- a/app/models/bigbluebutton_meeting.rb +++ b/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] } diff --git a/app/models/bigbluebutton_room.rb b/app/models/bigbluebutton_room.rb index 24619700..0040c1e4 100644 --- a/app/models/bigbluebutton_room.rb +++ b/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 running def is_running? - @running + # TODO: cache this maybe? + fetch_is_running? end # Fetches info from BBB about this room. @@ -121,7 +122,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 +135,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 +156,7 @@ def fetch_meeting_info # Triggers API call: isMeetingRunning. 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 +288,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 +311,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 +352,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 +368,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 @@ -462,7 +461,6 @@ def init # fetched attributes @participant_count = 0 @moderator_count = 0 - @running = false @has_been_forcibly_ended = false @end_time = nil @current_attendees = [] diff --git a/app/models/bigbluebutton_server.rb b/app/models/bigbluebutton_server.rb index a4d29df7..580d4255 100644 --- a/app/models/bigbluebutton_server.rb +++ b/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 diff --git a/app/views/bigbluebutton/api/rooms/running.rabl b/app/views/bigbluebutton/api/rooms/running.rabl index 9694251f..872fdc04 100644 --- a/app/views/bigbluebutton/api/rooms/running.rabl +++ b/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 } diff --git a/spec/controllers/bigbluebutton/servers_controller_spec.rb b/spec/controllers/bigbluebutton/servers_controller_spec.rb index 5e64eb8e..ec4edb0e 100644 --- a/spec/controllers/bigbluebutton/servers_controller_spec.rb +++ b/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)) } diff --git a/spec/models/bigbluebutton_meeting_spec.rb b/spec/models/bigbluebutton_meeting_spec.rb index 5e3d438a..62d983b8 100644 --- a/spec/models/bigbluebutton_meeting_spec.rb +++ b/spec/models/bigbluebutton_meeting_spec.rb @@ -82,16 +82,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 } @@ -101,7 +101,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 } diff --git a/spec/models/bigbluebutton_room_spec.rb b/spec/models/bigbluebutton_room_spec.rb index dc97f703..c2bfdc2a 100644 --- a/spec/models/bigbluebutton_room_spec.rb +++ b/spec/models/bigbluebutton_room_spec.rb @@ -46,7 +46,7 @@ it { should accept_nested_attributes_for(:metadata).allow_destroy(true) } # attr_accessors - [:running, :participant_count, :moderator_count, :current_attendees, + [:participant_count, :moderator_count, :current_attendees, :has_been_forcibly_ended, :create_time, :end_time, :external, :request_headers, :record_meeting, :duration].each do |attr| it { should respond_to(attr) } @@ -185,9 +185,9 @@ it { should respond_to(:instance_variables_compare) } it { room.instance_variables_compare(room2).should be_empty } it "compares instance variables" do - room2.running = !room.running + room2.end_time = !room.end_time room.instance_variables_compare(room2).should_not be_empty - room.instance_variables_compare(room2).should include(:@running) + room.instance_variables_compare(room2).should include(:@end_time) end it "ignores attributes" do room2.private = !room.private @@ -202,7 +202,7 @@ it { should respond_to(:attr_equal?) } it { room.attr_equal?(room2).should be_truthy } it "differentiates instance variables" do - room2.running = !room.running + room2.end_time = !room.end_time room.attr_equal?(room2).should be(false) end it "differentiates attributes" do @@ -221,7 +221,6 @@ it "fetched attributes before they are fetched" do room.participant_count.should be(0) room.moderator_count.should be(0) - room.running.should be(false) room.has_been_forcibly_ended.should be(false) room.create_time.should be_nil room.end_time.should be_nil @@ -303,8 +302,6 @@ room.should_receive(:select_server).and_return(mocked_server) } before(:each) { @response = room.fetch_is_running? } - it { room.running.should be(false) } - it { room.is_running?.should be(false) } it { @response.should be(false) } end @@ -314,8 +311,6 @@ room.should_receive(:select_server).and_return(mocked_server) } before(:each) { @response = room.fetch_is_running? } - it { room.running.should be_truthy } - it { room.is_running?.should be_truthy } it { @response.should be_truthy } end @@ -362,7 +357,6 @@ room.should_receive(:select_server).and_return(mocked_server) } before(:each) { room.fetch_meeting_info } - it { room.running.should == false } it { room.has_been_forcibly_ended.should == false } it { room.participant_count.should == 0 } it { room.moderator_count.should == 0 } @@ -379,7 +373,6 @@ room.should_receive(:select_server).and_return(mocked_server) } before(:each) { room.fetch_meeting_info } - it { room.running.should == true } it { room.has_been_forcibly_ended.should == false } it { room.participant_count.should == 4 } it { room.moderator_count.should == 2 } @@ -403,7 +396,7 @@ room.should_receive(:select_server).and_return(mocked_server) # here's the validation - room.should_receive(:update_current_meeting_record).with(metadata, true) + room.should_receive(:update_current_meeting_record).with(hash_info2, true) } it { room.fetch_meeting_info } end @@ -1186,7 +1179,6 @@ describe "#create_meeting" do let(:room) { FactoryGirl.create(:bigbluebutton_room) } let(:user) { FactoryGirl.build(:user) } - before { room.should_receive(:fetch_is_running?) } context "when the conference is running" do before { @@ -1309,29 +1301,39 @@ context "if #create_time is set" do let(:user) { FactoryGirl.build(:user) } - let(:metadata) { - m = {} - m[BigbluebuttonRails.configuration.metadata_user_id] = user.id - m[BigbluebuttonRails.configuration.metadata_user_name] = user.name - m + let(:create_time) { Time.now.to_i - 123 } + let(:running) { false } + let(:response) { + r = {} + r[:createTime] = create_time + r[:running] = running + r[:metadata] = {} + r[:metadata][BigbluebuttonRails.configuration.metadata_user_id] = user.id + r[:metadata][BigbluebuttonRails.configuration.metadata_user_name] = user.name + r } before { room.create_time = Time.now.utc - room.running = !room.running # to change its default value - room.create_time = Time.at(Time.now.to_i - 123) # to change its default value + room.create_time = create_time } context "and no metadata was passed" do + let(:response) { + r = {} + r[:createTime] = create_time + r[:running] = running + r + } before { FactoryGirl.create(:bigbluebutton_meeting, :room => room, :create_time => room.create_time) } before(:each) { expect { - room.update_current_meeting_record + room.update_current_meeting_record(response) }.not_to change{ BigbluebuttonMeeting.count } } subject { BigbluebuttonMeeting.find_by_room_id(room.id) } - it("sets running") { subject.running.should eq(room.running) } + it("sets running") { subject.running.should eq(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 } @@ -1343,11 +1345,11 @@ } before(:each) { expect { - room.update_current_meeting_record(metadata) + room.update_current_meeting_record(response) }.not_to change{ BigbluebuttonMeeting.count } } subject { BigbluebuttonMeeting.find_by_room_id(room.id) } - it("sets running") { subject.running.should eq(room.running) } + it("sets running") { subject.running.should eq(running) } it("sets create_time") { subject.create_time.should eq(room.create_time.to_i) } it("sets creator_id") { subject.creator_id.should eq(user.id) } it("sets creator_name") { subject.creator_name.should eq(user.name) } @@ -1363,7 +1365,7 @@ }.not_to change{ BigbluebuttonMeeting.count } } subject { BigbluebuttonMeeting.find_by_room_id(room.id) } - it("sets running") { subject.running.should eq(room.running) } + it("sets running") { subject.running.should eq(running) } it("sets create_time") { subject.create_time.should eq(room.create_time.to_i) } it("sets ended") { subject.ended.should be(false) } end diff --git a/spec/models/bigbluebutton_server_spec.rb b/spec/models/bigbluebutton_server_spec.rb index 2ecfd689..8744009a 100644 --- a/spec/models/bigbluebutton_server_spec.rb +++ b/spec/models/bigbluebutton_server_spec.rb @@ -173,9 +173,9 @@ # the hashes should be exactly as returned by bigbluebutton-api-ruby to be sure we are testing it right let(:meetings) { [ - { :meetingID => room1.meetingid, :attendeePW => "ap", :moderatorPW => "mp", :hasBeenForciblyEnded => false, :running => true}, - { :meetingID => room2.meetingid, :attendeePW => "pass", :moderatorPW => "pass", :hasBeenForciblyEnded => true, :running => false}, - { :meetingID => "im not in the db", :attendeePW => "pass", :moderatorPW => "pass", :hasBeenForciblyEnded => true, :running => true} + { :meetingID => room1.meetingid, createTime: Time.now.to_i, :attendeePW => "ap", :moderatorPW => "mp", :hasBeenForciblyEnded => false, :running => true}, + { :meetingID => room2.meetingid, createTime: Time.now.to_i + 123, :attendeePW => "pass", :moderatorPW => "pass", :hasBeenForciblyEnded => true, :running => false}, + { :meetingID => "im not in the db", createTime: Time.now.to_i + 234, :attendeePW => "pass", :moderatorPW => "pass", :hasBeenForciblyEnded => true, :running => true} ] } let(:hash) { @@ -204,7 +204,6 @@ it { server.meetings[2].name.should == "im not in the db" } it { server.meetings[2].attendee_api_password.should == "pass" } it { server.meetings[2].moderator_api_password.should == "pass" } - it { server.meetings[2].running.should == true } it { server.meetings[2].new_record?.should be_truthy } it { server.meetings[2].external.should be_truthy } it { server.meetings[2].private.should be_truthy }