Remove unnecessary requires; cleanup test helper

johnnyshields · johnnyshields · commit 43b3f8c5432a · 2025-03-16T18:21:51.000+09:00
diff --git a/lib/ruby_saml/error_handling.rb b/lib/ruby_saml/error_handling.rb
@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-require "ruby_saml/validation_error"
+require 'ruby_saml/validation_error'
 
 module RubySaml
   module ErrorHandling
@@ -9,10 +9,10 @@ module ErrorHandling
     # Append the cause to the errors array, and based on the value of soft, return false or raise
     # an exception. soft_override is provided as a means of overriding the object's notion of
     # soft for just this invocation.
-    def append_error(error_msg, soft_override = nil)
+    def append_error(error_msg, soft_override = false) # rubocop:disable Style/OptionalBooleanParameter
       @errors << error_msg
 
-      unless soft_override.nil? ? soft : soft_override
+      unless soft_override || (respond_to?(:soft) && soft)
         raise ValidationError.new(error_msg)
       end
 
diff --git a/lib/ruby_saml/response.rb b/lib/ruby_saml/response.rb
@@ -16,11 +16,8 @@ class Response < SamlMessage
       'a' => RubySaml::XML::NS_ASSERTION
     }.freeze
 
-    # TODO: Settings should probably be initialized too... WDYT?
-
-    # RubySaml::Settings Toolkit settings
+    # TODO: This should not be an accessor
     attr_accessor :settings
-
     attr_reader :document
     attr_reader :decrypted_document
     attr_reader :response
diff --git a/test/attributes_test.rb b/test/attributes_test.rb
@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 
 require_relative 'test_helper'
-require 'ruby_saml/attributes'
 
 class AttributesTest < Minitest::Test
   describe 'Attributes' do
diff --git a/test/authrequest_test.rb b/test/authrequest_test.rb
@@ -1,8 +1,6 @@
 # frozen_string_literal: true
 
 require_relative 'test_helper'
-require 'ruby_saml/authrequest'
-require 'ruby_saml/setting_error'
 
 class AuthrequestTest < Minitest::Test
 
diff --git a/test/idp_metadata_parser_test.rb b/test/idp_metadata_parser_test.rb
@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 
 require_relative 'test_helper'
-require 'ruby_saml/idp_metadata_parser'
 
 class IdpMetadataParserTest < Minitest::Test
   class MockSuccessResponse < Net::HTTPSuccess
diff --git a/test/logging_test.rb b/test/logging_test.rb
@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 
 require_relative 'test_helper'
-require 'ruby_saml/logging'
 
 class LoggingTest < Minitest::Test
 
diff --git a/test/logoutrequest_test.rb b/test/logoutrequest_test.rb
@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 
 require_relative 'test_helper'
-require 'ruby_saml/logoutrequest'
 
 class RequestTest < Minitest::Test
 
diff --git a/test/logoutresponse_test.rb b/test/logoutresponse_test.rb
@@ -1,11 +1,9 @@
 # frozen_string_literal: true
 
 require_relative 'test_helper'
-require 'ruby_saml/logoutresponse'
 require 'logout_responses/logoutresponse_fixtures'
 
 class RubySamlTest < Minitest::Test
-
   describe "Logoutresponse" do
     let(:valid_logout_response_without_settings) { RubySaml::Logoutresponse.new(valid_logout_response_document) }
     let(:valid_logout_response) { RubySaml::Logoutresponse.new(valid_logout_response_document, settings) }
diff --git a/test/metadata_test.rb b/test/metadata_test.rb
@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 
 require_relative 'test_helper'
-require 'ruby_saml/metadata'
 
 class MetadataTest < Minitest::Test
 
diff --git a/test/onelogin_alias_test.rb b/test/onelogin_alias_test.rb
@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 
 require_relative 'test_helper'
-require 'ruby_saml/metadata'
 
 class OneloginAliasTest < Minitest::Test
 
diff --git a/test/response_test.rb b/test/response_test.rb
@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 
 require_relative 'test_helper'
-require 'ruby_saml/response'
 
 class RubySamlTest < Minitest::Test
 
@@ -58,8 +57,8 @@ class RubySamlTest < Minitest::Test
     let(:response_encrypted_nameid) { RubySaml::Response.new(response_document_encrypted_nameid) }
 
     def generate_audience_error(expected, actual)
-      s = actual.count > 1 ? 's' : '';
-      return "Invalid Audience#{s}. The audience#{s} #{actual.join(',')}, did not match the expected audience #{expected}"
+      s = actual.count > 1 ? 's' : ''
+      "Invalid Audience#{s}. The audience#{s} #{actual.join(',')}, did not match the expected audience #{expected}"
     end
 
     it "raise an exception when response is initialized with nil" do
