Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GIT-718: Implement offset pagination for getRecordings (closes #718) #720

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,8 @@ These variables are used by the service startup scripts in the Docker images, bu
* `PROTECTED_RECORDINGS_ENABLED`: Applies to the recording import process. If set to "true", then newly imported recordings will have protected links enabled. Default is "false".
* `PROTECTED_RECORDINGS_TOKEN_TIMEOUT`: Protected recording link token timeout in minutes. This is the amount of time that the one-time-use link returned in `getRecordings` calls will be valid for. Defaults to 60 minutes (1 hour).
* `PROTECTED_RECORDINGS_TIMEOUT`: Protected recordings resource access cookie timeout in minutes. This is the amount of time that a user will be granted access to view a recording for after clicking on the one-time-use link. Defaults to 360 minutes (6 hours).
* `DEFAULT_PAGINATION_LIMIT`: The number of records that will be displayed per page, this value will be considered if the pagination limit is not specified in GET_RECORDINGS_API params.
* `MAX_PAGINATION_LIMIT`: The max pagination limit value for get_recordings api. Defaults to 100.
* `SCALELITE_API_PORT`: Runs the SCALELITE_API in custom port number. Defaults to 3000.

### Redis Connection (`config/redis_store.yml`)
Expand Down
8 changes: 4 additions & 4 deletions app/controllers/bigbluebutton_api_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -268,10 +268,7 @@ def join
end

def get_recordings
if Rails.configuration.x.get_recordings_api_filtered && (params[:recordID].blank? && params[:meetingID].blank?)
raise BBBError.new('missingParameters', 'param meetingID or recordID must be included.')
end

validate_get_recordings_params
query = Recording.includes(playback_formats: [:thumbnails], metadata: []).references(:metadata)
query = if params[:state].present?
states = params[:state].split(',')
Expand All @@ -293,6 +290,9 @@ def get_recordings
query = query.with_recording_id_prefixes(params[:recordID].split(',')) if params[:recordID].present?
query = query.where(meeting_id: params[:meetingID].split(',')) if params[:meetingID].present?

offset = params[:offset] || 0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Validate that the provided offset param is ≥0

query = query.offset(offset).limit(params[:limit])

@recordings = query.order(starttime: :desc).all
@url_prefix = "#{request.protocol}#{request.host_with_port}"

Expand Down
20 changes: 20 additions & 0 deletions app/controllers/concerns/api_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,26 @@ def valid_token?(token)
decoded_token(token)
end

def validate_get_recordings_params
if params[:limit].present? && !params[:limit].to_i.between?(1, Rails.configuration.x.max_pagination_limit)
raise BBBError.new('invalidParamLimit',
"param limit should be within the range 1 and #{Rails.configuration.x.max_pagination_limit}")
end

if params[:offset].present? && params[:offset].to_i.negative?
raise BBBError.new('invalidParamOffset', 'param offset cannot be negative.')
end

if Rails.configuration.x.get_recordings_api_filtered && (params[:recordID].blank? &&
params[:meetingID].blank? && limit.nil?)
raise BBBError.new('missingParameters', 'param meetingID, recordID or limit must be included.')
end
end

def limit
params[:limit] || Rails.configuration.x.default_pagination_limit
end

def post_req(uri, body)
http = Net::HTTP.new(uri.host, uri.port)
http.use_ssl = (uri.scheme == 'https')
Expand Down
6 changes: 6 additions & 0 deletions config/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -127,5 +127,11 @@ class Application < Rails::Application
config.x.recording_token_ttl = ENV.fetch('PROTECTED_RECORDINGS_TOKEN_TIMEOUT', '60').to_i.minutes
# Protected recordings resource access cookie timeout in minutes. Defaults to 360 (6 hours)
config.x.recording_cookie_ttl = ENV.fetch('PROTECTED_RECORDINGS_TIMEOUT', '360').to_i.minutes

# Set default pagination limit for get_recordings api.
config.x.default_pagination_limit = ENV['DEFAULT_PAGINATION_LIMIT']

# Set max pagination limit for get_recordings api.
config.x.max_pagination_limit = ENV.fetch('MAX_PAGINATION_LIMIT', '100').to_i
end
end
74 changes: 73 additions & 1 deletion test/controllers/bigbluebutton_api_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1122,7 +1122,7 @@ class BigBlueButtonApiControllerTest < ActionDispatch::IntegrationTest
assert_response :success
assert_select 'response>returncode', 'FAILED'
assert_select 'response>messageKey', 'missingParameters'
assert_select 'response>message', 'param meetingID or recordID must be included.'
assert_select 'response>message', 'param meetingID, recordID or limit must be included.'
end

test 'getRecordings with only checksum returns all recordings for a post request' do
Expand Down Expand Up @@ -1336,6 +1336,78 @@ class BigBlueButtonApiControllerTest < ActionDispatch::IntegrationTest
assert_select 'response>recordings>recording', 1
end

test 'getRecordings returns paginated recordings based on the limit given in params' do
create_list(:recording, 12, state: 'published')

params = encode_bbb_params('getRecordings', { limit: 4 }.to_query)

get bigbluebutton_api_get_recordings_url, params: params

assert_response :success
assert_select 'response>returncode', 'SUCCESS'
assert_select 'response>recordings>recording', 4
end

test 'getRecordings returns paginated recordings if offset & limit value is given' do
list = create_list(:recording, 20, state: 'unpublished')
offset_zero = list.last(4).first
offset_five = list.last(8).first

params = encode_bbb_params('getRecordings', { offset: 3, limit: 5 }.to_query)

get bigbluebutton_api_get_recordings_url, params: params

assert_response :success
assert_select 'response>returncode', 'SUCCESS'
assert_select 'response>recordings>recording', 5
response_xml = Nokogiri::XML(@response.body)
first_rec_id = response_xml.xpath('/response/recordings/recording').first.xpath('.//recordID').text
last_rec_id = response_xml.xpath('/response/recordings/recording').last.xpath('.//recordID').text
assert_equal first_rec_id, offset_zero.record_id
assert_equal last_rec_id, offset_five.record_id
end

test 'getRecordings returns all recordings if values of limit=nil & offset=0' do
list = create_list(:recording, 20, state: 'published')
offset_zero = list.last
offset_ten = list.first

BigBlueButtonApiController.stub_any_instance(:verify_checksum, nil) do
get bigbluebutton_api_get_recordings_url
end

assert_response :success
assert_select 'response>returncode', 'SUCCESS'
assert_select 'response>recordings>recording', 20
response_xml = Nokogiri::XML(@response.body)
first_rec_id = response_xml.xpath('/response/recordings/recording').first.xpath('.//recordID').text
last_rec_id = response_xml.xpath('/response/recordings/recording').last.xpath('.//recordID').text
assert_equal first_rec_id, offset_zero.record_id
assert_equal last_rec_id, offset_ten.record_id
end

test 'getRecordings raises error if value of limit is invalid' do
params = encode_bbb_params('getRecordings', { limit: 500 }.to_query)
BigBlueButtonApiController.stub_any_instance(:verify_checksum, nil) do
get bigbluebutton_api_get_recordings_url, params: params
end

assert_response :success
assert_select 'response>returncode', 'FAILED'
assert_select 'response>messageKey', 'invalidParamLimit'
end

test 'getRecordings raises error if value of offset is invalid' do
params = encode_bbb_params('getRecordings', { offset: -5 }.to_query)
BigBlueButtonApiController.stub_any_instance(:verify_checksum, nil) do
get bigbluebutton_api_get_recordings_url, params: params
end

assert_response :success
assert_select 'response>returncode', 'FAILED'
assert_select 'response>messageKey', 'invalidParamOffset'
end

test 'getRecordings with get_recordings_api_filtered filters based on recording states' do
create_list(:recording, 5, state: 'deleted')
r1 = create(:recording, state: 'published')
Expand Down