Skip to content

Commit

Permalink
Code cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
sferik committed Feb 28, 2024
1 parent 67dc80a commit 0a3672e
Show file tree
Hide file tree
Showing 14 changed files with 118 additions and 103 deletions.
47 changes: 44 additions & 3 deletions .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,53 @@ require:
- standard
- standard-performance

inherit_gem:
standard: config/base.yml

AllCops:
NewCops: enable
TargetRubyVersion: 3.0

Layout/ArgumentAlignment:
EnforcedStyle: with_fixed_indentation
IndentationWidth: 2

Layout/CaseIndentation:
EnforcedStyle: end

Layout/EndAlignment:
EnforcedStyleAlignWith: start_of_line

Layout/LineLength:
Max: 140

Layout/ParameterAlignment:
EnforcedStyle: with_fixed_indentation
IndentationWidth: 2

Layout/SpaceInsideHashLiteralBraces:
EnforcedStyle: no_space

Metrics/ParameterLists:
CountKeywordArgs: false

Minitest/MultipleAssertions:
Max: 5

Style/Alias:
EnforcedStyle: prefer_alias_method

Style/Documentation:
Enabled: false

Style/FrozenStringLiteralComment:
EnforcedStyle: never

Style/OpenStructUse:
Enabled: false

Style/StringLiterals:
EnforcedStyle: double_quotes

Style/StringLiteralsInInterpolation:
EnforcedStyle: double_quotes

Style/TernaryParentheses:
EnforcedStyle: require_parentheses
4 changes: 0 additions & 4 deletions lib/x/cgi.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,5 @@ class CGI
def self.escape(value)
::CGI.escape(value).gsub("+", "%20")
end

