Skip to content
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
3 changes: 3 additions & 0 deletions Pavel_Borisov/2/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
*.sublime-project
*.sublime-workspace

32 changes: 32 additions & 0 deletions Pavel_Borisov/2/checker.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
# frozen_string_literal: true

require_relative 'lib/machinery'

command_line = CliParser.new do |opts|
Copy link
Contributor

Choose a reason for hiding this comment

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

Это не command_line, это - parser

opts.banner = "Usage: #{__FILE__} [options] FILENAME"
opts.separator ''
opts.separator 'Available options:'
opts.on('--no-subdomains', 'Exclude entries with subdomains', TrueClass)
opts.on('--filter=WORD', 'Filter out results containing WORD in page body', String)
opts.on('--exclude-solutions', 'Exclude common open-source solutions', TrueClass)
end

command_line.parse!

domain_list_filename = command_line.args.first

if domain_list_filename.nil? || !File.exist?(domain_list_filename)
Copy link
Contributor

Choose a reason for hiding this comment

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

.nil? можно опустить, если конечно тебе не нужно, чтобы он был именно nil

print command_line.usage
exit
end

domains = DomainsList.new(domain_list_filename, command_line.options)

domains.process! do |percent, domain|
print "\e[1000D\e[0K#{percent}% complete – now checking #{domain}"
end
print "\e[1000D\e[0K"
Copy link
Contributor

Choose a reason for hiding this comment

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

Вынеси эту штуку в константу, а то довольно тяжело понять, что это такое)


domains.results.each { |r| puts r }
puts ''
puts domains.stats
145 changes: 145 additions & 0 deletions Pavel_Borisov/2/lib/machinery.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,145 @@
# frozen_string_literal: true
Copy link
Contributor

Choose a reason for hiding this comment

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

Для каждого класса необходимо создать отдельный файл. Это очень помогает в навигации по приложению и в чтении кода


require 'optparse'
require 'csv'
require 'net/http'
require 'nokogiri'

# Uses OptionParser to collect options into a hash
class CliParser
attr_reader :options, :args

def initialize(&block)
@options = {}
@args = []
@opt_parser = OptionParser.new(&block)
end

def parse!
@opt_parser.parse!(into: @options)
@args = @opt_parser.instance_variable_get(:@default_argv)
Comment on lines +19 to +20
Copy link
Contributor

Choose a reason for hiding this comment

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

После того, как объявлен attr_reader для инстанс переменных, можно использовать их без @, а-ля args, options etc

end

def to_s
@opt_parser.to_s
end

def usage
to_s
end
Comment on lines +23 to +29
Copy link
Contributor

Choose a reason for hiding this comment

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

Думаю, лучше объединить это в один метод

end

# Runs check for a single domain and contains the result
class DomainChecker
attr_reader :domain, :code, :response_time, :body, :status

def initialize(domain)
@domain = domain
@status = :unchecked
end

def check!
begin
timed_http_request
rescue StandardError => e
@status = :errored
cause = ('Timeout' if e.is_a? Timeout::Error) || e.cause
@error_message = "ERROR (#{cause})"
Comment on lines +45 to +47
Copy link
Contributor

Choose a reason for hiding this comment

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

Здесь нужно обрабатывать ошибки по отдельности, например:

def check!
  # begin в начале метода ставить не обязательно
  timed_http_request
rescue Timeout::Error => timeout_exception
  @status = :errored
  @error_message = "ERROR Timeout"
rescue => e
  p "Something went wrong: #{e.message}"
end

end
self
Copy link
Contributor

Choose a reason for hiding this comment

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

Зачем возвращаешь self?

end

def result
case @status
Copy link
Contributor

Choose a reason for hiding this comment

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

Та же история с attr_reader.

when :got_response
"#{@domain} - #{@code} (#{@response_time})"
when :errored
"#{@domain} - #{@error_message}"
when :unchecked
"#{@domain} - hasn't been checked"
end
end

private

def timed_http_request
http = Net::HTTP.new(@domain)
http.open_timeout = 1
http.read_timeout = 1
Comment on lines +66 to +68
Copy link
Contributor

Choose a reason for hiding this comment

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

Можно вынести создание http-клиента в отдельный метод, например client. Это сократит метод timed_http_request и упростит взаимодействие с клиентом(например, если нужно будет его переиспользовать)

start_time = Time.now.utc
response = http.get('/')
end_time = Time.now.utc
@response_time = "#{((end_time - start_time) * 1000).round}ms"
@code = response.code.to_i
@body = response.body
@status = :got_response
end
end

# Reads domains list from a file, filters the list according to the supplied options.
# Checks the domains using DomainChecker, filters the results if --filter option is specified.
# Provides methods to format results and stats for output.
class DomainsList
attr_reader :list

