From 6aa69bdaeae33effec0acebceaf05f0944c68d77 Mon Sep 17 00:00:00 2001 From: Daniel Waterworth Date: Wed, 22 Nov 2023 12:00:42 -0600 Subject: [PATCH] DEV: Allow setting different custom field length limits by key (#24505) --- app/models/concerns/has_custom_fields.rb | 86 +++++++++++++--------- spec/lib/concern/has_custom_fields_spec.rb | 10 +++ 2 files changed, 60 insertions(+), 36 deletions(-) diff --git a/app/models/concerns/has_custom_fields.rb b/app/models/concerns/has_custom_fields.rb index 57512febb09..3662c4534f6 100644 --- a/app/models/concerns/has_custom_fields.rb +++ b/app/models/concerns/has_custom_fields.rb @@ -4,39 +4,51 @@ module HasCustomFields extend ActiveSupport::Concern module Helpers - def self.append_field(target, key, value, types) + CUSTOM_FIELD_TRUE ||= %w[1 t true T True TRUE].freeze + end + + class FieldDescriptor < Struct.new(:type, :max_length) + def append_field(target, key, value) if target.has_key?(key) target[key] = [target[key]] if !target[key].is_a? Array - target[key] << cast_custom_field(key, value, types, _return_array = false) + target[key] << deserialize(key, value, false) else - target[key] = cast_custom_field(key, value, types) + target[key] = deserialize(key, value, true) end end - CUSTOM_FIELD_TRUE ||= %w[1 t true T True TRUE].freeze + def validate(obj, name, value) + return if value.nil? - def self.get_custom_field_type(types, key) - return unless types + size = + if Array === type || (type != :json && Array === value) + value.map { |v| serialize(v).bytesize }.max || 0 + else + serialize(value).bytesize + end - types[key] + if size > max_length + obj.errors.add( + :base, + I18n.t("custom_fields.validations.max_value_length", max_value_length: max_length), + ) + end end - def self.serialize(value, type) - if value.is_a?(Hash) || type == :json + def serialize(value) + if value.is_a?(Hash) || type == :json || (Array === type && type[0] == :json) value.to_json elsif TrueClass === value "t" elsif FalseClass === value "f" - elsif Integer === value - value.to_s else - value + value.to_s end end - def self.cast_custom_field(key, value, types, return_array = true) - return value unless type = get_custom_field_type(types, key) + def deserialize(key, value, return_array) + return value unless type = self.type array = nil @@ -48,7 +60,7 @@ module HasCustomFields result = case type when :boolean - !!CUSTOM_FIELD_TRUE.include?(value) + !!Helpers::CUSTOM_FIELD_TRUE.include?(value) when :integer value.to_i when :json @@ -60,7 +72,9 @@ module HasCustomFields array ? [result] : result end - def self.parse_json_value(value, key) + private + + def parse_json_value(value, key) ::JSON.parse(value) rescue JSON::ParserError Rails.logger.warn( @@ -70,8 +84,8 @@ module HasCustomFields end end + DEFAULT_FIELD_DESCRIPTOR = FieldDescriptor.new(:string, 10_000_000) CUSTOM_FIELDS_MAX_ITEMS = 100 - CUSTOM_FIELDS_MAX_VALUE_LENGTH = 10_000_000 module ClassMethods # To avoid n+1 queries, use this function to retrieve lots of custom fields in one go @@ -97,10 +111,10 @@ module HasCustomFields end def append_custom_field(target, key, value) - HasCustomFields::Helpers.append_field(target, key, value, @custom_field_types) + get_custom_field_descriptor(key).append_field(target, key, value) end - def register_custom_field_type(name, type) + def register_custom_field_type(name, type, max_length: DEFAULT_FIELD_DESCRIPTOR.max_length) if Array === type Discourse.deprecate( "Array types for custom fields are deprecated, use type :json instead", @@ -108,13 +122,11 @@ module HasCustomFields ) end - @custom_field_types ||= {} - @custom_field_types[name] = type + custom_field_meta_data[name] = FieldDescriptor.new(type, max_length) end - def get_custom_field_type(name) - @custom_field_types ||= {} - @custom_field_types[name] || :string + def get_custom_field_descriptor(name) + custom_field_meta_data[name] || DEFAULT_FIELD_DESCRIPTOR end def preload_custom_fields(objects, fields) @@ -142,10 +154,16 @@ module HasCustomFields preloaded.delete(name) if preloaded[name].nil? - HasCustomFields::Helpers.append_field(preloaded, name, value, @custom_field_types) + get_custom_field_descriptor(name).append_field(preloaded, name, value) end end end + + private + + def custom_field_meta_data + @custom_field_meta_data ||= {} + end end included do @@ -257,14 +275,15 @@ module HasCustomFields (dup.keys.to_set + fields_by_key.keys.to_set).each do |key| fields = fields_by_key[key] || [] value = dup[key] - field_type = self.class.get_custom_field_type(key) + descriptor = self.class.get_custom_field_descriptor(key) + field_type = descriptor.type if Array === field_type || (field_type != :json && Array === value) value = value || [] value.compact! sub_type = field_type[0] - value.map! { |v| HasCustomFields::Helpers.serialize(v, sub_type) } + value.map! { |v| descriptor.serialize(v) } unless value == fields.map(&:value) fields.each(&:destroy!) @@ -275,7 +294,7 @@ module HasCustomFields if value.nil? fields.each(&:destroy!) else - value = HasCustomFields::Helpers.serialize(value, field_type) + value = descriptor.serialize(value) field = fields.find { |f| f.value == value } fields.select { |f| f != field }.each(&:destroy!) @@ -329,13 +348,8 @@ module HasCustomFields end def custom_fields_value_length - return if custom_fields.values.all? { _1.to_s.size <= CUSTOM_FIELDS_MAX_VALUE_LENGTH } - errors.add( - :base, - I18n.t( - "custom_fields.validations.max_value_length", - max_value_length: CUSTOM_FIELDS_MAX_VALUE_LENGTH, - ), - ) + custom_fields.each do |name, value| + self.class.get_custom_field_descriptor(name).validate(self, name, value) + end end end diff --git a/spec/lib/concern/has_custom_fields_spec.rb b/spec/lib/concern/has_custom_fields_spec.rb index 0557193c61f..0ebba7bbda1 100644 --- a/spec/lib/concern/has_custom_fields_spec.rb +++ b/spec/lib/concern/has_custom_fields_spec.rb @@ -365,6 +365,16 @@ RSpec.describe HasCustomFields do end end + it "supports setting a maximum length" do + CustomFieldsTestItem.register_custom_field_type "foo", :string, max_length: 1 + test_item = CustomFieldsTestItem.new + test_item.custom_fields = { "foo" => "a" } + test_item.save! + + test_item.custom_fields = { "foo" => "aa" } + expect { test_item.save! }.to raise_error(ActiveRecord::RecordInvalid) + end + describe "upsert_custom_fields" do it "upserts records" do test_item = CustomFieldsTestItem.create