fix: observe the server version once

The reasons list read the version for its comparison and again for its message, and the doctor read it a third time for the passing text. A transient failure on a later read would have replaced an already determined result with a generic verification error, and every healthy check paid extra round trips. One observed version now decides both status and message.

Author: cardmagic

lib/solid_objects/database_adapter.rb

@@ -47,12 +47,15 @@ def server_version
       end
     end
 
-    # @rbs () -> Array[String]
-    def unsupported_server_reasons
+    # One observed version decides both the status and the message. Reading it
+    # again could let a transient failure replace an already determined result.
+    # @rbs (?Gem::Version?) -> Array[String]
+    def unsupported_server_reasons(observed = nil)
+      observed ||= server_version
       reasons = []
       minimum = minimum_server_version
-      if minimum && server_version < minimum
-        reasons << "#{self.class.name.demodulize} #{server_version} is older than " \
+      if minimum && observed < minimum
+        reasons << "#{self.class.name.demodulize} #{observed} is older than " \
           "Solid Objects requires, which is #{minimum}"
       end
       reasons.concat(additional_server_reasons)

lib/solid_objects/doctor.rb

@@ -195,12 +195,13 @@ def check_authorization
     # @rbs () -> Check
     def check_database_server
       adapter = SolidObjects.database_adapter
-      reasons = adapter.unsupported_server_reasons
+      observed = adapter.server_version
+      reasons = adapter.unsupported_server_reasons(observed)
       return warn_check(:database_server, reasons.join("; ")) unless reasons.empty?
 
       pass(
         :database_server,
-        "#{adapter.class.name.demodulize} #{adapter.server_version} meets the tested minimum"
+        "#{adapter.class.name.demodulize} #{observed} meets the tested minimum"
       )
     rescue => error
       warn_check(:database_server, "#{error.class}: #{error.message}")

sig/generated/lib/solid_objects/database_adapter.rbs

@@ -25,8 +25,10 @@ module SolidObjects
     # @rbs () -> Gem::Version
     def server_version: () -> Gem::Version
 
-    # @rbs () -> Array[String]
-    def unsupported_server_reasons: () -> Array[String]
+    # One observed version decides both the status and the message. Reading it
+    # again could let a transient failure replace an already determined result.
+    # @rbs (?Gem::Version?) -> Array[String]
+    def unsupported_server_reasons: (?Gem::Version?) -> Array[String]
 
     # @rbs () -> Array[String]
     def additional_server_reasons: () -> Array[String]

test/integration/database_version_test.rb

@@ -25,6 +25,22 @@ class DatabaseVersionTest < ActiveSupport::TestCase
     restore(adapter, :with_connection)
   end
 
+  test "the observed version is read once for status and message" do
+    adapter = SolidObjects.database_adapter
+    reads = 0
+    real = adapter.method(:server_version)
+    adapter.define_singleton_method(:server_version) {
+      reads += 1
+      real.call
+    }
+
+    SolidObjects::Doctor.new.call.check(:database_server)
+
+    assert_equal 1, reads, "the doctor should observe the server version once"
+  ensure
+    restore(adapter, :server_version)
+  end
+
   test "a supported server passes verification" do
     assert_empty SolidObjects.database_adapter.unsupported_server_reasons
   end
@@ -60,7 +76,7 @@ class DatabaseVersionTest < ActiveSupport::TestCase
 
   test "the doctor warns about an unsupported server" do
     adapter = SolidObjects.database_adapter
-    adapter.define_singleton_method(:unsupported_server_reasons) { [ "too old" ] }
+    adapter.define_singleton_method(:unsupported_server_reasons) { |_observed = nil| [ "too old" ] }
 
     check = SolidObjects::Doctor.new.call.check(:database_server)
 
@@ -72,7 +88,7 @@ class DatabaseVersionTest < ActiveSupport::TestCase
 
   test "an unsupported server does not fail the report" do
     adapter = SolidObjects.database_adapter
-    adapter.define_singleton_method(:unsupported_server_reasons) { [ "too old" ] }
+    adapter.define_singleton_method(:unsupported_server_reasons) { |_observed = nil| [ "too old" ] }
 
     assert SolidObjects::Doctor.new.call.healthy?
   ensure