def initialize(filename, options)
@initial_list = CSV.read(filename).map(&:first)
Copy link
Contributor

Choose a reason for hiding this comment

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

Лучше переименовать @initial_list в, например, @domains_list. Сейчас проблема в том, что initial_list постепенно может измениться в процессе обработки и уже потом список будет не совсем initial :) Это не критично, но на такие моменты стоит обращать внимание.


reject_subdomains! if options[:'no-subdomains']
reject_solutions! if options[:'exclude-solutions']

@filtered_word = options[:filter]
@list = @initial_list.map { |domain| DomainChecker.new(domain) }
end

def process!
total_count = @list.count
processed_count = 0
@list.each do |item|
item.check!
processed_count += 1
completion_percentage = (processed_count / total_count.to_f * 100).to_i
yield(completion_percentage, item.domain) if block_given?
end
keep_results_with_word!(@filtered_word) if @filtered_word
self
Copy link
Contributor

Choose a reason for hiding this comment

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

Для чего возвращаешь self?

end

def results
@list.map(&:result)
end

def stats
total = @list.count
errored = @list.count { |item| item.status == :errored }
Copy link
Contributor

Choose a reason for hiding this comment

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

Можно немного упросить такие проверки путем добавления методов #error?, #finished?, #success?, #failed? и тд в DataChecker. Вообще, там есть неплохое подспорье для реализации стейт машины


success = @list.count do |item|
item.status == :got_response && (200..399).cover?(item.code)
end

failed = @list.count do |item|
item.status == :got_response && (400..599).cover?(item.code)
end

"Total: #{total}, Success: #{success}, Failed: #{failed}, Errored: #{errored}"
end

private

def reject_subdomains!
@initial_list.filter! { |domain| domain.count('.') == 1 }
end

def reject_solutions!
@initial_list.filter! do |domain|
!domain.match(/(\bgitlab\b|\bredmine\b|\bgit\b|\brepo\b|\brm\b|\bsource\b|\bsvn\b)/)
end
end

def keep_results_with_word!(word)
@list.keep_if do |response|
body_text = Nokogiri::HTML.parse(response.body).css('body').text
body_text.downcase.match? word.downcase
end
end
end
3 changes: 3 additions & 0 deletions Pavel_Borisov/2/test/fixtures/domains.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
example.com
subdomain.example.com
gitlab.com
57 changes: 57 additions & 0 deletions Pavel_Borisov/2/test/machinery_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
# frozen_string_literal: true

require 'minitest/autorun'
require_relative '../lib/machinery'

class DomainCheckerTest < Minitest::Test
def test_initial_status_is_unchecked
assert_equal :unchecked, DomainChecker.new('example.com').status
end

def test_unchecked_result_is_correct
assert_equal "example.com - hasn't been checked",
DomainChecker.new('example.com').result
Copy link
Contributor

Choose a reason for hiding this comment

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

Вынеси example.com и example.com.invld в константы, так будет проще к ним обращаться и изменять, в случае необходимости

end

def test_valid_domain_checked_status_is_got_response
assert_equal :got_response, DomainChecker.new('example.com').check!.status
end

def test_valid_domain_checked_result_is_correct
assert_match(/example.com - 200 \(\d+ms\)/,
DomainChecker.new('example.com').check!.result)
end

def test_invalid_domain_checked_status_is_errored
assert_equal :errored, DomainChecker.new('example.com.invld').check!.status
end

def test_invalid_domain_checked_result_is_correct
assert_match(/example.com.invld - ERROR/,
DomainChecker.new('example.com.invld').check!.result)
end
end

class DomainsListTest < Minitest::Test
Copy link
Contributor

Choose a reason for hiding this comment

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

В отдельный файл

TEST_DOMAINS_FILE = "#{File.dirname(__FILE__)}/fixtures/domains.csv"

def test_csv_file_is_read_correctly
assert_equal ['example.com', 'subdomain.example.com', 'gitlab.com'],
DomainsList.new(TEST_DOMAINS_FILE, {}).list.map(&:domain)
end

def test_no_subdomains_option_rejects_subdomains
assert_equal ['example.com', 'gitlab.com'],
DomainsList.new(TEST_DOMAINS_FILE, { 'no-subdomains': true }).list.map(&:domain)
end

def test_exclude_solutions_rejects_gitlab
assert_equal ['example.com', 'subdomain.example.com'],
DomainsList.new(TEST_DOMAINS_FILE, { 'exclude-solutions': true }).list.map(&:domain)
end

def test_filter_word_keeps_only_results_with_word_in_body
assert_equal ['example.com'],
DomainsList.new(TEST_DOMAINS_FILE, { filter: 'example' }).process!.list.map(&:domain)
end
end