Skip to content

Commit 260b3e4

Browse files
committed
Reduce linter-flagged issues
1 parent ce64d05 commit 260b3e4

File tree

4 files changed

+206
-199
lines changed

4 files changed

+206
-199
lines changed
Lines changed: 168 additions & 155 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
# frozen_string_literal: true
2+
13
# This file is part of SSID.
24
#
35
# SSID is free software: you can redistribute it and/or modify
@@ -16,183 +18,194 @@
1618
require 'zip'
1719
require 'api_keys_handler'
1820

19-
class Api::V1::AssignmentsController < ApplicationController
20-
skip_before_action :authenticate_user!
21-
22-
before_action do |_controller|
23-
@course = Course.find(params['course_id']) if params['course_id']
24-
end
25-
26-
before_action :init_api_key_handler
27-
28-
# Define valid zip mime types as constant variables
29-
X_ZIP_COMPRESSED_MIME_TYPE = 'application/x-zip-compressed'
30-
ZIP_COMPRESSED_MIME_TYPE = 'application/zip-compressed'
31-
APPLICATION_ZIP_MIME_TYPE = 'application/zip'
32-
MULTIPART_X_ZIP_MIME_TYPE = 'multipart/x-zip'
33-
OCTET_STREAM_MIME_TYPE = 'application/octet-stream'
34-
REQUIRED_PARAMS = %w[title language]
35-
ALLOWED_PARAMS = %w[title language useFingerprints minimumMatchLength sizeOfNGram studentSubmissions
36-
mappingFile]
37-
ALLOWED_LANGUAGES = %w[java python3 c cpp javascript r ocaml matlab scala]
38-
39-
def init_api_key_handler
40-
APIKeysHandler.api_key = ApiKey.find_by(value: request.headers['X-API-KEY'])
41-
APIKeysHandler.course = @course
42-
end
21+
module Api
22+
module V1
23+
class AssignmentsController < ApplicationController
24+
skip_before_action :authenticate_user!
4325

44-
# GET api/v1/courses/1/assignments/new
45-
def new
46-
@assignment = Assignment.new
47-
end
26+
before_action do |_controller|
27+
@course = Course.find(params['course_id']) if params['course_id']
28+
end
4829

49-
# POST api/v1/courses/1/assignments
50-
def create
51-
REQUIRED_PARAMS.each do |p|
52-
if params[p].nil?
53-
render json: { error: "Missing required parameter '#{p}'" }, status: :bad_request
54-
return
30+
before_action :init_api_key_handler
31+
32+
# Define valid zip mime types as constant variables
33+
X_ZIP_COMPRESSED_MIME_TYPE = 'application/x-zip-compressed'
34+
ZIP_COMPRESSED_MIME_TYPE = 'application/zip-compressed'
35+
APPLICATION_ZIP_MIME_TYPE = 'application/zip'
36+
MULTIPART_X_ZIP_MIME_TYPE = 'multipart/x-zip'
37+
OCTET_STREAM_MIME_TYPE = 'application/octet-stream'
38+
REQUIRED_PARAMS = %w[title language].freeze
39+
ALLOWED_PARAMS = %w[title language useFingerprints minimumMatchLength sizeOfNGram studentSubmissions
40+
mappingFile].freeze
41+
ALLOWED_LANGUAGES = %w[java python3 c cpp javascript r ocaml matlab scala].freeze
42+
43+
def init_api_key_handler
44+
APIKeysHandler.api_key = ApiKey.find_by(value: request.headers['X-API-KEY'])
45+
APIKeysHandler.course = @course
5546
end
56-
end
5747

58-
request.request_parameters.each do |k, _v|
59-
if ALLOWED_PARAMS.include?(k) == false
60-
render json: { error: "Parameter #{k} is invalid or not yet supported." }, status: :bad_request
61-
return
48+
# GET api/v1/courses/1/assignments/new
49+
def new
50+
@assignment = Assignment.new
6251
end
63-
end
6452

65-
@assignment = Assignment.new do |a|
66-
a.title = params['title']
67-
a.language = params['language']
68-
a.min_match_length = params['minimumMatchLength'].presence || 2 # defaults to 2 if not specified
69-
a.ngram_size = params['sizeOfNGram'].presence || 5 # defaults to 5 if not specified
70-
a.course_id = @course.id
71-
end
53+
# POST api/v1/courses/1/assignments
54+
def create
55+
REQUIRED_PARAMS.each do |p|
56+
if params[p].nil?
57+
render json: { error: "Missing required parameter '#{p}'" }, status: :bad_request
58+
return
59+
end
60+
end
7261