diff --git a/test/settings_test.rb b/test/settings_test.rb
@@ -1,8 +1,6 @@
 # frozen_string_literal: true
 
 require_relative 'test_helper'
-require 'ruby_saml/settings'
-require 'ruby_saml/validation_error'
 
 class SettingsTest < Minitest::Test
 
diff --git a/test/slo_logoutrequest_test.rb b/test/slo_logoutrequest_test.rb
@@ -1,9 +1,6 @@
 # frozen_string_literal: true
 
 require_relative 'test_helper'
-require 'logout_responses/logoutresponse_fixtures'
-require 'ruby_saml/slo_logoutrequest'
-require 'timecop'
 
 class RubySamlTest < Minitest::Test
 
diff --git a/test/slo_logoutresponse_test.rb b/test/slo_logoutresponse_test.rb
@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 
 require_relative 'test_helper'
-require 'ruby_saml/slo_logoutresponse'
 
 class SloLogoutresponseTest < Minitest::Test
 
diff --git a/test/test_helper.rb b/test/test_helper.rb
@@ -384,16 +384,12 @@ def ruby_saml_key_text
     read_certificate("ruby-saml.key")
   end
 
-  #
-  # logoutresponse fixtures
-  #
+  # LogoutResponse fixtures
   def random_id
     "_#{RubySaml::Utils.generate_uuid}"
   end
 
-  #
-  # decodes a base64 encoded SAML response for use in SloLogoutresponse tests
-  #
+  # Decodes a base64 encoded SAML response for use in SloLogoutresponse tests
   def decode_saml_response_payload(unauth_url)
     payload = CGI.unescape(unauth_url.split("SAMLResponse=").last)
     decoded = Base64.decode64(payload)
@@ -405,9 +401,7 @@ def decode_saml_response_payload(unauth_url)
     inflated
   end
 
-  #
-  # decodes a base64 encoded SAML request for use in Logoutrequest tests
-  #
+  # Decodes a base64 encoded SAML request for use in Logoutrequest tests
   def decode_saml_request_payload(unauth_url)
     payload = CGI.unescape(unauth_url.split("SAMLRequest=").last)
     decoded = Base64.decode64(payload)
@@ -419,33 +413,11 @@ def decode_saml_request_payload(unauth_url)
     inflated
   end
 
-  SCHEMA_DIR = File.expand_path(File.join(__FILE__, '../../lib/ruby_saml/schemas'))
-
-  #
-  # validate an xml document against the given schema
-  #
+  # Validate an xml document against the given schema
   def validate_xml!(document, schema)
-    Dir.chdir(SCHEMA_DIR) do
-      xsd = if schema.is_a? Nokogiri::XML::Schema
-              schema
-            else
-              Nokogiri::XML::Schema(File.read(schema))
-            end
-
-      xml = if document.is_a? Nokogiri::XML::Document
-              document
-            else
-              Nokogiri::XML(document, &:strict)
-            end
-
-      result = xsd.validate(xml)
-
-      if result.length != 0
-        raise "Schema validation failed! XSD validation errors: #{result.join(", ")}"
-      else
-        true
-      end
-    end
+    result = load_schema(schema).validate(load_xml(document))
+    raise "Schema validation failed! XSD validation errors: #{result.join(", ")}" if result.length != 0
+    true
   end
 
   # Allows to emulate Azure AD request behavior
@@ -501,6 +473,23 @@ def encrypt_xml(assertion_xml, private_key)
 
     encrypted_assertion_xml
   end
+
+  private
+
+  def load_schema(schema)
+    return schema if schema.is_a?(Nokogiri::XML::Schema)
+
+    # Dir.chdir is necessary to load related schema files
+    Dir.chdir(File.expand_path('../lib/ruby_saml/schemas', __dir__)) do
+      Nokogiri::XML::Schema(File.read(schema))
+    end
+  end
+
+  def load_xml(document)
+    return document if document.is_a?(Nokogiri::XML::Document)
+
+    Nokogiri::XML(document, &:strict)
+  end
 end
 
 # Remove after https://github.com/jruby/jruby/issues/6613 is fixed
diff --git a/test/xml_security_shim_test.rb b/test/xml_security_shim_test.rb
@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 
 require_relative 'test_helper'
-require 'xml_security'
 
 class XMLSecurityTest < Minitest::Test
   def test_base_document_inherits_from_rexml_document
diff --git a/test/xml_test.rb b/test/xml_test.rb
@@ -1,7 +1,6 @@
 # frozen_string_literal: true
 
 require_relative 'test_helper'
-require 'ruby_saml/xml'
 
 class XmlTest < Minitest::Test
 