def self.escape_params(params)
params.map { |k, v| "#{k}=#{escape(v)}" }.join("&")
end
end
end
19 changes: 7 additions & 12 deletions lib/x/client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -38,11 +38,10 @@ def initialize(api_key: nil, api_key_secret: nil, access_token: nil, access_toke
initialize_authenticator
@base_url = base_url
initialize_default_classes(default_array_class, default_object_class)
@connection = Connection.new(open_timeout: open_timeout, read_timeout: read_timeout,
write_timeout: write_timeout, debug_output: debug_output, proxy_url: proxy_url)
@connection = Connection.new(open_timeout: open_timeout, read_timeout: read_timeout, write_timeout: write_timeout,
debug_output: debug_output, proxy_url: proxy_url)
@request_builder = RequestBuilder.new
@redirect_handler = RedirectHandler.new(connection: @connection, request_builder: @request_builder,
max_redirects: max_redirects)
@redirect_handler = RedirectHandler.new(connection: @connection, request_builder: @request_builder, max_redirects: max_redirects)
@response_parser = ResponseParser.new
end

Expand All @@ -51,13 +50,11 @@ def get(endpoint, headers: {}, array_class: default_array_class, object_class: d
end

def post(endpoint, body = nil, headers: {}, array_class: default_array_class, object_class: default_object_class)
execute_request(:post, endpoint, body: body, headers: headers, array_class: array_class,
object_class: object_class)
execute_request(:post, endpoint, body: body, headers: headers, array_class: array_class, object_class: object_class)
end

def put(endpoint, body = nil, headers: {}, array_class: default_array_class, object_class: default_object_class)
execute_request(:put, endpoint, body: body, headers: headers, array_class: array_class,
object_class: object_class)
execute_request(:put, endpoint, body: body, headers: headers, array_class: array_class, object_class: object_class)
end

def delete(endpoint, headers: {}, array_class: default_array_class, object_class: default_object_class)
Expand Down Expand Up @@ -119,11 +116,9 @@ def initialize_authenticator

def execute_request(http_method, endpoint, body: nil, headers: {}, array_class: default_array_class, object_class: default_object_class)
uri = URI.join(base_url, endpoint)
request = @request_builder.build(http_method: http_method, uri: uri, body: body, headers: headers,
authenticator: @authenticator)
request = @request_builder.build(http_method: http_method, uri: uri, body: body, headers: headers, authenticator: @authenticator)
response = @connection.perform(request: request)
response = @redirect_handler.handle(response: response, request: request, base_url: base_url,
authenticator: @authenticator)
response = @redirect_handler.handle(response: response, request: request, base_url: base_url, authenticator: @authenticator)
@response_parser.parse(response: response, array_class: array_class, object_class: object_class)
end
end
Expand Down
10 changes: 4 additions & 6 deletions lib/x/media_uploader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,10 @@ def chunked_upload(client:, file_path:, media_category:, media_type: infer_media
media_category), boundary: SecureRandom.hex, chunk_size_mb: 8)
validate!(file_path: file_path, media_category: media_category)
upload_client = client.dup.tap { |c| c.base_url = "https://upload.twitter.com/1.1/" }
media = init(upload_client: upload_client, file_path: file_path, media_type: media_type,
media_category: media_category)
media = init(upload_client: upload_client, file_path: file_path, media_type: media_type, media_category: media_category)
chunk_size = chunk_size_mb * BYTES_PER_MB
append(upload_client: upload_client, file_paths: split(file_path, chunk_size), media: media,
media_type: media_type, boundary: boundary)
append(upload_client: upload_client, file_paths: split(file_path, chunk_size), media: media, media_type: media_type,
boundary: boundary)
upload_client.post("media/upload.json?command=FINALIZE&media_id=#{media["media_id"]}")
end

Expand Down Expand Up @@ -92,8 +91,7 @@ def append(upload_client:, file_paths:, media:, media_type:, boundary: SecureRan
upload_body = construct_upload_body(file_path: file_path, media_type: media_type, boundary: boundary)
query = "command=APPEND&media_id=#{media["media_id"]}&segment_index=#{index}"
headers = {"Content-Type" => "multipart/form-data, boundary=#{boundary}"}
upload_chunk(upload_client: upload_client, query: query, upload_body: upload_body, file_path: file_path,
headers: headers)
upload_chunk(upload_client: upload_client, query: query, upload_body: upload_body, file_path: file_path, headers: headers)
end
end
threads.each(&:join)
Expand Down
2 changes: 1 addition & 1 deletion lib/x/oauth_authenticator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ def hmac_signature(base_string)
end

def signature_base_string(method, url, params)
"#{method}&#{CGI.escape(url)}&#{CGI.escape(CGI.escape_params(params.sort))}"
"#{method}&#{CGI.escape(url)}&#{CGI.escape(URI.encode_www_form(params.sort))}"
end

def signing_key
Expand Down
3 changes: 1 addition & 2 deletions lib/x/request_builder.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
require "net/http"
require "uri"
require_relative "authenticator"
require_relative "cgi"
require_relative "version"

module X
Expand Down Expand Up @@ -51,7 +50,7 @@ def add_headers(request:, headers:)
end

def escape_query_params(uri)
URI(uri).tap { |u| u.query = CGI.escape_params(URI.decode_www_form(u.query)) if u.query }
URI(uri).tap { |u| u.query = URI.encode_www_form(URI.decode_www_form(u.query)) if u.query }
end
end
end
1 change: 0 additions & 1 deletion sig/x.rbs
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,5 @@ module X

class CGI
def self.escape: (String value) -> String
def self.escape_params: (Hash[String, String] | Array[[String, String]] params) -> String
end
end
9 changes: 9 additions & 0 deletions test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,15 @@
TEST_OAUTH_TIMESTAMP = Time.utc(1983, 11, 24).to_i.to_s
TEST_MEDIA_ID = 1_234_567_890

def test_oauth_credentials
{
api_key: TEST_API_KEY,
api_key_secret: TEST_API_KEY_SECRET,
access_token: TEST_ACCESS_TOKEN,
access_token_secret: TEST_ACCESS_TOKEN_SECRET
}
end

def test_oauth_params
{
"oauth_consumer_key" => TEST_API_KEY,
Expand Down
6 changes: 0 additions & 6 deletions test/x/cgi_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,5 @@ def test_escape
assert_equal "foo%2Bbar", X::CGI.escape("foo+bar")
assert_equal "%21%40%23%24", X::CGI.escape('!@#$')
end

def test_escape_params
assert_equal "key1=value1&key2=value2", X::CGI.escape_params([%w[key1 value1], %w[key2 value2]])
assert_equal "foo=bar%20baz", X::CGI.escape_params({"foo" => "bar baz"})
assert_equal "a=1%2F2&b=3%2B4", X::CGI.escape_params({"a" => "1/2", "b" => "3+4"})
end
end
end
19 changes: 5 additions & 14 deletions test/x/client_initailization_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,8 @@ def setup
@client = Client.new
end

def oauth_credentials
{
api_key: TEST_API_KEY,
api_key_secret: TEST_API_KEY_SECRET,
access_token: TEST_ACCESS_TOKEN,
access_token_secret: TEST_ACCESS_TOKEN_SECRET
}
end

def test_initialize_oauth_credentials
client = Client.new(**oauth_credentials)
client = Client.new(**test_oauth_credentials)

authenticator = client.instance_variable_get(:@authenticator)

Expand All @@ -31,15 +22,15 @@ def test_initialize_oauth_credentials
end

def test_missing_oauth_credentials
oauth_credentials.each_key do |missing_credential|
client = Client.new(**oauth_credentials.except(missing_credential))
test_oauth_credentials.each_key do |missing_credential|
client = Client.new(**test_oauth_credentials.except(missing_credential))

assert_instance_of Authenticator, client.instance_variable_get(:@authenticator)
end
end

def test_setting_oauth_credentials
oauth_credentials.each do |credential, value|
test_oauth_credentials.each do |credential, value|
@client.public_send(:"#{credential}=", value)

assert_equal value, @client.public_send(credential)
Expand All @@ -49,7 +40,7 @@ def test_setting_oauth_credentials
end

def test_setting_oauth_credentials_reinitializes_authenticator
oauth_credentials.each do |credential, value|
test_oauth_credentials.each do |credential, value|
initialize_authenticator_called = false
@client.stub :initialize_authenticator, -> { initialize_authenticator_called = true } do
@client.public_send(:"#{credential}=", value)
Expand Down
8 changes: 2 additions & 6 deletions test/x/client_request_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,7 @@ def setup
.to_return(body: '{"set": [1, 2, 2, 3]}', headers: {"Content-Type" => "application/json"})
ostruct = @client.public_send(http_method, "tweets", object_class: OpenStruct, array_class: Set)

assert_kind_of OpenStruct, ostruct
assert_kind_of Set, ostruct.set
assert_equal Set.new([1, 2, 3]), ostruct.set
assert_equal OpenStruct.new(set: Set.new([1, 2, 3])), ostruct
end

define_method :"test_#{http_method}_request_with_custom_response_objects_client_configuration" do
Expand All @@ -40,9 +38,7 @@ def setup
client = Client.new(default_object_class: OpenStruct, default_array_class: Set)
ostruct = client.public_send(http_method, "tweets")

assert_kind_of OpenStruct, ostruct
assert_kind_of Set, ostruct.set
assert_equal Set.new([1, 2, 3]), ostruct.set
assert_equal OpenStruct.new(set: Set.new([1, 2, 3])), ostruct
end
end

Expand Down
37 changes: 15 additions & 22 deletions test/x/media_uploader_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ def setup
end

def test_upload
file_path = "test/sample_files/sample.jpg"
stub_request(:post, "https://upload.twitter.com/1.1/media/upload.json?media_category=#{MediaUploader::TWEET_IMAGE}")
.to_return(body: @media.to_json, headers: {"Content-Type" => "application/json"})

result = MediaUploader.upload(client: @client, file_path: "test/sample_files/sample.jpg",
media_category: MediaUploader::TWEET_IMAGE, boundary: "AaB03x")
result = MediaUploader.upload(client: @client, file_path: file_path, media_category: MediaUploader::TWEET_IMAGE, boundary: "AaB03x")

assert_equal TEST_MEDIA_ID, result["media_id"]
end
Expand All @@ -30,24 +30,24 @@ def test_chunked_upload
stub_request(:post, "https://upload.twitter.com/1.1/media/upload.json?command=FINALIZE&media_id=#{TEST_MEDIA_ID}")
.to_return(status: 201, headers: {"content-type" => "application/json"}, body: @media.to_json)

response = MediaUploader.chunked_upload(client: @client, file_path: file_path,
media_category: MediaUploader::TWEET_VIDEO, chunk_size_mb: (total_bytes - 1) / MediaUploader::BYTES_PER_MB.to_f)
response = MediaUploader.chunked_upload(client: @client, file_path: file_path, media_category: MediaUploader::TWEET_VIDEO,
chunk_size_mb: (total_bytes - 1) / MediaUploader::BYTES_PER_MB.to_f)

assert_equal TEST_MEDIA_ID, response["media_id"]
end

def test_append_method
file_path = "test/sample_files/sample.mp4"
chunk_paths = MediaUploader.send(:split, file_path, File.size(file_path) - 1)
file_paths = MediaUploader.send(:split, file_path, File.size(file_path) - 1)

chunk_paths.each_with_index do |_chunk_path, segment_index|
file_paths.each_with_index do |_chunk_path, segment_index|
stub_request(:post, "https://upload.twitter.com/1.1/media/upload.json?command=APPEND&media_id=#{TEST_MEDIA_ID}&segment_index=#{segment_index}")
.with(headers: {"Content-Type" => "multipart/form-data, boundary=AaB03x"}).to_return(status: 204)
end
MediaUploader.send(:append, upload_client: @upload_client, file_paths: chunk_paths, media: @media,
MediaUploader.send(:append, upload_client: @upload_client, file_paths: file_paths, media: @media,
media_type: "video/mp4", boundary: "AaB03x")

chunk_paths.each_with_index { |_, segment_index| assert_requested(:post, "https://upload.twitter.com/1.1/media/upload.json?command=APPEND&media_id=#{TEST_MEDIA_ID}&segment_index=#{segment_index}") }
file_paths.each_with_index { |_, segment_index| assert_requested(:post, "https://upload.twitter.com/1.1/media/upload.json?command=APPEND&media_id=#{TEST_MEDIA_ID}&segment_index=#{segment_index}") }
end

def test_await_processing
Expand Down Expand Up @@ -102,38 +102,31 @@ def test_validate_with_invalid_media_category
end

def test_infer_media_type_for_gif
assert_equal MediaUploader::GIF_MIME_TYPE,
MediaUploader.send(:infer_media_type, "test/sample_files/sample.gif", "tweet_gif")
assert_equal MediaUploader::GIF_MIME_TYPE, MediaUploader.send(:infer_media_type, "test/sample_files/sample.gif", "tweet_gif")
end

def test_infer_media_type_for_jpg
assert_equal MediaUploader::JPEG_MIME_TYPE,
MediaUploader.send(:infer_media_type, "test/sample_files/sample.jpg", "tweet_image")
assert_equal MediaUploader::JPEG_MIME_TYPE, MediaUploader.send(:infer_media_type, "test/sample_files/sample.jpg", "tweet_image")
end

def test_infer_media_type_for_mp4
assert_equal MediaUploader::MP4_MIME_TYPE,
MediaUploader.send(:infer_media_type, "test/sample_files/sample.mp4", "tweet_video")
assert_equal MediaUploader::MP4_MIME_TYPE, MediaUploader.send(:infer_media_type, "test/sample_files/sample.mp4", "tweet_video")
end

def test_infer_media_type_for_png
assert_equal MediaUploader::PNG_MIME_TYPE,
MediaUploader.send(:infer_media_type, "test/sample_files/sample.png", "tweet_image")
assert_equal MediaUploader::PNG_MIME_TYPE, MediaUploader.send(:infer_media_type, "test/sample_files/sample.png", "tweet_image")
end

def test_infer_media_type_for_srt
assert_equal MediaUploader::SUBRIP_MIME_TYPE,
MediaUploader.send(:infer_media_type, "test/sample_files/sample.srt", "subtitles")
assert_equal MediaUploader::SUBRIP_MIME_TYPE, MediaUploader.send(:infer_media_type, "test/sample_files/sample.srt", "subtitles")
end

def test_infer_media_type_for_webp
assert_equal MediaUploader::WEBP_MIME_TYPE,
MediaUploader.send(:infer_media_type, "test/sample_files/sample.webp", "tweet_image")
assert_equal MediaUploader::WEBP_MIME_TYPE, MediaUploader.send(:infer_media_type, "test/sample_files/sample.webp", "tweet_image")
end

def test_infer_media_type_with_default
assert_equal MediaUploader::DEFAULT_MIME_TYPE,
MediaUploader.send(:infer_media_type, "test/sample_files/sample.unknown", "tweet_image")
assert_equal MediaUploader::DEFAULT_MIME_TYPE, MediaUploader.send(:infer_media_type, "test/sample_files/sample.dne", "tweet_image")
end
end
end
Loading

0 comments on commit 0a3672e

Please sign in to comment.