Allow benchmarks in YAML format (#268)

Author: OsamaSayegh

app/controllers/user_scripts_controller.rb

@@ -19,8 +19,11 @@ def create
     bench.validate!
     if bench.valid?
       bench.run
+      links = bench.names.map do |name|
+        "<a href=\"/ruby/ruby/commits?result_type=#{name}\">#{name}</a>"
+      end.join(', ')
 
-      render plain: t('.index.successful_submit', path: "/ruby/ruby/commits?result_type=#{bench.name}")
+      render plain: t('.index.successful_submit', links: links)
     else
       render plain: bench.errors.join("\n"), status: 422
     end

app/jobs/remote_server_job.rb

@@ -27,7 +27,7 @@ def perform(initiator_key, benchmark_group, options = {})
     Net::SSH.start(
       secrets.bare_metal_server_ip,
       secrets.bare_metal_server_user,
-      password: secrets.bare_metal_server_password
+      password: secrets.bare_metal_server_password.to_s
     ) do |ssh|
 
       send(benchmark_group, ssh, initiator_key, options)

app/services/user_bench.rb

@@ -1,26 +1,37 @@
+require 'net/http'
+
 class UserBench
-  attr_reader :errors, :name, :url, :sha, :sha2, :commits
+  attr_reader :errors, :url, :sha, :sha2, :commits, :names
 
   def initialize(name, url, sha, sha2 = nil)
     @name = name&.strip
     @url = url&.strip
     @sha = sha&.strip
     @sha2 = sha2&.strip
+    @names = []
     @errors = []
     @commits = []
     @client = Octokit::Client.new(access_token: Rails.application.secrets.github_api_token)
   end
 
   def validate!
-    errors.push(err('missing_name')) if name.blank?
+    errors.push(err('missing_name')) if @name.blank?
     errors.push(err('missing_url')) if url.blank?
     errors.push(err('missing_sha')) if sha.blank?
     return if !valid?
 
-    errors.push(err('name_already_taken')) if name_taken?
     errors.push(err('bad_url')) unless valid_url?
-    errors.push(err('unallowed_characters', name: name)) unless valid_name?
+    errors.push(err('unallowed_characters', name: @name)) unless valid_name?
+    return if !valid?
+
+    if !yaml_script? && !ruby_script?
+      errors.push(err('unkown_extension', url: url))
+      return
+    end
 
+    validate_yaml if yaml_script?
+    validate_names
+    validate_script if ruby_script?
     return if !valid?
 
     validate_sha(sha)
@@ -40,31 +51,80 @@ def run
         url: 'https://github.com/tgxworld/',
       )
     )
-    BenchmarkType.create!(
-      category: name,
-      script_url: url,
-      from_user: true,
-      repo: repo
-    )
+    @names.each do |ty|
+      BenchmarkType.create!(
+        category: ty,
+        script_url: url,
+        from_user: true,
+        repo: repo
+      )
+    end
 
-    RunUserBench.perform_later(name, url, commits.last.commit.committer.date.iso8601, commits.first.sha)
+    RunUserBench.perform_later(@name, url, commits.last.commit.committer.date.iso8601, commits.first.sha)
   end
 
   private
 
+  def validate_names
+    if yaml_script? && Hash === @parsed_yaml
+      benchmark = @parsed_yaml['benchmark']
+      if Array === benchmark
+        benchmark.each { |hash| @names << hash['name'] }
+      elsif Hash === benchmark
+        benchmark.keys.each { |n| @names << n }
+      elsif String === benchmark
+        @names << benchmark
+      end
+    end
+
+    if yaml_script? && @names.size == 0
+      errors.push(err('yaml_file_without_benchmarks'))
+      return
+    elsif ruby_script?
+      @names << @name
+    end
+
+    taken_names = BenchmarkType.where(category: @names).pluck(:category)
+    if taken_names.size > 0
+      errors.push(err('name_already_taken', names: taken_names.join(', ')))
+    end
+  end
+
+  def yaml_script?
+    url.match?(/\.ya?ml$/)
+  end
+
+  def ruby_script?
+    url.end_with?('.rb')
+  end
+
+  def validate_script
+    content = Net::HTTP.get(URI.parse(url)).strip
+    content.force_encoding(Encoding::UTF_8)
+    path = File.join(Dir.tmpdir, SecureRandom.hex)
+    File.open(path, 'wt') { |file| file.write(content) }
+    if !system("ruby -c #{path} > /dev/null 2>&1")
+      errors.push(err('invalid_ruby_code', lines: CGI.escapeHTML(content.split("\n").first(5).join("\n"))))
+    end
+    File.delete(path)
+  end
+
+  def validate_yaml
+    yaml = Net::HTTP.get(URI.parse(url))
+    @parsed_yaml = YAML.safe_load(yaml)
+  rescue Psych::SyntaxError, Psych::DisallowedClass
+    errors.push(err('invalid_yaml'))
+  end
+
   def valid_url?
     uri = URI.parse(url)
     URI::HTTP === uri || URI::HTTPS === uri
   rescue URI::InvalidURIError
     false
   end
 
