Merge branch 'nullify-if-blank' into 'master'

Add helper for clearing blank model attributes before validation

See merge request gitlab-org/gitlab!53681

app/models/clusters/platforms/kubernetes.rb

@@ -7,6 +7,7 @@ module Clusters
       include EnumWithNil
       include AfterCommitQueue
       include ReactiveCaching
+      include NullifyIfBlank
 
       RESERVED_NAMESPACES = %w(gitlab-managed-apps).freeze
 
@@ -25,7 +26,6 @@ module Clusters
         key: Settings.attr_encrypted_db_key_base_truncated,
         algorithm: 'aes-256-cbc'
 
-      before_validation :nullify_blank_namespace
       before_validation :enforce_namespace_to_lower_case
       before_validation :enforce_ca_whitespace_trimming
 
@@ -64,6 +64,8 @@ module Clusters
 
       default_value_for :authorization_type, :rbac
 
+      nullify_if_blank :namespace
+
       def predefined_variables(project:, environment_name:, kubernetes_namespace: nil)
         Gitlab::Ci::Variables::Collection.new.tap do |variables|
           variables.append(key: 'KUBE_URL', value: api_url)
@@ -255,10 +257,6 @@ module Clusters
         true
       end
 
-      def nullify_blank_namespace
-        self.namespace = nil if namespace.blank?
-      end
-
       def extract_relevant_pod_data(pods)
         pods.map do |pod|
           {

app/models/concerns/nullify_if_blank.rb

+# frozen_string_literal: true
+
+# Helper that sets attributes to nil prior to validation if they
+# are blank (are false, empty or contain only whitespace), to avoid
+# unnecessarily persisting empty strings.
+#
+# Model usage:
+#
+#   class User < ApplicationRecord
+#     include NullifyIfBlank
+#
+#     nullify_if_blank :name, :email
+#   end
+#
+#
+# Test usage:
+#
+#   RSpec.describe User do
+#     it { is_expected.to nullify_if_blank(:name) }
+#     it { is_expected.to nullify_if_blank(:email) }
+#   end
+#
+module NullifyIfBlank
+  extend ActiveSupport::Concern
+
+  class_methods do
+    def nullify_if_blank(*attributes)
+      self.attributes_to_nullify += attributes
+    end
+  end
+
+  included do
+    class_attribute :attributes_to_nullify,
+      instance_accessor: false,
+      instance_predicate: false,
+      default: Set.new
+
+    before_validation :nullify_blank_attributes
+  end
+
+  private
+
+  def nullify_blank_attributes
+    self.class.attributes_to_nullify.each do |attribute|
+      assign_attributes(attribute => nil) if read_attribute(attribute).blank?
+    end
+  end
+end

spec/models/clusters/platforms/kubernetes_spec.rb

@@ -17,6 +17,8 @@ RSpec.describe Clusters::Platforms::Kubernetes do
   it { is_expected.to delegate_method(:enabled?).to(:cluster) }
   it { is_expected.to delegate_method(:provided_by_user?).to(:cluster) }
 
+  it { is_expected.to nullify_if_blank(:namespace) }
+
   it_behaves_like 'having unique enum values'
 
   describe 'before_validation' do
@@ -29,14 +31,6 @@ RSpec.describe Clusters::Platforms::Kubernetes do
         expect(kubernetes.namespace).to eq('abc')
       end
     end
-
-    context 'when namespace is blank' do
-      let(:namespace) { '' }
-
-      it 'nullifies the namespace' do
-        expect(kubernetes.namespace).to be_nil
-      end
-    end
   end
 
   describe 'validation' do

spec/models/concerns/nullify_if_blank_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe NullifyIfBlank do
+  let_it_be(:model) do
+    Class.new(ApplicationRecord) do
+      include NullifyIfBlank
+
+      nullify_if_blank :name
+
+      self.table_name = 'users'
+    end
+  end
+
+  context 'attribute exists' do
+    let(:instance) { model.new(name: name) }
+
+    subject { instance.name }
+
+    before do
+      instance.validate
+    end
+
+    context 'attribute is blank' do
+      let(:name) { '' }
+
+      it { is_expected.to be_nil }
+    end
+
+    context 'attribute is nil' do
+      let(:name) { nil }
+
+      it { is_expected.to be_nil}
+    end
+
+    context 'attribute is not blank' do
+      let(:name) { 'name' }
+
+      it { is_expected.to eq('name') }
+    end
+  end
+
+  context 'attribute does not exist' do
+    before do
+      model.table_name = 'issues'
+    end
+
+    it { expect { model.new.valid? }.to raise_error(ActiveModel::UnknownAttributeError) }
+  end
+end

spec/support/matchers/nullify_if_blank_matcher.rb

+# frozen_string_literal: true
+
+RSpec::Matchers.define :nullify_if_blank do |attribute|
+  match do |record|
+    expect(record.class.attributes_to_nullify).to include(attribute)
+  end
+
+  failure_message do |record|
+    "expected nullify_if_blank configuration on #{record.class} to include #{attribute}"
+  end
+end