73-
begin
74-
APIKeysHandler.authenticate_api_key
75-
rescue APIKeysHandler::APIKeyError => e
76-
render json: { error: e.message }, status: e.status
77-
return
78-
end
62+
request.request_parameters.each do |k, _v|
63+
if ALLOWED_PARAMS.include?(k) == false
64+
render json: { error: "Parameter #{k} is invalid or not yet supported." }, status: :bad_request
65+
return
66+
end
67+
end
7968

80-
REQUIRED_PARAMS.each do |p|
81-
if params[p].nil?
82-
render json: { error: "Missing required parameter '#{p}'" }, status: :bad_request
83-
return
84-
end
85-
end
69+
@assignment = Assignment.new do |a|
70+
a.title = params['title']
71+
a.language = params['language']
72+
a.min_match_length = params['minimumMatchLength'].presence || 2 # defaults to 2 if not specified
73+
a.ngram_size = params['sizeOfNGram'].presence || 5 # defaults to 5 if not specified
74+
a.course_id = @course.id
75+
end
8676

87-
unless ALLOWED_LANGUAGES.include?(params['language'])
88-
render json: { error: "Value of language is not valid. We currently support #{ALLOWED_LANGUAGES}. The parameter value must be in lowercase and match exactly one of the options." },
89-
status: :bad_request
90-
return
91-
end
77+
begin
78+
APIKeysHandler.authenticate_api_key
79+
rescue APIKeysHandler::APIKeyError => e
80+
render json: { error: e.message }, status: e.status
81+
return
82+
end
9283

93-
if params['useFingerprints'] && !%w[Yes No].include?(params['useFingerprints'])
94-
render json: { error: 'Value of useFingerprints is not valid. The value should be "Yes" or "No". The parameter value must be in lowercase and match exactly one of the options.' },
95-
status: :bad_request
96-
return
97-
end
84+
REQUIRED_PARAMS.each do |p|
85+
if params[p].nil?
86+
render json: { error: "Missing required parameter '#{p}'" }, status: :bad_request
87+
return
88+
end
89+
end
9890

