diff --git a/CHANGELOG.md b/CHANGELOG.md index 981c09407..5decf05e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,8 @@ _None_ ### New Features -- Add the possibility to configure in DerivedBuildCodeFormatter a versioning prefix instead of always defaulting to 1. [#656] +- Add the possibility to configure in `DerivedBuildCodeFormatter` a versioning prefix instead of always defaulting to 1. [#656] +- Add configurable number of digits in version components in `DerivedBuildCodeFormatter`. [#657] ### Bug Fixes diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/models/app_version.rb b/lib/fastlane/plugin/wpmreleasetoolkit/models/app_version.rb index d81dd076f..5b65bb0e3 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/models/app_version.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/models/app_version.rb @@ -33,6 +33,14 @@ def initialize(major, minor, patch = 0, build_number = 0) def to_s "#{@major}.#{@minor}.#{@patch}.#{@build_number}" end + + # Returns an array of the version components. + # + # @return [Array] an array of the version components. + # + def components + [@major, @minor, @patch, @build_number] + end end end end diff --git a/lib/fastlane/plugin/wpmreleasetoolkit/versioning/formatters/derived_build_code_formatter.rb b/lib/fastlane/plugin/wpmreleasetoolkit/versioning/formatters/derived_build_code_formatter.rb index 4ddc536ce..c1efb9415 100644 --- a/lib/fastlane/plugin/wpmreleasetoolkit/versioning/formatters/derived_build_code_formatter.rb +++ b/lib/fastlane/plugin/wpmreleasetoolkit/versioning/formatters/derived_build_code_formatter.rb @@ -3,23 +3,35 @@ module Fastlane module Wpmreleasetoolkit module Versioning + # Max total for `*_digits` params, not counting prefix + MAX_TOTAL_DIGITS = 8 + MIN_DIGIT_COUNT = 1 + MAX_DIGIT_COUNT = 3 + # The `DerivedBuildCodeFormatter` class is a specialized build code formatter for derived build codes. # It takes in an AppVersion object and derives a build code from it. class DerivedBuildCodeFormatter - # Initialize the formatter with a configurable prefix. + # Initialize the formatter with configurable prefix and digit counts. # # @param [String] prefix The prefix to use for the build code. Must be a single digit (0-9), or empty string / nil. + # @param [Integer] major_digits Number of digits for major version. Must be between 1–3. Defaults to 2. + # @param [Integer] minor_digits Number of digits for minor version. Must be between 1–3. Defaults to 2. + # @param [Integer] patch_digits Number of digits for patch version. Must be between 1–3. Defaults to 2. + # @param [Integer] build_digits Number of digits for build number. Must be between 1–3. Defaults to 2. # - def initialize(prefix: nil) - prefix ||= '' + def initialize(prefix: nil, major_digits: 2, minor_digits: 2, patch_digits: 2, build_digits: 2) validate_prefix!(prefix) @prefix = prefix.to_s + + @digit_counts = [major_digits, minor_digits, patch_digits, build_digits] + @digit_counts.each { |d| validate_digit_count!(d) } + validate_total_digits!(@digit_counts) end # Calculate the next derived build code. # # This method derives a new build code from the given AppVersion object by concatenating the configured prefix, - # the major version, the minor version, the patch version, and the build number. + # the major version, the minor version, the patch version, and the build number with configurable digit counts. # # @param [AppVersion] version The AppVersion object to derive the next build code from. # @@ -28,19 +40,16 @@ def initialize(prefix: nil) # # @return [String] The formatted build code string. # - def build_code(build_code = nil, version:) - result = format( - # The prefix is configurable to allow for additional platforms or - # extensions that could use a different digit prefix such as 2, etc. - '%s%.2i%.2i%.2i%.2i', - prefix: @prefix, - major: version.major, - minor: version.minor, - patch: version.patch, - build_number: version.build_number - ) - - result.gsub(/^0+/, '') + def build_code(_build_code = nil, version:) + formatted_components = version.components.zip(@digit_counts).map do |value, width| + comp = value.to_s.rjust(width, '0') + if comp.length > width + UI.user_error!("Version component value (#{value}) exceeds maximum allowed width of #{width} characters. " \ + "Consider increasing the corresponding `*_digits` parameter of your `#{self.class.name}`") + end + comp + end + [@prefix, *formatted_components].join.gsub(/^0+/, '') end private @@ -52,6 +61,10 @@ def build_code(build_code = nil, version:) # @raise [StandardError] If the prefix is invalid # def validate_prefix!(prefix) + unless prefix.nil? || prefix.is_a?(String) || prefix.is_a?(Integer) + UI.user_error!("Prefix must be a string or integer, got: #{prefix.class}") + end + prefix_str = prefix.to_s # Allow empty string @@ -67,6 +80,37 @@ def validate_prefix!(prefix) UI.user_error!("Prefix must be an integer digit (0-9) or empty string, got: '#{prefix_str}'") end + + # Validates that the digit count is a valid positive integer within reasonable limits. + # + # @param [Integer] digit_count The digit count to validate + # + # @raise [StandardError] If the digit count is invalid + # + def validate_digit_count!(digit_count) + # Check if it's an integer + unless digit_count.is_a?(Integer) + UI.user_error!("Digit count must be an integer, got: #{digit_count.class}") + end + + return if digit_count.between?(MIN_DIGIT_COUNT, MAX_DIGIT_COUNT) + + UI.user_error!("Digit count must be between #{MIN_DIGIT_COUNT} and #{MAX_DIGIT_COUNT} digits, got: #{digit_count}") + end + + # Validates that the total number of digits (excluding prefix) doesn't exceed the maximum for multiplatform compatibility. + # + # Since Google Play's max versionCode is ≈ 2_000_000_000, we want to avoid being too close to the limit + # as this would then block us from submitting any updates for that app if we reached it. + def validate_total_digits!(digits_list) + total_digits = digits_list.sum + + # Limit total digits to 8 (excluding prefix) + return if total_digits <= MAX_TOTAL_DIGITS + + UI.user_error!("Total digit count (#{total_digits}) exceeds maximum allowed (#{MAX_TOTAL_DIGITS}). " \ + "Current config: major(#{digits_list[0]}) + minor(#{digits_list[1]}) + patch(#{digits_list[2]}) + build(#{digits_list[3]}) digits") + end end end end diff --git a/spec/derived_build_code_formatter_spec.rb b/spec/derived_build_code_formatter_spec.rb index c49f1b5e1..547e0e20c 100644 --- a/spec/derived_build_code_formatter_spec.rb +++ b/spec/derived_build_code_formatter_spec.rb @@ -19,13 +19,6 @@ end context 'with explicit prefix' do - it 'derives the build code with prefix "1"' do - version = Fastlane::Models::AppVersion.new(1, 2, 3, 4) - formatter = described_class.new(prefix: '1') - build_code_string = formatter.build_code(version: version) - expect(build_code_string.to_s).to eq('101020304') - end - it 'derives the build code with prefix "2"' do version = Fastlane::Models::AppVersion.new(1, 2, 3, 4) formatter = described_class.new(prefix: '2') @@ -33,7 +26,7 @@ expect(build_code_string.to_s).to eq('201020304') end - it 'derives the build code with prefix "0"' do + it 'derives the build code with prefix "0" and trims leading zeros' do version = Fastlane::Models::AppVersion.new(12, 34, 56, 78) formatter = described_class.new(prefix: '0') build_code_string = formatter.build_code(version: version) @@ -81,6 +74,125 @@ end end + describe 'configurable digit counts' do + context 'with custom digit counts' do + it 'uses 1 digit for each component' do + version = Fastlane::Models::AppVersion.new(1, 2, 3, 4) + formatter = described_class.new(prefix: '1', major_digits: 1, minor_digits: 1, patch_digits: 1, build_digits: 1) + build_code_string = formatter.build_code(version: version) + expect(build_code_string.to_s).to eq('11234') + end + + it 'uses 2 digits for each component and pads with zeros' do + # Test both large numbers and zero-padding in one test + version_large = Fastlane::Models::AppVersion.new(12, 34, 56, 78) + formatter = described_class.new(prefix: '2', major_digits: 2, minor_digits: 2, patch_digits: 2, build_digits: 2) + expect(formatter.build_code(version: version_large).to_s).to eq('212345678') + + # Test zero-padding with smaller numbers + version_small = Fastlane::Models::AppVersion.new(1, 2, 3, 4) + expect(formatter.build_code(version: version_small).to_s).to eq('201020304') + end + + it 'uses mixed digit counts' do + version = Fastlane::Models::AppVersion.new(1, 23, 45, 678) + formatter = described_class.new(prefix: '', major_digits: 1, minor_digits: 2, patch_digits: 2, build_digits: 3) + build_code_string = formatter.build_code(version: version) + # 1(1 digit) + 23(2 digits) + 45(2 digits) + 678(3 digits) = "12345678" + expect(build_code_string.to_s).to eq('12345678') + end + + it 'handles maximum values within digit limits' do + version = Fastlane::Models::AppVersion.new(99, 99, 99, 99) + formatter = described_class.new(prefix: '1', major_digits: 2, minor_digits: 2, patch_digits: 2, build_digits: 2) + build_code_string = formatter.build_code(version: version) + expect(build_code_string.to_s).to eq('199999999') + end + end + + context 'with empty prefix and custom digits' do + it 'trims leading zeros correctly with 1-digit major' do + version = Fastlane::Models::AppVersion.new(5, 12, 34, 56) + formatter = described_class.new(prefix: '', major_digits: 1, minor_digits: 2, patch_digits: 2, build_digits: 2) + build_code_string = formatter.build_code(version: version) + expect(build_code_string.to_s).to eq('5123456') + end + + it 'trims leading zeros correctly with larger number of major digits' do + version = Fastlane::Models::AppVersion.new(7, 8, 9, 10) + formatter = described_class.new(prefix: '', major_digits: 2, minor_digits: 2, patch_digits: 2, build_digits: 2) + build_code_string = formatter.build_code(version: version) + expect(build_code_string.to_s).to eq('7080910') + end + + it 'handles edge case where all components start with zeros' do + version = Fastlane::Models::AppVersion.new(0, 1, 2, 3) + formatter = described_class.new(prefix: '', major_digits: 2, minor_digits: 2, patch_digits: 2, build_digits: 2) + build_code_string = formatter.build_code(version: version) + # ''(empty prefix) + '00'(2 digits) + '01'(2 digits) + '02'(2 digits) + '03'(2 digits) = "00010203", trimmed to "10203" + expect(build_code_string.to_s).to eq('10203') + end + end + + context 'with backward compatibility (default 2 digits)' do + it 'maintains existing behavior when no digit parameters specified' do + version = Fastlane::Models::AppVersion.new(1, 2, 3, 4) + formatter_old = described_class.new(prefix: '1') + formatter_new = described_class.new(prefix: '1', major_digits: 2, minor_digits: 2, patch_digits: 2, build_digits: 2) + + expect(formatter_old.build_code(version: version)).to eq(formatter_new.build_code(version: version)) + end + end + end + + describe 'digit count validation' do + # Generate all combinations of [1,2,3] for each of the 4 digit count parameters + all_digit_combinations = Array.new(4, [1, 2, 3]).reduce(&:product).map(&:flatten) + + context 'with valid digit counts' do + it 'accepts digit counts from 1 to 3 individually' do + (1..3).each do |count| + # Test each parameter individually with safe defaults for others that stay within 8 digit limit + expect { described_class.new(major_digits: count, minor_digits: 1, patch_digits: 1, build_digits: 1) }.not_to raise_error + expect { described_class.new(major_digits: 1, minor_digits: count, patch_digits: 1, build_digits: 1) }.not_to raise_error + expect { described_class.new(major_digits: 1, minor_digits: 1, patch_digits: count, build_digits: 1) }.not_to raise_error + expect { described_class.new(major_digits: 1, minor_digits: 1, patch_digits: 1, build_digits: count) }.not_to raise_error + end + end + + context 'with mixed valid digit counts within 8 total digits' do + all_digit_combinations.select { |combination| combination.sum <= 8 }.each do |major, minor, patch, build| + it "accepts #{major},#{minor},#{patch},#{build} digit combination" do + expect { described_class.new(major_digits: major, minor_digits: minor, patch_digits: patch, build_digits: build) }.not_to raise_error + end + end + end + end + + context 'with invalid digit counts' do + it 'rejects digit counts outside valid range (1-3)' do + expect { described_class.new(major_digits: 0) }.to raise_error(/Digit count must be between 1 and 3/) + expect { described_class.new(minor_digits: -1) }.to raise_error(/Digit count must be between 1 and 3/) + expect { described_class.new(patch_digits: 4) }.to raise_error(/Digit count must be between 1 and 3/) + end + + it 'rejects non-integer digit counts' do + expect { described_class.new(build_digits: '3') }.to raise_error(/Digit count must be an integer, got: String/) + expect { described_class.new(major_digits: 2.5) }.to raise_error(/Digit count must be an integer, got: Float/) + expect { described_class.new(minor_digits: 1.0) }.to raise_error(/Digit count must be an integer, got: Float/) + expect { described_class.new(patch_digits: nil) }.to raise_error(/Digit count must be an integer, got: NilClass/) + end + + context 'with configurations exceeding 8 total digits' do + all_digit_combinations.select { |combination| combination.sum > 8 }.each do |major, minor, patch, build| + it "rejects #{major},#{minor},#{patch},#{build} digit combination (total: #{major + minor + patch + build})" do + expect { described_class.new(major_digits: major, minor_digits: minor, patch_digits: patch, build_digits: build) }.to raise_error(/Total digit count \(#{major + minor + patch + build}\) exceeds maximum allowed \(8\)/) + end + end + end + end + end + describe 'prefix validation' do context 'with valid prefixes' do it 'accepts single digits 0-9' do @@ -96,25 +208,108 @@ it 'accepts integer inputs' do expect { described_class.new(prefix: 5) }.not_to raise_error end + + it 'accepts nil prefix' do + expect { described_class.new(prefix: nil) }.not_to raise_error + end end context 'with invalid prefixes' do - it 'rejects multi-character strings' do + it 'rejects invalid prefix formats' do + # Multi-character strings and out-of-range numbers expect { described_class.new(prefix: '12') }.to raise_error(/Prefix must be a single digit or empty string/) - end + expect { described_class.new(prefix: '10') }.to raise_error(/Prefix must be a single digit or empty string/) + expect { described_class.new(prefix: '-1') }.to raise_error(/Prefix must be a single digit or empty string/) - it 'rejects non-numeric strings' do + # Non-numeric characters expect { described_class.new(prefix: 'a') }.to raise_error(/Prefix must be an integer digit/) + expect { described_class.new(prefix: '@') }.to raise_error(/Prefix must be an integer digit/) end - it 'rejects numbers outside 0-9 range' do - expect { described_class.new(prefix: '10') }.to raise_error(/Prefix must be a single digit or empty string/) - expect { described_class.new(prefix: '-1') }.to raise_error(/Prefix must be a single digit or empty string/) + it 'rejects non-string and non-integer prefix types' do + # Arrays, hashes, and other objects + expect { described_class.new(prefix: []) }.to raise_error(/Prefix must be a string or integer/) + expect { described_class.new(prefix: {}) }.to raise_error(/Prefix must be a string or integer/) + expect { described_class.new(prefix: Object.new) }.to raise_error(/Prefix must be a string or integer/) + expect { described_class.new(prefix: true) }.to raise_error(/Prefix must be a string or integer/) + expect { described_class.new(prefix: false) }.to raise_error(/Prefix must be a string or integer/) + expect { described_class.new(prefix: 3.14) }.to raise_error(/Prefix must be a string or integer/) end + end + end - it 'rejects symbols and special characters' do - expect { described_class.new(prefix: '@') }.to raise_error(/Prefix must be an integer digit/) - expect { described_class.new(prefix: '#') }.to raise_error(/Prefix must be an integer digit/) + describe 'version component validation' do + context 'with version components within digit limits' do + it 'accepts version components that fit within their digit limits' do + formatter = described_class.new(major_digits: 1, minor_digits: 2, patch_digits: 2, build_digits: 3) + + # Valid cases: major=9 (1 digit), minor=99 (2 digits), patch=99 (2 digits), build=999 (3 digits) + version = Fastlane::Models::AppVersion.new(9, 99, 99, 999) + expect { formatter.build_code(version: version) }.not_to raise_error + end + + it 'accepts version components at exactly the digit limit' do + formatter = described_class.new(major_digits: 2, minor_digits: 1, patch_digits: 3, build_digits: 2) + + # Edge cases: exactly at limits + version = Fastlane::Models::AppVersion.new(99, 9, 999, 99) + expect { formatter.build_code(version: version) }.not_to raise_error + end + end + + context 'with version components exceeding digit limits' do + it 'rejects major version exceeding digit limit' do + formatter = described_class.new(major_digits: 1, minor_digits: 2, patch_digits: 2, build_digits: 2) + version = Fastlane::Models::AppVersion.new(10, 1, 1, 1) # major=10 is longer than 1 digit + + expect { formatter.build_code(version: version) }.to raise_error( + /Version component value \(10\) exceeds maximum allowed width of 1 characters/ + ) + end + + it 'rejects minor version exceeding digit limit' do + formatter = described_class.new(major_digits: 2, minor_digits: 1, patch_digits: 2, build_digits: 2) + version = Fastlane::Models::AppVersion.new(1, 23, 4, 5) # minor=23 is longer than 1 digit + + expect { formatter.build_code(version: version) }.to raise_error( + /Version component value \(23\) exceeds maximum allowed width of 1 characters/ + ) + end + + it 'rejects patch version exceeding digit limit' do + formatter = described_class.new(major_digits: 2, minor_digits: 2, patch_digits: 1, build_digits: 2) + version = Fastlane::Models::AppVersion.new(1, 2, 34, 5) # patch=34 is longer than 1 digit + + expect { formatter.build_code(version: version) }.to raise_error( + /Version component value \(34\) exceeds maximum allowed width of 1 characters/ + ) + end + + it 'rejects build number exceeding digit limit' do + formatter = described_class.new(major_digits: 2, minor_digits: 2, patch_digits: 2, build_digits: 1) + version = Fastlane::Models::AppVersion.new(1, 2, 3, 46) # build_number=46 is longer than 1 digit + + expect { formatter.build_code(version: version) }.to raise_error( + /Version component value \(46\) exceeds maximum allowed width of 1 characters/ + ) + end + + it 'provides helpful error messages with different digit limits' do + formatter = described_class.new(major_digits: 2, minor_digits: 2, patch_digits: 2, build_digits: 2) + version = Fastlane::Models::AppVersion.new(123, 4, 5, 6) # major=123 is longer than 2 digits + + expect { formatter.build_code(version: version) }.to raise_error( + /Version component value \(123\) exceeds maximum allowed width of 2 characters.*Consider increasing the corresponding `\*_digits` parameter/ + ) + end + + it 'validates the first component that exceeds limits' do + formatter = described_class.new(major_digits: 1, minor_digits: 1, patch_digits: 1, build_digits: 1) + version = Fastlane::Models::AppVersion.new(12, 34, 56, 78) # All exceed 1 digit limit, but major is checked first + + expect { formatter.build_code(version: version) }.to raise_error( + /Version component value \(12\) exceeds maximum allowed width of 1 characters/ + ) end end end