-  def name_taken?
-    BenchmarkType.exists?(category: name)
-  end
-
   def valid_name?
-    name.match?(/^[a-zA-Z0-9\-_]+$/)
+    @name.match?(/^[a-zA-Z0-9\-_]+$/)
   end
 
   def validate_sha(sha)

config/locales/en.yml

@@ -23,15 +23,22 @@ en:
       commit_sha: Commit SHA
       second_commit_sha: Second commit SHA
       submit: Submit
-      successful_submit: "Success! Results will be published <a href=\"%{path}\">here</a>."
+      successful_submit: "Success! Results will be published at %{links}."
     errors:
       missing_name: Missing Benchmark Name
       missing_url: Missing Script URL
       missing_sha: Missing Commit SHA
-      name_already_taken: Benchmarks Name is already taken
+      name_already_taken: "`%{names}` already exist"
       bad_url: Invalid Script URL
+      unkown_extension: "Unknown file extension in the URL %{url}"
       unallowed_characters: "\"%{name}\" contains unallowed characters. Allowed characters are: a-z, A-Z, 0-9, hyphens and underscores"
       bad_sha: "Couldn't find Ruby commit for SHA %{sha}"
+      invalid_yaml: Invalid YAML syntax
+      invalid_ruby_code: |
+        The ruby code you provided has syntax errors. The first 5 lines are:
+        %{lines}
+
+      yaml_file_without_benchmarks: The provided YAML file doesn't contain any benchmarks
   organizations:
     index:
       title: Benchmarks

test/controllers/user_scripts_controller_test.rb

@@ -33,13 +33,18 @@ class UserScriptsControllerTest < ActionDispatch::IntegrationTest
 
   test 'trusted users can POST to #create' do
     sign_in trusted: true
+    Net::HTTP.stubs(:get).returns(<<~RUBY)
+      def xzczx
+        puts 45
+      end
+    RUBY
     assert_enqueued_with(job: RunUserBench) do
       VCR.use_cassette('github ruby 6ffef8d459') do
-        post '/user-scripts.json', params: { name: 'some_benchmark', url: 'http://script.com', sha: '6ffef8d459' }
+        post '/user-scripts.json', params: { name: 'some_benchmark', url: 'http://script.com/script.rb', sha: '6ffef8d459' }
       end
     end
     assert_response :success
-    assert_equal response.body, 'Success! Results will be published <a href="/ruby/ruby/commits?result_type=some_benchmark">here</a>.'
+    assert_equal response.body, I18n.t('user_scripts.index.successful_submit', links: '<a href="/ruby/ruby/commits?result_type=some_benchmark">some_benchmark</a>')
     assert_equal BenchmarkType.where(category: 'some_benchmark', from_user: true).count, 1
   end
 end

test/services/user_bench_test.rb

@@ -15,10 +15,17 @@ def valid_name
     'valid_name'
   end
 
-  def valid_url
-    'http://github.com'
+  def valid_url(ext = 'rb')
+    "https://raw.githubusercontent.com/ruby/ruby/master/benchmark/irb_color.#{ext}"
   end
 
+  def valid_ruby
+    <<~RUBY
+      def msdasd
+        puts 54544
+      end
+    RUBY
+  end
   test '#validate! @name presence, uniqueness and allowed characters' do
     bench = new_bench('')
     bench.validate!
@@ -30,9 +37,10 @@ def valid_url
       repo_id: 77
     )
 
+    Net::HTTP.stubs(:get).returns(valid_ruby)
     bench = new_bench('taken_name', valid_url, 'asdasd')
     bench.validate!
-    assert_includes bench.errors, err('name_already_taken')
+    assert_includes bench.errors, err('name_already_taken', names: 'taken_name')
 
     bench = new_bench('/invalid name', valid_url, 'dads')
     bench.validate!
