Turn server info status into a typed struct

app/models/reservation.rb

@@ -303,6 +303,7 @@ def logs_tf_url
     "http://logs.tf/search/log?s=#{SITE_HOST}+%23#{id}"
   end
 
+  sig { params(server_info: ServerInfo::Status).returns(T.nilable(T.any(String, ActiveSupport::Multibyte::Chars))) }
   def save_sdr_info(server_info)
     return if server_info.ip.nil?
 
@@ -314,12 +315,12 @@ def save_sdr_info(server_info)
     update_columns(
       sdr_ip: server_info.ip,
       sdr_port: server_info.port,
-      sdr_tv_port: server_info.port + 1
+      sdr_tv_port: server_info.port.to_i + 1
     )
     server&.update_columns(
       last_sdr_ip: server_info.ip,
       last_sdr_port: server_info.port,
-      last_sdr_tv_port: server_info.port + 1
+      last_sdr_tv_port: server_info.port.to_i + 1
     )
 
     broadcast_connect_info

app/models/server_info.rb

@@ -19,64 +19,74 @@ def auth
     server.rcon_auth
   end
 
-  sig { returns(String) }
+  class Status < T::Struct
+    prop :version, T.nilable(Integer)
+    prop :ip, T.nilable(String)
+    prop :port, T.nilable(Integer)
+    prop :server_name, T.nilable(String), default: 'unknown'
+    prop :map_name, T.nilable(String), default: 'unknown'
+    prop :number_of_players, T.nilable(Integer)
+    prop :max_players, Integer, default: 0
+  end
+
+  sig { returns(T.nilable(String)) }
   def server_name
-    status.fetch(:server_name, 'unknown').to_s
+    status.server_name
   end
 
   sig { returns(T.nilable(Integer)) }
   def version
-    status.fetch(:version, nil)
+    status.version
   end
 
   sig { returns(T.nilable(String)) }
   def ip
-    status.fetch(:ip, nil)
+    status.ip
   end
 
   sig { returns(T.nilable(Integer)) }
   def port
-    status.fetch(:port, nil)
+    status.port
   end
 
   sig { returns(T.nilable(Integer)) }
   def number_of_players
-    status.fetch(:number_of_players,  nil)
+    status.number_of_players
   end
 
   sig { returns(Integer) }
   def max_players
-    status.fetch(:max_players,        0)
+    status.max_players
   end
 
-  sig { returns(String) }
+  sig { returns(T.nilable(String)) }
   def map_name
-    status.fetch(:map_name,           'unknown')
+    status.map_name
   end
 
-  sig { returns(Hash) }
+  sig { returns(Status) }
   def status
     Rails.cache.fetch "server_info_#{server.id}", expires_in: 1.minute do
-      out = {}
+      out = Status.new
       fetch_rcon_status.lines.each do |line|
         case line
         when /^version\s+:\s+(\d+)/
-          out[:version] ||= Regexp.last_match(1)&.to_i
+          out.version ||= Regexp.last_match(1)&.to_i
         when %r{^udp/ip\s+:\s+(\d+\.\d+\.\d+\.\d+):(\d+)}
-          out[:ip] ||= Regexp.last_match(1)
-          out[:port] ||= Regexp.last_match(2)&.to_i
+          out.ip ||= Regexp.last_match(1)
+          out.port ||= Regexp.last_match(2)&.to_i
         when /^hostname\W+(.*)$/
-          out[:server_name] ||= Regexp.last_match(1)
+          out.server_name = Regexp.last_match(1) if out.server_name == 'unknown'
         when /^map\W+(\S+)/
