From 914f11d8a87c8788e6501b3958a0ebb87cd930f9 Mon Sep 17 00:00:00 2001 From: Shane Warden <shanew@activestate.com> Date: Tue, 28 Jun 2022 09:26:36 -0700 Subject: [PATCH 1/2] Add int64 slice to Version struct This is currently unused, but it can store integers, not decimals. --- pkg/version/version.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/pkg/version/version.go b/pkg/version/version.go index 2ce4c3b..21fb9ae 100644 --- a/pkg/version/version.go +++ b/pkg/version/version.go @@ -34,6 +34,7 @@ package version //go:generate enumer -type ParsedAs . import ( + "encoding/json" "errors" "fmt" @@ -69,6 +70,8 @@ const ( type Version struct { // Original is the string that was passed to the parsing func. Original string `json:"version"` + // The simplest form of data + Ints []int64 // Decimal contains a slice of `*decimal.Big` values. This will always // contain at least one element. Decimal []*decimal.Big `json:"sortable_version"` @@ -76,6 +79,29 @@ type Version struct { ParsedAs ParsedAs `json:"-"` } +func (v *Version) MarshalJSON() ([]byte, error) { + if v.Decimal != nil { + return json.Marshal(&struct { + Original string `json:"version"` + Decimal []*decimal.Big `json:"sortable_version"` + ParsedAs ParsedAs `json:"-"` + }{ + Original: v.Original, + Decimal: v.Decimal, + ParsedAs: v.ParsedAs, + }) + } + return json.Marshal(&struct { + Original string `json:"version"` + Ints []int64 `json:"sortable_version"` + ParsedAs ParsedAs `json:"-"` + }{ + Original: v.Original, + Ints: v.Ints, + ParsedAs: v.ParsedAs, + }) +} + // fromStringSlice take a version type and a slice of strings and returns a // new Version struct. Each element of the string slice should contain a // string representation of a number. This returns an error if any element of From 6a92d9ab8842f1298562476b4448c9f483c2b70c Mon Sep 17 00:00:00 2001 From: Shane Warden <shanew@activestate.com> Date: Tue, 28 Jun 2022 09:43:23 -0700 Subject: [PATCH 2/2] Enable Ints/Decimal promotion in Version --- pkg/version/perl_test.go | 4 +- pkg/version/python_test.go | 4 +- pkg/version/shared_test.go | 44 ++++++++-- pkg/version/version.go | 166 +++++++++++++++++++++++++++++++++---- 4 files changed, 193 insertions(+), 25 deletions(-) diff --git a/pkg/version/perl_test.go b/pkg/version/perl_test.go index 268654a..bf759e0 100644 --- a/pkg/version/perl_test.go +++ b/pkg/version/perl_test.go @@ -123,8 +123,8 @@ func TestParsePerl(t *testing.T) { } else { require.NoError(t, err) assert.Equal(t, pa, actual.ParsedAs, "got expected ParsedAs value") - assertDecimalEqualString(t, tt.expected, actual.Decimal) - assertDecimalEqualDecimal(t, tt.expected, actual.Decimal) + assertStringEquality(t, tt.expected, actual) + assertNumericEquality(t, tt.expected, actual) } }) } diff --git a/pkg/version/python_test.go b/pkg/version/python_test.go index 050edb2..6387261 100644 --- a/pkg/version/python_test.go +++ b/pkg/version/python_test.go @@ -111,8 +111,8 @@ func TestParsePython(t *testing.T) { actual, err := ParsePython(tt.version) require.NoError(t, err) assert.Equal(t, pa, actual.ParsedAs, "got expected ParsedAs value") - assertDecimalEqualString(t, tt.expected, actual.Decimal) - assertDecimalEqualDecimal(t, tt.expected, actual.Decimal) + assertStringEquality(t, tt.expected, actual) + assertNumericEquality(t, tt.expected, actual) }) } } diff --git a/pkg/version/shared_test.go b/pkg/version/shared_test.go index bdb8764..578fee7 100644 --- a/pkg/version/shared_test.go +++ b/pkg/version/shared_test.go @@ -2,6 +2,7 @@ package version import ( "fmt" + "strconv" "testing" "github.com/ericlagergren/decimal" @@ -54,8 +55,8 @@ func TestParseGeneric(t *testing.T) { actual, err := ParseGeneric(tt.version) require.NoError(t, err) assert.Equal(t, Generic, actual.ParsedAs, "got expected ParsedAs value") - assertDecimalEqualString(t, tt.expected, actual.Decimal) - assertDecimalEqualDecimal(t, tt.expected, actual.Decimal) + assertStringEquality(t, tt.expected, actual) + assertNumericEquality(t, tt.expected, actual) }) } } @@ -150,8 +151,8 @@ func TestParseSemVer(t *testing.T) { } else { require.NoError(t, err) assert.Equal(t, SemVer, actual.ParsedAs, "got expected ParsedAs value") - assertDecimalEqualString(t, test.expected, actual.Decimal) - assertDecimalEqualDecimal(t, test.expected, actual.Decimal) + assertStringEquality(t, test.expected, actual) + assertNumericEquality(t, test.expected, actual) } }) } @@ -237,6 +238,29 @@ func TestIsNumber(t *testing.T) { assert.False(t, isNumber("1.2.3")) } +func assertStringEquality(t *testing.T, expected []string, actual *Version) { + if actual.Decimal != nil { + assertDecimalEqualString(t, expected, actual.Decimal) + } else { + assertIntsEqualString(t, expected, actual.Ints) + } +} + +func assertNumericEquality(t *testing.T, expected []string, actual *Version) { + if actual.Decimal != nil { + assertDecimalEqualDecimal(t, expected, actual.Decimal) + } else { + assertIntsEqualInts(t, expected, actual.Ints) + } +} + +func assertIntsEqualString(t *testing.T, expected []string, actual []int64) { + require.Equal(t, len(expected), len(actual)) + for i := range expected { + assert.Equal(t, expected[i], strconv.FormatInt(actual[i], 10)) + } +} + func assertDecimalEqualString(t *testing.T, expected []string, actual []*decimal.Big) { require.Equal(t, len(expected), len(actual)) for i := range expected { @@ -244,6 +268,12 @@ func assertDecimalEqualString(t *testing.T, expected []string, actual []*decimal } } +func assertIntsEqualInts(t *testing.T, expected []string, actual []int64) { + expectedInts, err := stringsToInts(expected) + assert.NoError(t, err) + assert.Equal(t, expectedInts, actual) +} + func assertDecimalEqualDecimal(t *testing.T, expected []string, actual []*decimal.Big) { expectedDecimals, err := stringsToDecimals(expected) assert.NoError(t, err) @@ -354,8 +384,8 @@ func TestClone(t *testing.T) { assert.Equal(t, v1.Original, v2.Original, "cloned version has same Original string") assert.Equal(t, v1.ParsedAs, v2.ParsedAs, "cloned version has same ParsedAs value") - v1.Decimal[0] = decimal.New(0, 0) - assert.NotEqual(t, 0, Compare(v1, v2), "changing Decimal slice in original does not change clone") + v1.Ints[0] = 0 + assert.NotEqual(t, 0, Compare(v1, v2), "changing slice in original does not change clone") } func TestString(t *testing.T) { @@ -385,7 +415,7 @@ func TestTrimTrailingZeros(t *testing.T) { for _, tt := range tests { t.Run(fmt.Sprint(tt.input), func(t *testing.T) { input := mustStringsToDecimal(t, tt.input) - actual := trimTrailingZeros(input) + actual := trimTrailingZerosDecimals(input) expected := mustStringsToDecimal(t, tt.expected) assert.Equal(t, expected, actual, "expected %v got %v", expected, actual) }) diff --git a/pkg/version/version.go b/pkg/version/version.go index 21fb9ae..490500b 100644 --- a/pkg/version/version.go +++ b/pkg/version/version.go @@ -37,6 +37,7 @@ import ( "encoding/json" "errors" "fmt" + "strconv" "github.com/ericlagergren/decimal" ) @@ -107,19 +108,67 @@ func (v *Version) MarshalJSON() ([]byte, error) { // string representation of a number. This returns an error if any element of // the slice cannot be converted to a *decimal.Big value. func fromStringSlice(pa ParsedAs, original string, strings []string) (*Version, error) { + if isOnlyInts(strings) { + ints, err := stringsToInts(strings) + if err == nil { + ints = trimTrailingZerosInts(ints) + return &Version{ + Original: original, + Ints: ints, + Decimal: nil, + ParsedAs: pa, + }, nil + } + } + + // fall back to this case if there aren't only ints + // or if string to int64 conversion failed at any point + return makeDecimalVersions(pa, original, strings) +} + +func makeDecimalVersions(pa ParsedAs, original string, strings []string) (*Version, error) { decimals, err := stringsToDecimals(strings) if err != nil { return nil, err } - decimals = trimTrailingZeros(decimals) + decimals = trimTrailingZerosDecimals(decimals) return &Version{ Original: original, + Ints: nil, Decimal: decimals, ParsedAs: pa, }, nil } +// 2^63 is 19 digits long, so play it safe +func isOnlyInts(strings []string) bool { + for _, s := range strings { + if len(s) > 18 { + return false + } + } + + return true +} + +func stringsToInts(strings []string) ([]int64, error) { + if len(strings) == 0 { + return nil, errors.New("The provided string slice must have at least one element") + } + + ints := make([]int64, len(strings)) + for i, s := range strings { + n, err := strconv.ParseInt(s, 10, 64) + if err != nil { + return nil, errors.New("Failed to create int64 from " + s) + } + ints[i] = n + } + + return ints, nil +} + func stringsToDecimals(strings []string) ([]*decimal.Big, error) { if len(strings) == 0 { return nil, errors.New("The provided string slice must have at least one element") @@ -137,7 +186,19 @@ func stringsToDecimals(strings []string) ([]*decimal.Big, error) { return decimals, nil } -func trimTrailingZeros(decimals []*decimal.Big) []*decimal.Big { +func trimTrailingZerosInts(ints []int64) []int64 { + indexOfLastZero := len(ints) + for i := len(ints) - 1; i > 0; i-- { + if ints[i] != 0 { + break + } + indexOfLastZero = i + } + + return ints[0:indexOfLastZero] +} + +func trimTrailingZerosDecimals(decimals []*decimal.Big) []*decimal.Big { indexOfLastZero := len(decimals) for i := len(decimals) - 1; i > 0; i-- { if decimals[i].Cmp(bigZero) != 0 { @@ -159,7 +220,35 @@ var bigZero = decimal.New(0, 0) // Versions that differ only by trailing zeros (e.g. "1.2" and "1.2.0") are // equal. func Compare(v1, v2 *Version) int { - min, max, longest, flip := minMax(v1.Decimal, v2.Decimal) + if v1.Decimal != nil && v2.Decimal != nil { + return compareDecimals(v1, v2) + } else if v1.Decimal != nil && v2.Decimal == nil { + v2.Decimal = promoteDecimals(v2) + cmp := compareDecimals(v1, v2) + v2.Decimal = nil + return cmp + } else if v1.Decimal == nil && v2.Decimal != nil { + v1.Decimal = promoteDecimals(v1) + cmp := compareDecimals(v1, v2) + v1.Decimal = nil + return cmp + } else { + return compareInts(v1, v2) + } +} + +func promoteDecimals(v *Version) []*decimal.Big { + decimals := make([]*decimal.Big, len(v.Ints)) + for i, n := range v.Ints { + d := decimal.New(n, 0) + decimals[i] = d + } + + return decimals +} + +func compareDecimals(v1 *Version, v2 *Version) int { + min, max, longest, flip := minMaxDecimals(v1.Decimal, v2.Decimal) // find any difference between these versions where they have the same number of segments for i := 0; i < min; i++ { @@ -180,8 +269,47 @@ func Compare(v1, v2 *Version) int { return 0 } -// helper function to find the lengths of and longest version segment array -func minMax(v1 []*decimal.Big, v2 []*decimal.Big) (int, int, []*decimal.Big, int) { +func compareInts(v1 *Version, v2 *Version) int { + min, max, longest, flip := minMaxInts(v1.Ints, v2.Ints) + + // find any difference between these versions where they have the same number of segments + for i := 0; i < min; i++ { + if v1.Ints[i] != v2.Ints[i] { + if v1.Ints[i] < v2.Ints[i] { + return -1 + } else { + return 1 + } + } + } + + // compare remaining segments to zero + for i := min; i < max; i++ { + if longest[i] != 0 { + if longest[i] < 0 { + return -1 * flip + } else { + return flip + } + } + } + + return 0 +} + +// helper function to find the lengths of and longest version segment array of Decimals +func minMaxDecimals(v1 []*decimal.Big, v2 []*decimal.Big) (int, int, []*decimal.Big, int) { + l1 := len(v1) + l2 := len(v2) + + if l1 < l2 { + return l1, l2, v2, -1 + } + return l2, l1, v1, 1 +} + +// helper function to find the lengths of and longest version segment array of Ints +func minMaxInts(v1 []int64, v2 []int64) (int, int, []int64, int) { l1 := len(v1) l2 := len(v2) @@ -194,15 +322,25 @@ func minMax(v1 []*decimal.Big, v2 []*decimal.Big) (int, int, []*decimal.Big, int // Clone returns a new *Version that is a clone of the one passed as the // method receiver. func (v *Version) Clone() *Version { - d := make([]*decimal.Big, len(v.Decimal)) - for i := range v.Decimal { - d[i] = decimal.New(0, 0) - d[i].Copy(v.Decimal[i]) - } - return &Version{ - Original: v.Original, - Decimal: d, - ParsedAs: v.ParsedAs, + if v.Decimal != nil { + d := make([]*decimal.Big, len(v.Decimal)) + for i := range v.Decimal { + d[i] = decimal.New(0, 0) + d[i].Copy(v.Decimal[i]) + } + return &Version{ + Original: v.Original, + Decimal: d, + ParsedAs: v.ParsedAs, + } + } else { + i := make([]int64, len(v.Ints)) + copy(i, v.Ints) + return &Version{ + Original: v.Original, + Ints: i, + ParsedAs: v.ParsedAs, + } } }