@@ -55,6 +63,7 @@ def valid_url
     assert_includes bench.errors, err('missing_sha')
 
     bench = new_bench(valid_name, valid_url, 'doesntexist')
+    Net::HTTP.stubs(:get).returns(valid_ruby)
     VCR.use_cassette('github ruby doesntexist') do
       bench.validate!
     end
@@ -63,6 +72,7 @@ def valid_url
 
   test '#validate! when everything is valid' do
     bench = new_bench(valid_name, valid_url, '6ffef8d459')
+    Net::HTTP.stubs(:get).returns(valid_ruby)
     VCR.use_cassette('github ruby 6ffef8d459') do
       bench.validate!
     end
@@ -73,6 +83,7 @@ def valid_url
 
   test "#validate! @sha2 existence on github only if it's given" do
     bench = new_bench(valid_name, valid_url, '6ffef8d459', 'doesntexist')
+    Net::HTTP.stubs(:get).returns(valid_ruby)
     VCR.use_cassette('github ruby 6ffef8d459') do
       VCR.use_cassette('github ruby doesntexist') do
         bench.validate!
@@ -83,6 +94,7 @@ def valid_url
 
   test '#validate! allows 2 shas' do
     bench = new_bench(valid_name, valid_url, '6ffef8d459', 'fe0ddf0e58')
+    Net::HTTP.stubs(:get).returns(valid_ruby)
     VCR.use_cassette('github ruby 6ffef8d459') do
       VCR.use_cassette('github ruby fe0ddf0e58') do
         bench.validate!
@@ -96,4 +108,68 @@ def valid_url
     second_commit_date = bench.commits.second.commit.committer.date
     assert first_commit_date < second_commit_date
   end
+
+  test '#validate! yaml file and benchmark names inside of it are validated' do
+    yaml = <<~YAML
+      prelude: |
+        str1 = [*"a".."z",*"0".."9"].join("")
+        str10 = str1 * 10
+        str100 = str10 * 10
+        str1000 = str100 * 10
+      benchmark:
+        upcase-1: str1.upcase
+        upcase-10: str10.upcase
+        upcase-100: str100.upcase
+        upcase-1000: str1000.upcase
+    YAML
+    Net::HTTP.stubs(:get).returns(yaml)
+    bench = new_bench(valid_name, valid_url('yml'), '6ffef8d459')
+    VCR.use_cassette('github ruby 6ffef8d459') do
+      bench.validate!
+    end
+    expected = %w{upcase-1 upcase-10 upcase-100 upcase-1000}
+    assert bench.valid?
+    assert_equal bench.names.size, expected.size
+    expected.each { |e| assert_includes bench.names, e }
+  end
+
+  test '#validate! yaml syntax and benchmark-driver format' do
+    yaml = <<~YAML
+      prelude: |
+        str1 = [*"a".."z",*"0".."9"].join("")
+        str10 = str1 * 10
+        str100 = str10 * 10
+        str1000 = str100 * 10
+    YAML
+    Net::HTTP.stubs(:get).returns(yaml)
+    bench = new_bench(valid_name, valid_url('yml'), '6ffef8d459')
+    VCR.use_cassette('github ruby 6ffef8d459') do
+      bench.validate!
+    end
+    assert_not bench.valid?
+    assert_includes bench.errors, err('yaml_file_without_benchmarks')
+
+    yaml = 'asddsf: weads:'
+    Net::HTTP.stubs(:get).returns(yaml)
+    bench = new_bench(valid_name, valid_url('yml'), '6ffef8d459')
+    VCR.use_cassette('github ruby 6ffef8d459') do
+      bench.validate!
+    end
+    assert_not bench.valid?
+    assert_includes bench.errors, err('invalid_yaml')
+  end
+
+  test '#validate! ruby code' do
+    bad_ruby = <<~RUBY
+      def txxsr(
+        [1,2,].each do
+    RUBY
+    Net::HTTP.stubs(:get).returns(bad_ruby)
+    bench = new_bench(valid_name, valid_url, '6ffef8d459')
+    VCR.use_cassette('github ruby 6ffef8d459') do
+      bench.validate!
+    end
+    assert_not bench.valid?
+    assert_includes bench.errors.map(&:strip), err('invalid_ruby_code', lines: bad_ruby).strip
+  end
 end