99-
puts 'DEBUG 06: Enable fingerprints checkbox?'
100-
puts "Checkbox: #{params['useFingerprints']}"
101-
# Process file if @assignment is valid and file was uploaded
102-
if @assignment.valid?
103-
104-
# Save assignment to obtain id
105-
return render action: 'new' unless @assignment.save
106-
107-
is_map_enabled = params['mappingFile'].nil? ? false : true
108-
used_fingerprints = params['useFingerprints'] == 'Yes'
109-
110-
# No student submission file was uploaded
111-
# Student submission file is a valid zip
112-
if is_valid_zip?(params['studentSubmissions'].content_type, params['studentSubmissions'].path)
113-
# Don't process the file and show error if the mapping was enabled but no mapping file was uploaded
114-
if is_valid_map_or_no_map?(is_map_enabled, params['mappingFile'])
115-
start_upload(@assignment, params['studentSubmissions'], is_map_enabled, params['mappingFile'],
116-
used_fingerprints)
117-
# Don't process the file and show error if the mapping was enabled but no mapping file was uploaded
118-
else
119-
@assignment.errors.add :mapfile, 'containing mapped student names must be a valid csv file'
120-
render json: { error: "Value of mappingFile is not valid. The mapping file must be a valid csv file. #{is_map_enabled}, #{params['mappingFile'].content_type}" },
91+
unless ALLOWED_LANGUAGES.include?(params['language'])
92+
render json: { error: "Value of language is not valid.' +
93+
'We currently support #{ALLOWED_LANGUAGES}.' +
94+
'The parameter value must be in lowercase and match exactly one of the options." },
12195
status: :bad_request
96+
return
97+
end
98+
99+
if params['useFingerprints'] && %w[Yes No].exclude?(params['useFingerprints'])
100+
render json: { error: 'Value of useFingerprints is not valid. ' \
101+
'The value should be "Yes" or "No". ' \
102+
'The parameter value must be in lowercase and match exactly one of the options.' },
103+
status: :bad_request
104+
return
105+
end
106+
107+
Rails.logger.debug 'DEBUG 06: Enable fingerprints checkbox?'
108+
Rails.logger.debug { "Checkbox: #{params['useFingerprints']}" }
109+
# Process file if @assignment is valid and file was uploaded
110+
if @assignment.valid?
111+
112+
# Save assignment to obtain id
113+
return render action: 'new' unless @assignment.save
114+
115+
is_map_enabled = !params['mappingFile'].nil?
116+
used_fingerprints = params['useFingerprints'] == 'Yes'
117+
118+
# No student submission file was uploaded
119+
# Student submission file is a valid zip
120+
if valid_zip?(params['studentSubmissions'].content_type, params['studentSubmissions'].path)
121+
# Don't process the file and show error if the mapping was enabled but no mapping file was uploaded
122+
if valid_map_or_no_map?(is_map_enabled, params['mappingFile'])
123+
start_upload(@assignment, params['studentSubmissions'], is_map_enabled, params['mappingFile'],
124+
used_fingerprints)
125+
# Don't process the file and show error if the mapping was enabled but no mapping file was uploaded
126+
else
127+
@assignment.errors.add :mapfile, 'containing mapped student names must be a valid csv file'
128+
render json: { error: "Value of mappingFile is not valid. '
129+
+ 'The mapping file must be a valid csv file." },
130+
status: :bad_request
131+
end
132+
# Student submission file is not a valid zip file
133+
else
134+
@assignment.errors.add :file, 'containing student submission files must be a valid zip file'
135+
render json: { error: 'Value of studentSubmissions is not valid. ' \
136+
'studentSubmissions must be a valid zip file.' },
137+
status: :bad_request
138+
render action: 'new'
139+
end
140+
else
141+
render action: 'new'
122142
end
123-
# Student submission file is not a valid zip file
124-
else
125-
@assignment.errors.add :file, 'containing student submission files must be a valid zip file'
126-
render json: { error: 'Value of studentSubmissions is not valid. studentSubmissions must be a valid zip file.' },
127-
status: :bad_request
128-
render action: 'new'
129143
end
130-
else
131-
render action: 'new'
132-
end
133-
end
134144

135-
def start_upload(assignment, submissionFile, is_map_enabled, mapFile, used_fingerprints)
136-
require 'submissions_handler'
145+
def start_upload(assignment, submission_file, is_map_enabled, map_file, used_fingerprints)
146+
require 'submissions_handler'
137147

138-
# Process upload file
139-
submissions_path = SubmissionsHandler.process_upload(submissionFile, is_map_enabled, mapFile, assignment)
140-
if submissions_path
141-
# Launch java program to process submissions
142-
SubmissionsHandler.process_submissions(submissions_path, assignment, is_map_enabled, used_fingerprints)
148+
# Process upload file
149+
submissions_path = SubmissionsHandler.process_upload(submission_file, is_map_enabled, map_file, assignment)
150+
if submissions_path
151+
# Launch java program to process submissions
152+
SubmissionsHandler.process_submissions(submissions_path, assignment, is_map_enabled, used_fingerprints)
143153

144-
process = assignment.submission_similarity_process
145-
notice = 'SSID will start to process the assignment now. Please refresh this page after a few minutes to view the similarity results.'
146-
if process && process.status == SubmissionSimilarityProcess::STATUS_WAITING
147-
notice = 'Your assignment has been put into a waiting list. SSID will process it soon. Thank you for your patience.'
154+
render json: { assignmentID: @assignment.id }, status: :ok
155+
else
156+
assignment.errors.add 'Submission zip file',
157+
': SSID supports both directory-based and file-based submissions. ' \
158+
'Please select the submissions you want to evaluate and compress.'
159+
render action: 'show'
160+
end
148161
end
149-
render json: { assignmentID: @assignment.id }, status: :ok
150-
else
151-
assignment.errors.add 'Submission zip file',
152-
': SSID supports both directory-based and file-based submissions. Please select the submissions you want to evaluate and compress.'
153-
render action: 'show'
154-
end
155-
end
156162

157-
# Responsible for verifying whether a uploaded file is zip by checking its mime type and/or whether can it be extracted by the zip library.
158-
# For files with mime type = application/octet-stream, it needs to be further verified by the zip library as it can be a rar file.
159-
# Params:
160-
# +mimeType+:: string that contains the file's mimetype
161-
# +filePath+:: string that contains the file's path which is to be used by the zip library when extracting the file
162-
def is_valid_zip?(mimeType, filePath)
163-
# Valid zip file mime types that does not required to be further verified by the zip library
164-
if [X_ZIP_COMPRESSED_MIME_TYPE, ZIP_COMPRESSED_MIME_TYPE, APPLICATION_ZIP_MIME_TYPE,
165-
MULTIPART_X_ZIP_MIME_TYPE].include?(mimeType)
166-
true
167-
# Need to be further verified by zip library as it can be a rar file
168-
elsif mimeType == OCTET_STREAM_MIME_TYPE && is_opened_as_zip?(filePath)
169-
return true
170-
# For other mime types, safe to consider that it is not a zip file
171-
false
172-
end
173-
end
163+
# Responsible for verifying whether a uploaded file is zip by checking its mime
164+
# type and/or whether can it be extracted by the zip library.
165+
# For files with mime type = application/octet-stream, it needs to be further verified
166+
# by the zip library as it can be a rar file.
167+
# Params:
168+
# +mime_type+:: string that contains the file's mimetype
169+
# +filePath+:: string that contains the file's path which is to be used
170+
# by the zip library when extracting the file
171+
def valid_zip?(mime_type, file_path)
172+
# Valid zip file mime types that does not required to be further verified by the zip library
173+
if [X_ZIP_COMPRESSED_MIME_TYPE, ZIP_COMPRESSED_MIME_TYPE, APPLICATION_ZIP_MIME_TYPE,
174+
MULTIPART_X_ZIP_MIME_TYPE].include?(mime_type)
175+
true
176+
# Need to be further verified by zip library as it can be a rar file
177+
elsif mime_type == OCTET_STREAM_MIME_TYPE && opened_as_zip?(file_path)
178+
return true
179+
# For other mime types, safe to consider that it is not a zip file
180+
end
181+
false
182+
end
174183

175-
# Responsible for verifying whether a uploaded file is zip by checking whether can it be extracted by the zip library
176-
# Params:
177-
# +filePath+:: string that contains the file's path which is to be used by the zip library when extracting the file
178-
def is_opened_as_zip?(path)
179-
# File is zip if the zip library is able to extract the file
180-
zip = Zip::File.open(path)
181-
true
182-
rescue StandardError => e
183-
puts e
184-
false
185-
ensure
186-
zip.close if zip
187-
end
184+
# Responsible for verifying whether a uploaded file is zip by checking whether can it be extracted by the zip
185+
# library
186+
# Params:
187+
# +filePath+:: string that contains the file's path which is to be used by the zip library
188+
# when extracting the file
189+
def opened_as_zip?(path)
190+
# File is zip if the zip library is able to extract the file
191+
zip = Zip::File.open(path)
192+
true
193+
rescue StandardError => e
194+
Rails.logger.debug e
195+
false
196+
ensure
197+
zip&.close
198+
end
188199

189-
def is_valid_map_or_no_map?(is_map_enabled, mapFile)
190-
return true unless is_map_enabled
200+
def valid_map_or_no_map?(is_map_enabled, map_file)
201+
return true unless is_map_enabled
191202

192-
if mapFile.nil?
193-
false
194-
else
195-
mapFile.path.split('.').last.to_s.downcase == 'csv'
203+
if map_file.nil?
204+
false
205+
else
206+
map_file.path.split('.').last.to_s.downcase == 'csv'
207+
end
208+
end
196209
end
197210
end
198211
end

app/controllers/courses_controller.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,11 +84,11 @@ def create
8484
m.course = @course
8585
m.role = UserCourseMembership::ROLE_TEACHING_STAFF
8686
}
87-
88-
if @membership.valid? and @membership.save
87+
88+
if @membership.valid? && @membership.save
8989
redirect_to courses_url, notice: 'Course was successfully created.'
9090
else
91-
render action: "new"
91+
render action: 'new'
9292
end
9393
end
9494

config/routes.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -120,9 +120,9 @@
120120

121121
namespace :api do
122122
namespace :v1 do
123-
resources :courses, controller: "courses" do
124-
resources :assignments, controller: "assignments"
123+
resources :courses, controller: 'courses' do
124+
resources :assignments, controller: 'assignments'
125125
end
126126
end
127127
end
128-
end
128+
end

0 commit comments

Comments
 (0)