Rename confusing methods in LabelSetValidator

LabelSetValidator has methods `valid?` and `validate`. It's not very clear
what these do, what's the difference between them, and the name `valid?`
would imply (by convention) that it returns a Boolean, rather than raising
an exception on invalid input.

Renamed these to `validate_symbols!` and `validate_labelset!`, to make it
clearer what each of them do. The `!` also follows the usual Ruby
convention of "do X, and if that doesn't work, Raise"

This commit also starts drawing the distinction between an array of `labels`,
and a hash of label keys and values (which we call a `labelset`). The term
`labels` is used interchangeably for both concepts throughout the code.
This commit doesn't fix all instances, but it's a step in that direction.

Signed-off-by: Daniel Magliola <[email protected]>

lib/prometheus/client/label_set_validator.rb

@@ -21,7 +21,7 @@ def initialize(expected_labels:, reserved_labels: [])
         @validated = {}
       end
 
-      def valid?(labels)
+      def validate_symbols!(labels)
         unless labels.respond_to?(:all?)
           raise InvalidLabelSetError, "#{labels} is not a valid label set"
         end
@@ -33,24 +33,24 @@ def valid?(labels)
         end
       end
 
-      def validate(labels)
-        return labels if @validated.key?(labels.hash)
+      def validate_labelset!(labelset)
+        return labelset if @validated.key?(labelset.hash)
 
-        valid?(labels)
+        validate_symbols!(labelset)
 
-        unless keys_match?(labels)
+        unless keys_match?(labelset)
           raise InvalidLabelSetError, "labels must have the same signature " \
-                                      "(keys given: #{labels.keys.sort} vs." \
+                                      "(keys given: #{labelset.keys.sort} vs." \
                                       " keys expected: #{expected_labels}"
         end
 
-        @validated[labels.hash] = labels
+        @validated[labelset.hash] = labelset
       end
 
       private
 
-      def keys_match?(labels)
-        labels.keys.sort == expected_labels
+      def keys_match?(labelset)
+        labelset.keys.sort == expected_labels
       end
 
       def validate_symbol(key)

lib/prometheus/client/metric.rb

@@ -19,8 +19,8 @@ def initialize(name,
         validate_docstring(docstring)
         @validator = LabelSetValidator.new(expected_labels: labels,
                                            reserved_labels: reserved_labels)
-        @validator.valid?(labels)
-        @validator.valid?(preset_labels)
+        @validator.validate_symbols!(labels)
+        @validator.validate_symbols!(preset_labels)
 
         @labels = labels
         @store_settings = store_settings
@@ -36,7 +36,7 @@ def initialize(name,
         )
 
         if preset_labels.keys.length == labels.length
-          @validator.validate(preset_labels)
+          @validator.validate_labelset!(preset_labels)
           @all_labels_preset = true
         end
       end
@@ -85,7 +85,7 @@ def validate_docstring(docstring)
       def label_set_for(labels)
         # We've already validated, and there's nothing to merge. Save some cycles
         return preset_labels if @all_labels_preset && labels.empty?
-        @validator.validate(preset_labels.merge(labels))
+        @validator.validate_labelset!(preset_labels.merge(labels))
       end
     end
   end

spec/prometheus/client/label_set_validator_spec.rb

@@ -13,67 +13,67 @@
     end
   end
 
-  describe '#valid?' do
+  describe '#validate_symbols!' do
     it 'returns true for a valid label check' do
-      expect(validator.valid?(version: 'alpha')).to eql(true)
+      expect(validator.validate_symbols!(version: 'alpha')).to eql(true)
     end
 
     it 'raises Invaliddescribed_classError if a label set is not a hash' do
       expect do
-        validator.valid?('invalid')
+        validator.validate_symbols!('invalid')
       end.to raise_exception invalid
     end
 
     it 'raises InvalidLabelError if a label key is not a symbol' do
       expect do
-        validator.valid?('key' => 'value')
+        validator.validate_symbols!('key' => 'value')
       end.to raise_exception(described_class::InvalidLabelError)
     end
 
     it 'raises InvalidLabelError if a label key starts with __' do
       expect do
-        validator.valid?(__reserved__: 'key')
+        validator.validate_symbols!(__reserved__: 'key')
       end.to raise_exception(described_class::ReservedLabelError)
     end
 
     it 'raises ReservedLabelError if a label key is reserved' do
       [:job, :instance].each do |label|
         expect do
-          validator.valid?(label => 'value')
+          validator.validate_symbols!(label => 'value')
         end.to raise_exception(described_class::ReservedLabelError)
       end
     end
   end
 
-  describe '#validate' do
+  describe '#validate_labelset!' do
     let(:expected_labels) { [:method, :code] }
 
     it 'returns a given valid label set' do
       hash = { method: 'get', code: '200' }
 
-      expect(validator.validate(hash)).to eql(hash)
+      expect(validator.validate_labelset!(hash)).to eql(hash)
     end
 
-    it 'raises an exception if a given label set is not `valid?`' do
+    it 'raises an exception if a given label set is not `validate_symbols!`' do
       input = 'broken'
-      expect(validator).to receive(:valid?).with(input).and_raise(invalid)
+      expect(validator).to receive(:validate_symbols!).with(input).and_raise(invalid)
 
-      expect { validator.validate(input) }.to raise_exception(invalid)
+      expect { validator.validate_labelset!(input) }.to raise_exception(invalid)
     end
 
     it 'raises an exception if there are unexpected labels' do
       expect do
-        validator.validate(method: 'get', code: '200', exception: 'NoMethodError')
+        validator.validate_labelset!(method: 'get', code: '200', exception: 'NoMethodError')
       end.to raise_exception(invalid, /keys given: \[:code, :exception, :method\] vs. keys expected: \[:code, :method\]/)
     end
 
     it 'raises an exception if there are missing labels' do
       expect do
-        validator.validate(method: 'get')
+        validator.validate_labelset!(method: 'get')
       end.to raise_exception(invalid, /keys given: \[:method\] vs. keys expected: \[:code, :method\]/)
 
       expect do
-        validator.validate(code: '200')
+        validator.validate_labelset!(code: '200')
       end.to raise_exception(invalid, /keys given: \[:code\] vs. keys expected: \[:code, :method\]/)
     end
   end