-          out[:map_name] ||= Regexp.last_match(1)
+          out.map_name = Regexp.last_match(1) if out.map_name == 'unknown'
         when /^players\W+(\S+).+\((\d+)/
-          out[:number_of_players] ||= Regexp.last_match(1).to_i
-          out[:max_players] ||= Regexp.last_match(2).to_i
+          out.number_of_players ||= Regexp.last_match(1).to_i
+          out.max_players = Regexp.last_match(2).to_i if out.max_players.zero?
         end
       end
       out
     rescue SteamCondenser::Error, Errno::ECONNREFUSED
-      {}
+      out
     end
   end
 

spec/models/server_info_spec.rb

@@ -11,55 +11,35 @@
 
   context 'statistics available without rcon' do
     before do
-      status = {  server_name: 'Server name',
-                  number_of_players: 10,
-                  max_players: 20,
-                  map_name: 'cp_badlands' }
+      status = ServerInfo::Status.new(server_name: 'Server name',
+                                      number_of_players: 10,
+                                      max_players: 20,
+                                      map_name: 'cp_badlands')
       subject.stub(status: status)
     end
 
     describe '#server_name' do
       it 'gets the server_name from the status hash' do
         subject.server_name.should eql 'Server name'
       end
-
-      it 'returns unknown if it cant get the server_name from the hash' do
-        subject.status.delete_if { |key| key == :server_name }
-        subject.server_name.should eql 'unknown'
-      end
     end
 
     describe '#number_of_players' do
       it 'gets the number_of_players from the status hash' do
         subject.number_of_players.should eql 10
       end
-
-      it 'returns nil if it cant get the number_of_players from the hash' do
-        subject.status.delete_if { |key| key == :number_of_players }
-        subject.number_of_players.should eql nil
-      end
     end
 
     describe '#max_players' do
       it 'gets the max_players from the status hash' do
         subject.max_players.should eql 20
       end
-
-      it 'returns 0 if it cant get the max_players from the hash' do
-        subject.status.delete_if { |key| key == :max_players }
-        subject.max_players.should eql 0
-      end
     end
 
     describe '#map_name' do
       it 'gets the map_name from the status hash' do
         subject.map_name.should eql 'cp_badlands'
       end
-
-      it 'returns unknown if it cant get the map_name from the hash' do
-        subject.status.delete_if { |key| key == :map_name }
-        subject.map_name.should eql 'unknown'
-      end
     end
   end
 
@@ -183,10 +163,10 @@
       subject.version.should eql 3032525
     end
 
-    it 'returns an empty hash if something went wrong' do
+    it 'returns an empty struct if something went wrong' do
       subject.stub(:fetch_rcon_status).and_raise(SteamCondenser::Error.new('BOOM'))
 
-      expect(subject.status).to eql({})
+      expect(subject.status.serialize).to eql(ServerInfo::Status.new.serialize)
     end
   end
 

spec/services/server_metric_spec.rb

@@ -42,7 +42,7 @@
 
   let(:reservation) { create :reservation }
   let(:server) { double :server, id: reservation.server_id, current_reservation: reservation, condenser: double }
-  let(:server_info_hash) { { number_of_players: 1, map_name: 'cp_granlands' } }
+  let(:server_info_hash) { ServerInfo::Status.new(number_of_players: 1, map_name: 'cp_granlands') }
   let(:server_info) { ServerInfo.new(server) }
 
   before do
@@ -57,12 +57,12 @@
     expect(PlayerStatistic.count).to eql 2
     expect(ServerStatistic.count).to eql 1
     server_statistic = ServerStatistic.last
-    server_statistic.cpu_usage.should == 25
-    server_statistic.fps.should == 67
+    expect(server_statistic.cpu_usage).to eql 25
+    expect(server_statistic.fps).to eql 67
 
     player_statistic = PlayerStatistic.last
-    player_statistic.ping.should == 76
-    player_statistic.loss.should == 1
-    player_statistic.reservation_player.ip.should == '1.128.0.2'
+    expect(player_statistic.ping).to eql 76
+    expect(player_statistic.loss).to eql 1
+    expect(player_statistic.reservation_player.ip).to eql '1.128.0.2'
   end
 end