diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index c018867fa9..9947efd7c6 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config --no-exclude-limit` -# on 2026-04-22 15:46:09 UTC using RuboCop version 1.86.1. +# on 2026-05-05 14:43:06 UTC using RuboCop version 1.86.1. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new @@ -301,12 +301,11 @@ Lint/UselessOr: - 'app/models/qcable/statemachine.rb' - 'app/models/ui_helper/summary.rb' -# Offense count: 11 +# Offense count: 7 # Configuration parameters: AllowedMethods, AllowedPatterns, CountRepeatedAttributes, Max. Metrics/AbcSize: Exclude: - 'app/controllers/api/v2/transfers_controller.rb' - - 'app/controllers/studies/information_controller.rb' - 'app/jobs/export_pool_xp_to_traction_job.rb' - 'app/models/accession_service/base_service.rb' - 'app/sample_manifest_excel/sample_manifest_excel/manifest_type_list.rb' @@ -334,7 +333,7 @@ Metrics/CyclomaticComplexity: - 'app/models/accession_service/base_service.rb' - 'lib/limber/helper.rb' -# Offense count: 19 +# Offense count: 18 # Configuration parameters: CountComments, Max, CountAsOne, AllowedMethods, AllowedPatterns. Metrics/MethodLength: Exclude: @@ -617,7 +616,7 @@ RSpec/BeforeAfterAll: - 'spec/sample_manifest_excel/worksheet_spec.rb' - 'spec/sequencescape_excel/worksheet_spec.rb' -# Offense count: 342 +# Offense count: 330 # Configuration parameters: Prefixes, AllowedPatterns. # Prefixes: when, with, without RSpec/ContextWording: @@ -649,7 +648,6 @@ RSpec/ContextWording: - 'spec/models/barcode_spec.rb' - 'spec/models/broadcast_event/lab_event_spec.rb' - 'spec/models/broadcast_event/qc_assay_spec.rb' - - 'spec/models/bulk_submission_spec.rb' - 'spec/models/comment_spec.rb' - 'spec/models/illumina_htp/initial_stock_tube_purpose_spec.rb' - 'spec/models/location_report_form_spec.rb' @@ -840,7 +838,7 @@ RSpec/ExampleWording: - 'spec/sequencescape_excel/validation_spec.rb' - 'spec/sequencescape_excel/worksheet_spec.rb' -# Offense count: 27 +# Offense count: 26 RSpec/ExpectInHook: Exclude: - 'spec/controllers/labwhere_receptions_controller_spec.rb' @@ -1197,7 +1195,7 @@ RSpec/MultipleExpectations: - 'spec/views/labware/show_chromium_chip_spec.rb' - 'spec/views/samples/index_html_erb_spec.rb' -# Offense count: 243 +# Offense count: 249 # Configuration parameters: EnforcedStyle, IgnoreSharedExamples. # SupportedStyles: always, named_only RSpec/NamedSubject: diff --git a/Gemfile b/Gemfile index b7a7b80445..1157904c8f 100644 --- a/Gemfile +++ b/Gemfile @@ -21,10 +21,6 @@ group :default do gem 'faraday-multipart' gem 'rest-client' # Deprecated, but still used in some places, replace with Faraday where possible - # Fix incompatibility with between Ruby 3.1 and Psych 4 (used for yaml) - # see https://stackoverflow.com/a/71192990 - gem 'psych', '< 4' - # State machine gem 'aasm' gem 'after_commit_everywhere', '~> 1.0' # Required by AASM @@ -34,7 +30,7 @@ group :default do # Provides bulk insert capabilities gem 'activerecord-import' - gem 'record_loader', git: 'https://github.com/sanger/record_loader' + gem 'record_loader', git: 'https://github.com/sanger/record_loader', branch: 'sh51/psych-5' gem 'mysql2', platforms: :mri gem 'will_paginate' diff --git a/Gemfile.lock b/Gemfile.lock index 612b74f9c8..5ea5a0267b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -16,9 +16,11 @@ GIT GIT remote: https://github.com/sanger/record_loader - revision: 9e7481f4d2342f042ab13465962e5d6689863198 + revision: cbd59574d1090a0a81a770478a6b4042fa882f6a + branch: sh51/psych-5 specs: - record_loader (0.3.0) + record_loader (1.0.0) + psych (~> 5.0) GIT remote: https://github.com/sanger/sanger_barcode_format.git @@ -377,7 +379,9 @@ GEM pry (>= 0.13, < 0.17) pry-rails (0.3.11) pry (>= 0.13.0) - psych (3.3.4) + psych (5.3.1) + date + stringio public_suffix (7.0.5) puma (7.2.0) nio4r (~> 2.0) @@ -588,6 +592,7 @@ GEM rbtree set (~> 1.0) ssrf_filter (1.2.0) + stringio (3.2.0) syntax_tree (6.3.0) prettier_print (>= 1.2.0) syntax_tree-haml (4.0.3) @@ -715,7 +720,6 @@ DEPENDENCIES prettier_print pry-byebug pry-rails - psych (< 4) puma rack-acceptable rack-cors diff --git a/app/models/broadcast_event.rb b/app/models/broadcast_event.rb index 7493d841e6..146c5571ea 100644 --- a/app/models/broadcast_event.rb +++ b/app/models/broadcast_event.rb @@ -19,7 +19,7 @@ class BroadcastEvent < ApplicationRecord # https://api.rubyonrails.org/classes/ActiveRecord/Inheritance/ClassMethods.html validates :sti_type, presence: true - serialize :properties, coder: YAML + serialize :properties, coder: YAML, yaml: { permitted_classes: [ActionController::Parameters] } self.inheritance_column = 'sti_type' broadcast_with_warren diff --git a/app/models/request_type/validator.rb b/app/models/request_type/validator.rb index d863ac909f..db32ee1cba 100644 --- a/app/models/request_type/validator.rb +++ b/app/models/request_type/validator.rb @@ -104,7 +104,14 @@ def valid_options belongs_to :request_type, optional: false validates :request_option, :valid_options, presence: true - serialize :valid_options, coder: YAML + serialize :valid_options, coder: YAML, yaml: { + permitted_classes: [ + Range, + RequestType::Validator::ArrayWithDefault, + RequestType::Validator::FlowcellTypeValidator, + RequestType::Validator::LibraryTypeValidator + ] + } delegate :include?, to: :valid_options diff --git a/app/models/submission/request_options_behaviour.rb b/app/models/submission/request_options_behaviour.rb index f140271a35..24489254f1 100644 --- a/app/models/submission/request_options_behaviour.rb +++ b/app/models/submission/request_options_behaviour.rb @@ -7,7 +7,14 @@ class HashWrapper def self.load(hash_yaml) return hash_yaml if hash_yaml.nil? - YAML.load(hash_yaml) + YAML.safe_load(hash_yaml, + aliases: true, + permitted_classes: [ + ActiveSupport::HashWithIndifferentAccess, + ActiveSupport::TimeWithZone, + ActiveSupport::TimeZone, + Time + ]) end def self.dump(hash) @@ -28,22 +35,20 @@ def request_options=(options) super end + private + def check_request_options check_multipliers_are_valid end - private :check_request_options - # rubocop:todo Metrics/PerceivedComplexity, Metrics/AbcSize - def check_multipliers_are_valid # rubocop:todo Metrics/CyclomaticComplexity + def check_multipliers_are_valid # rubocop:disable Metrics/CyclomaticComplexity multipliers = request_options.try(:[], :multiplier) return if multipliers.blank? # We're ok with nothing being specified! # TODO[xxx]: should probably error if they've specified a request type that isn't being used - errors.add(:request_options, 'negative multiplier supplied') if multipliers.values.map(&:to_i).any?(&:negative?) - errors.add(:request_options, 'zero multiplier supplied') if multipliers.values.map(&:to_i).any?(&:zero?) + multiplier_values = multipliers.values.map(&:to_i) + errors.add(:request_options, 'negative multiplier supplied') if multiplier_values.any?(&:negative?) + errors.add(:request_options, 'zero multiplier supplied') if multiplier_values.any?(&:zero?) false unless errors.empty? end - - # rubocop:enable Metrics/AbcSize, Metrics/PerceivedComplexity - private :check_multipliers_are_valid end diff --git a/app/sequencescape_excel/sequencescape_excel/helpers.rb b/app/sequencescape_excel/sequencescape_excel/helpers.rb index 07c60d4513..0317705694 100644 --- a/app/sequencescape_excel/sequencescape_excel/helpers.rb +++ b/app/sequencescape_excel/sequencescape_excel/helpers.rb @@ -5,7 +5,8 @@ module SequencescapeExcel # Helpers module Helpers def load_file(folder, filename) - YAML.load_file(Rails.root.join(folder, "#{filename}.yml")).with_indifferent_access + file_path = Rails.root.join(folder, "#{filename}.yml") + YAML.safe_load_file(file_path, permitted_classes: [Symbol], aliases: true).with_indifferent_access end end end diff --git a/config/default_records/asset_shapes/default_records.yml b/config/default_records/asset_shapes/default_records.yml index 593842c86c..6e5b3778b5 100644 --- a/config/default_records/asset_shapes/default_records.yml +++ b/config/default_records/asset_shapes/default_records.yml @@ -62,8 +62,8 @@ Fluidigm192: sizes: [192] StripTubeColumn: - horizontal_ratio: 1, - vertical_ratio: 8, + horizontal_ratio: 1 + vertical_ratio: 8 description_strategy: Sequential sizes: [8] diff --git a/config/initializers/accession.rb b/config/initializers/accession.rb index 5602bbdd39..cd061dc4f8 100644 --- a/config/initializers/accession.rb +++ b/config/initializers/accession.rb @@ -9,5 +9,5 @@ end # add ena requirement fields here -ena_requirement_fields = YAML.load_file('config/ena_requirement_fields.yml') +ena_requirement_fields = YAML.safe_load_file('config/ena_requirement_fields.yml') Rails.application.config.ena_requirement_fields = ena_requirement_fields.with_indifferent_access diff --git a/config/initializers/failure_reasons.rb b/config/initializers/failure_reasons.rb index 87c12b48f6..4efde706db 100644 --- a/config/initializers/failure_reasons.rb +++ b/config/initializers/failure_reasons.rb @@ -1,2 +1,2 @@ # frozen_string_literal: true -FAILURE_REASONS = YAML.load(File.open("#{Rails.root}/config/failure_reasons.yml")) +FAILURE_REASONS = YAML.safe_load_file("#{Rails.root}/config/failure_reasons.yml") diff --git a/config/initializers/flipper.rb b/config/initializers/flipper.rb index c64a5c5c81..5d9d87a3b9 100644 --- a/config/initializers/flipper.rb +++ b/config/initializers/flipper.rb @@ -2,7 +2,7 @@ require 'yaml' require 'flipper/adapters/active_record' -FLIPPER_FEATURES = YAML.load_file('./config/feature_flags.yml') +FLIPPER_FEATURES = YAML.safe_load_file('./config/feature_flags.yml') Rails.application.configure do ## Memoization ensures that only one adapter call is made per feature per request. diff --git a/config/initializers/phi_x.rb b/config/initializers/phi_x.rb index d9c687a077..b4c8cb406e 100644 --- a/config/initializers/phi_x.rb +++ b/config/initializers/phi_x.rb @@ -1,4 +1,4 @@ # frozen_string_literal: true -phi_x_config = YAML.load_file('config/phi_x.yml') +phi_x_config = YAML.safe_load_file('config/phi_x.yml') Rails.application.config.phi_x = phi_x_config.with_indifferent_access diff --git a/config/initializers/process_locale_files_with_erb.rb b/config/initializers/process_locale_files_with_erb.rb index c4eb19e95d..78cc131d13 100644 --- a/config/initializers/process_locale_files_with_erb.rb +++ b/config/initializers/process_locale_files_with_erb.rb @@ -4,7 +4,7 @@ module I18n module Backend module Base def load_yml(filename) - YAML.load(ERB.new(File.read(filename)).result) + YAML.unsafe_load(ERB.new(File.read(filename)).result) end end end diff --git a/config/initializers/psych.rb b/config/initializers/psych.rb index 472b3025db..d4c7384d6e 100644 --- a/config/initializers/psych.rb +++ b/config/initializers/psych.rb @@ -1,22 +1,6 @@ # frozen_string_literal: true Rails.application.configure do - # Fix for Psych::DisallowedClass: Tried to load unspecified class - config.active_record.yaml_column_permitted_classes = - Array(config.active_record.yaml_column_permitted_classes) + - %w[ - Symbol - ActiveSupport::HashWithIndifferentAccess - ActiveSupport::TimeWithZone - ActiveSupport::TimeZone - HashWithIndifferentAccess - RequestType::Validator::ArrayWithDefault - RequestType::Validator::LibraryTypeValidator - RequestType::Validator::FlowcellTypeValidator - ActionController::Parameters - Set - Range - FieldInfo - Time - ] + # Allow YAML columns to contain HashWithIndifferentAccess objects by default + ActiveRecord.yaml_column_permitted_classes += [ActiveSupport::HashWithIndifferentAccess] end diff --git a/db/seeds/0001_snp_plate_purposes.rb b/db/seeds/0001_snp_plate_purposes.rb index 2693e3e51a..c2c5b73765 100644 --- a/db/seeds/0001_snp_plate_purposes.rb +++ b/db/seeds/0001_snp_plate_purposes.rb @@ -51,7 +51,7 @@ EOS YAML - .load(plate_purposes) + .unsafe_load(plate_purposes) .each do |plate_purpose| attributes = plate_purpose.reverse_merge( diff --git a/lib/accession.rb b/lib/accession.rb index 8c27017783..64d98f2365 100644 --- a/lib/accession.rb +++ b/lib/accession.rb @@ -24,7 +24,8 @@ module Accession # @see ftp://ftp.sra.ebi.ac.uk/meta/xsd/ Schema definitions module Helpers def load_file(folder, filename) - YAML.load_file(Rails.root.join(folder, "#{filename}.yml")).with_indifferent_access + file_path = Rails.root.join(folder, "#{filename}.yml") + YAML.safe_load_file(file_path, permitted_classes: [Symbol]).with_indifferent_access end end diff --git a/spec/models/bulk_submission_spec.rb b/spec/models/bulk_submission_spec.rb index 68422d4018..6cbde9a8ce 100644 --- a/spec/models/bulk_submission_spec.rb +++ b/spec/models/bulk_submission_spec.rb @@ -57,7 +57,7 @@ create(:project, name: 'Test project') end - context 'a simple submission' do + context 'when creating a simple submission' do let(:submission_template_hash) do { name: 'Illumina-A - Cherrypick for pulldown - Pulldown WGS - HiSeq Paired end sequencing', @@ -93,7 +93,7 @@ end end - context 'an asset driven submission' do + context 'when creating an asset driven submission' do let(:spreadsheet_filename) { 'template_for_bulk_submission.csv' } let!(:asset) { create(:plate, barcode: 'SQPD-1', well_count: 1, well_factory: :untagged_well) } let(:submission_template_hash) do @@ -126,7 +126,7 @@ end end - context 'a submission with PCR cycles' do + context 'when creating a submission with PCR cycles' do let(:spreadsheet_filename) { 'pcr_cycles.csv' } let!(:submission_template) do @@ -157,7 +157,7 @@ end end - context 'a submission with primer_panels' do + context 'when creating a submission with primer_panels' do let(:spreadsheet_filename) { 'primer_panels.csv' } let!(:primer_panel) { create(:primer_panel, name: 'Test panel') } @@ -190,7 +190,7 @@ end end - context 'a submission with bait libraries' do + context 'when creating a submission with bait libraries' do let(:spreadsheet_filename) { '2_valid_sc_submissions.csv' } let!(:bait_library) { create(:bait_library, name: 'Bait library 1') } let!(:bait_library_2) { create(:bait_library, name: 'Bait library 2') } @@ -222,7 +222,7 @@ end end - context 'a submission with a lowercase library type' do + context 'when creating a submission with a lowercase library type' do let(:spreadsheet_filename) { 'with_lowercase_library_type.csv' } let!(:submission_template) do @@ -253,7 +253,7 @@ end end - context 'a submission with an unrecognised library type' do + context 'when creating a submission with an unrecognised library type' do let(:spreadsheet_filename) { 'with_unknown_library_type.csv' } let!(:submission_template) do @@ -274,7 +274,7 @@ end end - context 'a submission with additional template name validations' do + context 'when creating a submission with additional template name validations' do context 'when valid for scRNA template' do let(:submission_template_hash) do { @@ -420,13 +420,13 @@ end context 'when an scRNA Bulk Submission given with invalid number of samples per pool' do - context 'number of samples per pool < 5' do + context 'when number of samples per pool < 5' do let(:spreadsheet_filename) { 'scRNA_bulk_submission_tube_invalid.csv' } it_behaves_like 'an invalid scRNA Bulk Submission', 'scRNA_bulk_submission_tube_invalid', 4 end - context 'number of samples per pool > 25' do + context 'when number of samples per pool > 25' do let(:spreadsheet_filename) { 'scRNA_bulk_submission_tube_invalid_greater.csv' } it_behaves_like 'an invalid scRNA Bulk Submission', 'scRNA_bulk_submission_tube_invalid_greater', 32 @@ -434,7 +434,7 @@ end end - context 'a submission with a NovaSeqX sequencing request type' do + context 'when creating a submission with a NovaSeqX sequencing request type' do let(:spreadsheet_filename) { 'novaseqx_bulk_submission.csv' } let!(:request_types) { [create(:nova_seq_x_sequencing_request_type)] } let(:study) { create(:study, name: 'UAT Study') } @@ -480,7 +480,7 @@ end end - context 'a submission with a NovaSeq 6000 PE sequencing request type' do + context 'when creating a submission with a NovaSeq 6000 PE sequencing request type' do let(:spreadsheet_filename) { 'nova_seq_6000_pe_bulk_submission.csv' } let!(:request_types) { [create(:nova_seq_6000_p_e_sequencing_request_type)] } let(:study) { create(:study, name: 'UAT Study') }