From a215c51a084d55a5ed2f52a7a61a92a00a1a20c3 Mon Sep 17 00:00:00 2001 From: Rajendra Kadam <rkadam@gitlab.com> Date: Wed, 7 Feb 2024 17:37:12 +0000 Subject: [PATCH] Delete unused BufferedIO class Delete the specs for BufferedIO class Update any other references in the codebase for V2 --- .rubocop_todo/gitlab/namespaced_class.yml | 1 - .rubocop_todo/layout/line_length.yml | 2 - .../lint/assignment_in_condition.yml | 1 - .../lint/missing_cop_enable_directive.yml | 1 - .../lint/redundant_cop_disable_directive.yml | 1 - .rubocop_todo/rspec/feature_category.yml | 1 - .../style/inline_disable_annotation.yml | 1 - .../gitlab/http_v2/new_connection_adapter.rb | 2 +- .../lib/net_http/response_patch.rb | 3 -- lib/gitlab/buffered_io.rb | 46 ----------------- .../net_http_response_patch_spec.rb | 6 +-- spec/lib/gitlab/buffered_io_spec.rb | 50 ------------------- spec/support/rspec_order_todo.yml | 1 - 13 files changed, 4 insertions(+), 112 deletions(-) delete mode 100644 lib/gitlab/buffered_io.rb delete mode 100644 spec/lib/gitlab/buffered_io_spec.rb diff --git a/.rubocop_todo/gitlab/namespaced_class.yml b/.rubocop_todo/gitlab/namespaced_class.yml index 646dcf654baa3..825f48e8dcdbf 100644 --- a/.rubocop_todo/gitlab/namespaced_class.yml +++ b/.rubocop_todo/gitlab/namespaced_class.yml @@ -1079,7 +1079,6 @@ Gitlab/NamespacedClass: - 'lib/gitlab/batch_worker_context.rb' - 'lib/gitlab/blame.rb' - 'lib/gitlab/branch_push_merge_commit_analyzer.rb' - - 'lib/gitlab/buffered_io.rb' - 'lib/gitlab/build_access.rb' - 'lib/gitlab/changes_list.rb' - 'lib/gitlab/chaos.rb' diff --git a/.rubocop_todo/layout/line_length.yml b/.rubocop_todo/layout/line_length.yml index 5f9a6d282c886..2095d2c1ca3e1 100644 --- a/.rubocop_todo/layout/line_length.yml +++ b/.rubocop_todo/layout/line_length.yml @@ -2443,7 +2443,6 @@ Layout/LineLength: - 'lib/gitlab/background_migration/migrate_requirements_to_work_items.rb' - 'lib/gitlab/background_migration/populate_resolved_on_default_branch_column.rb' - 'lib/gitlab/background_migration/project_namespaces/backfill_project_namespaces.rb' - - 'lib/gitlab/buffered_io.rb' - 'lib/gitlab/bullet/exclusions.rb' - 'lib/gitlab/cache/helpers.rb' - 'lib/gitlab/changelog/config.rb' @@ -3497,7 +3496,6 @@ Layout/LineLength: - 'spec/lib/gitlab/background_migration/job_coordinator_spec.rb' - 'spec/lib/gitlab/background_migration/legacy_upload_mover_spec.rb' - 'spec/lib/gitlab/background_migration/update_jira_tracker_data_deployment_type_based_on_url_spec.rb' - - 'spec/lib/gitlab/buffered_io_spec.rb' - 'spec/lib/gitlab/cache/ci/project_pipeline_status_spec.rb' - 'spec/lib/gitlab/chat/output_spec.rb' - 'spec/lib/gitlab/checks/branch_check_spec.rb' diff --git a/.rubocop_todo/lint/assignment_in_condition.yml b/.rubocop_todo/lint/assignment_in_condition.yml index 20a51d0c774a7..44dc885eb8372 100644 --- a/.rubocop_todo/lint/assignment_in_condition.yml +++ b/.rubocop_todo/lint/assignment_in_condition.yml @@ -152,7 +152,6 @@ Lint/AssignmentInCondition: - 'lib/gitlab/authorized_keys.rb' - 'lib/gitlab/background_migration/fix_projects_without_project_feature.rb' - 'lib/gitlab/blob_helper.rb' - - 'lib/gitlab/buffered_io.rb' - 'lib/gitlab/changelog/config.rb' - 'lib/gitlab/changelog/generator.rb' - 'lib/gitlab/chat/output.rb' diff --git a/.rubocop_todo/lint/missing_cop_enable_directive.yml b/.rubocop_todo/lint/missing_cop_enable_directive.yml index 7716c274f18b6..5a4e8c8bb654a 100644 --- a/.rubocop_todo/lint/missing_cop_enable_directive.yml +++ b/.rubocop_todo/lint/missing_cop_enable_directive.yml @@ -137,7 +137,6 @@ Lint/MissingCopEnableDirective: - 'lib/gitlab/background_migration/populate_resolved_on_default_branch_column.rb' - 'lib/gitlab/background_migration/purge_stale_security_scans.rb' - 'lib/gitlab/background_migration/update_jira_tracker_data_deployment_type_based_on_url.rb' - - 'lib/gitlab/buffered_io.rb' - 'lib/gitlab/ci/reports/test_suite_summary.rb' - 'lib/gitlab/data_builder/push.rb' - 'lib/gitlab/database/load_balancing/connection_proxy.rb' diff --git a/.rubocop_todo/lint/redundant_cop_disable_directive.yml b/.rubocop_todo/lint/redundant_cop_disable_directive.yml index f10700b8d81b6..5555a3b67434b 100644 --- a/.rubocop_todo/lint/redundant_cop_disable_directive.yml +++ b/.rubocop_todo/lint/redundant_cop_disable_directive.yml @@ -165,7 +165,6 @@ Lint/RedundantCopDisableDirective: - 'lib/gitlab/background_migration/re_expire_o_auth_tokens.rb' - 'lib/gitlab/background_migration/remove_occurrence_pipelines_and_duplicate_vulnerabilities_findings.rb' - 'lib/gitlab/background_migration/update_jira_tracker_data_deployment_type_based_on_url.rb' - - 'lib/gitlab/buffered_io.rb' - 'lib/gitlab/cache/request_cache.rb' - 'lib/gitlab/ci/build/artifacts/metadata/entry.rb' - 'lib/gitlab/ci/pipeline/chain/command.rb' diff --git a/.rubocop_todo/rspec/feature_category.yml b/.rubocop_todo/rspec/feature_category.yml index d5b9845951e08..9a22fcee54ba1 100644 --- a/.rubocop_todo/rspec/feature_category.yml +++ b/.rubocop_todo/rspec/feature_category.yml @@ -2789,7 +2789,6 @@ RSpec/FeatureCategory: - 'spec/lib/gitlab/blame_spec.rb' - 'spec/lib/gitlab/blob_helper_spec.rb' - 'spec/lib/gitlab/branch_push_merge_commit_analyzer_spec.rb' - - 'spec/lib/gitlab/buffered_io_spec.rb' - 'spec/lib/gitlab/build_access_spec.rb' - 'spec/lib/gitlab/bullet_spec.rb' - 'spec/lib/gitlab/cache/helpers_spec.rb' diff --git a/.rubocop_todo/style/inline_disable_annotation.yml b/.rubocop_todo/style/inline_disable_annotation.yml index c8b4dfedc4107..9983b16d6b7e4 100644 --- a/.rubocop_todo/style/inline_disable_annotation.yml +++ b/.rubocop_todo/style/inline_disable_annotation.yml @@ -2325,7 +2325,6 @@ Style/InlineDisableAnnotation: - 'lib/gitlab/bitbucket_import/importers/issue_notes_importer.rb' - 'lib/gitlab/bitbucket_server_import/importers/pull_request_notes_importer.rb' - 'lib/gitlab/blob_helper.rb' - - 'lib/gitlab/buffered_io.rb' - 'lib/gitlab/cache/ci/project_pipeline_status.rb' - 'lib/gitlab/cache/import/caching.rb' - 'lib/gitlab/cache/request_cache.rb' diff --git a/gems/gitlab-http/lib/gitlab/http_v2/new_connection_adapter.rb b/gems/gitlab-http/lib/gitlab/http_v2/new_connection_adapter.rb index 6a1f1f0f5e367..30a413610f635 100644 --- a/gems/gitlab-http/lib/gitlab/http_v2/new_connection_adapter.rb +++ b/gems/gitlab-http/lib/gitlab/http_v2/new_connection_adapter.rb @@ -14,7 +14,7 @@ # # 2. Header timeouts # When the use_read_total_timeout option is used, that means the receiver -# of the HTTP request cannot be trusted. Gitlab::BufferedIo will be used, +# of the HTTP request cannot be trusted. Gitlab::HTTP_V2::BufferedIo will be used, # to read header data. It is a modified version of Net::BufferedIO that # raises a timeout error if reading header data takes too much time. diff --git a/gems/gitlab-http/lib/net_http/response_patch.rb b/gems/gitlab-http/lib/net_http/response_patch.rb index 303d629b32e4f..107a961dc6dd9 100644 --- a/gems/gitlab-http/lib/net_http/response_patch.rb +++ b/gems/gitlab-http/lib/net_http/response_patch.rb @@ -22,9 +22,6 @@ def self.each_response_header(sock) while true uses_buffered_io = sock.is_a?(Gitlab::HTTP_V2::BufferedIo) - # TODO: Gitlab::BufferedIo is temporarily used for an easy migration. - uses_buffered_io ||= sock.is_a?(Gitlab::BufferedIo) if defined?(Gitlab::BufferedIo) - line = uses_buffered_io ? sock.readuntil("\n", true, start_time) : sock.readuntil("\n", true) line = line.sub(/\s{0,10}\z/, '') break if line.empty? diff --git a/lib/gitlab/buffered_io.rb b/lib/gitlab/buffered_io.rb deleted file mode 100644 index b76c8191fa295..0000000000000 --- a/lib/gitlab/buffered_io.rb +++ /dev/null @@ -1,46 +0,0 @@ -# frozen_string_literal: true - -module Gitlab - # Net::BufferedIO is overwritten by webmock but in order to test this class, it needs to inherit from the original BufferedIO. - # https://github.com/bblimke/webmock/blob/867f4b290fd133658aa9530cba4ba8b8c52c0d35/lib/webmock/http_lib_adapters/net_http.rb#L266 - parent_class = if const_defined?('WebMock::HttpLibAdapters::NetHttpAdapter::OriginalNetBufferedIO') && Rails.env.test? - WebMock::HttpLibAdapters::NetHttpAdapter::OriginalNetBufferedIO - else - Net::BufferedIO - end - - class BufferedIo < parent_class - extend ::Gitlab::Utils::Override - - HEADER_READ_TIMEOUT = 20 - - # rubocop: disable Style/RedundantBegin - # rubocop: disable Style/RedundantReturn - # rubocop: disable Cop/LineBreakAfterGuardClauses - # rubocop: disable Layout/EmptyLineAfterGuardClause - - # Original method: - # https://github.com/ruby/ruby/blob/cdb7d699d0641e8f081d590d06d07887ac09961f/lib/net/protocol.rb#L190-L200 - override :readuntil - def readuntil(terminator, ignore_eof = false, start_time = Process.clock_gettime(Process::CLOCK_MONOTONIC)) - begin - until idx = @rbuf.index(terminator) - if (elapsed = Process.clock_gettime(Process::CLOCK_MONOTONIC) - start_time) > HEADER_READ_TIMEOUT - raise Gitlab::HTTP::HeaderReadTimeout, "Request timed out after reading headers for #{elapsed} seconds" - end - - rbuf_fill - end - - return rbuf_consume(idx + terminator.size) - rescue EOFError - raise unless ignore_eof - return rbuf_consume(@rbuf.size) - end - end - # rubocop: disable Style/RedundantBegin - # rubocop: enable Style/RedundantReturn - # rubocop: enable Cop/LineBreakAfterGuardClauses - # rubocop: enable Layout/EmptyLineAfterGuardClause - end -end diff --git a/spec/initializers/net_http_response_patch_spec.rb b/spec/initializers/net_http_response_patch_spec.rb index 8074047d6aa36..b46f9514a8945 100644 --- a/spec/initializers/net_http_response_patch_spec.rb +++ b/spec/initializers/net_http_response_patch_spec.rb @@ -17,7 +17,7 @@ end before do - stub_const('Gitlab::BufferedIo::HEADER_READ_TIMEOUT', 0.1) + stub_const('Gitlab::HTTP_V2::BufferedIo::HEADER_READ_TIMEOUT', 0.1) end subject(:each_response_header) { Net::HTTPResponse.each_response_header(socket) { |k, v| } } @@ -46,9 +46,9 @@ end end - context 'with Gitlab::BufferedIo' do + context 'with Gitlab::HTTP_V2::BufferedIo' do let(:mock_io) { StringIO.new(server_response) } - let(:socket) { Gitlab::BufferedIo.new(mock_io) } + let(:socket) { Gitlab::HTTP_V2::BufferedIo.new(mock_io) } it 'forwards start time to the socket' do allow(socket).to receive(:readuntil).and_call_original diff --git a/spec/lib/gitlab/buffered_io_spec.rb b/spec/lib/gitlab/buffered_io_spec.rb deleted file mode 100644 index 0ec377550c184..0000000000000 --- a/spec/lib/gitlab/buffered_io_spec.rb +++ /dev/null @@ -1,50 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe Gitlab::BufferedIo do - describe '#readuntil' do - let(:mock_io) { StringIO.new('a') } - let(:start_time) { Process.clock_gettime(Process::CLOCK_MONOTONIC) } - - before do - stub_const('Gitlab::BufferedIo::HEADER_READ_TIMEOUT', 0.1) - end - - subject(:readuntil) do - described_class.new(mock_io).readuntil('a', false, start_time) - end - - it 'does not raise a timeout error' do - expect { readuntil }.not_to raise_error - end - - context 'when the response contains infinitely long headers' do - before do - read_counter = 0 - - allow(mock_io).to receive(:read_nonblock) do |buffer_size, *_| - read_counter += 1 - raise 'Test did not raise HeaderReadTimeout' if read_counter > 10 - - sleep 0.01 - 'H' * buffer_size - end - end - - it 'raises a timeout error' do - expect { readuntil }.to raise_error(Gitlab::HTTP::HeaderReadTimeout, /Request timed out after reading headers for 0\.[0-9]+ seconds/) - end - - context 'when not passing start_time' do - subject(:readuntil) do - described_class.new(mock_io).readuntil('a', false) - end - - it 'raises a timeout error' do - expect { readuntil }.to raise_error(Gitlab::HTTP::HeaderReadTimeout, /Request timed out after reading headers for 0\.[0-9]+ seconds/) - end - end - end - end -end diff --git a/spec/support/rspec_order_todo.yml b/spec/support/rspec_order_todo.yml index 170df80f730de..d1f126eb6f942 100644 --- a/spec/support/rspec_order_todo.yml +++ b/spec/support/rspec_order_todo.yml @@ -5342,7 +5342,6 @@ - './spec/lib/gitlab/blame_spec.rb' - './spec/lib/gitlab/blob_helper_spec.rb' - './spec/lib/gitlab/branch_push_merge_commit_analyzer_spec.rb' -- './spec/lib/gitlab/buffered_io_spec.rb' - './spec/lib/gitlab/build_access_spec.rb' - './spec/lib/gitlab/bullet/exclusions_spec.rb' - './spec/lib/gitlab/bullet_spec.rb' -- GitLab