From 04cf97aaf826664fbbc3f6fc4e0d59ef7b6e1930 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lovro=20Ma=C5=BEgon?= Date: Thu, 14 Mar 2024 13:54:38 +0100 Subject: [PATCH 1/3] make sure plugins with non-semantic version are registered as latest --- pkg/plugin/name.go | 26 ++++++++++++++++++-------- pkg/plugin/name_test.go | 9 +++++++-- 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/pkg/plugin/name.go b/pkg/plugin/name.go index f6a31fb71..2c79c2342 100644 --- a/pkg/plugin/name.go +++ b/pkg/plugin/name.go @@ -72,18 +72,28 @@ func (fn FullName) PluginVersion() string { return PluginVersionLatest // default } -func (fn FullName) PluginVersionGreaterThan(other FullName) bool { +func (fn FullName) PluginVersionGreaterThan(right FullName) bool { + if right == "" { + return true // right is empty, left is greater either way + } + if fn == "" { + return true // left is empty, right is greater either way + } + leftVersion := fn.PluginVersion() - rightVersion := other.PluginVersion() + leftSemver, errLeft := semver.NewVersion(leftVersion) - leftSemver, err := semver.NewVersion(leftVersion) - if err != nil { + rightVersion := right.PluginVersion() + rightSemver, errRight := semver.NewVersion(rightVersion) + + if errLeft != nil && errRight != nil { + // both are invalid semvers, compare as strings + return leftVersion < rightVersion + } else if errRight != nil { + return true // right is an invalid semver, left is greater either way + } else if errLeft != nil { return false // left is an invalid semver, right is greater either way } - rightSemver, err := semver.NewVersion(rightVersion) - if err != nil { - return true // left is a valid semver, right is not, left is greater - } return leftSemver.GreaterThan(rightSemver) } diff --git a/pkg/plugin/name_test.go b/pkg/plugin/name_test.go index 86b8da75e..4da83432d 100644 --- a/pkg/plugin/name_test.go +++ b/pkg/plugin/name_test.go @@ -28,6 +28,7 @@ func TestFullName(t *testing.T) { pn string // expected plugin name pv string // expected plugin version }{ + {fn: "", pt: "any", pn: "", pv: "latest"}, {fn: "my-plugin", pt: "any", pn: "my-plugin", pv: "latest"}, {fn: "builtin:my-plugin", pt: "builtin", pn: "my-plugin", pv: "latest"}, {fn: "my-plugin@v0.1.1", pt: "any", pn: "my-plugin", pv: "v0.1.1"}, @@ -50,7 +51,7 @@ func TestFullName_PluginVersionGreaterThanP(t *testing.T) { testCases := []struct { v1 string // left version v2 string // right version - gt bool // want greater than + gt bool // is left greater than right }{ {v1: "v0.0.1", v2: "v0.0.1", gt: false}, {v1: "v0.1", v2: "v0.0.1", gt: true}, @@ -60,8 +61,12 @@ func TestFullName_PluginVersionGreaterThanP(t *testing.T) { {v1: "foo", v2: "v0.0.1", gt: false}, {v1: "v0.0.1", v2: "foo", gt: true}, {v1: "foo", v2: "bar", gt: false}, - {v1: "bar", v2: "foo", gt: false}, + {v1: "bar", v2: "foo", gt: true}, {v1: "v0.0.1", v2: "v0.0.1-dirty", gt: true}, + {v1: "bar", v2: "", gt: true}, + {v1: "v1", v2: "", gt: true}, + {v1: "", v2: "bar", gt: false}, + {v1: "", v2: "v1", gt: false}, } for _, tc := range testCases { From 9bf3621976137920c3b5fdbabcf70daa42d5797d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lovro=20Ma=C5=BEgon?= Date: Thu, 14 Mar 2024 14:01:14 +0100 Subject: [PATCH 2/3] fix linter warning --- pkg/plugin/name.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/pkg/plugin/name.go b/pkg/plugin/name.go index 2c79c2342..bd204df5d 100644 --- a/pkg/plugin/name.go +++ b/pkg/plugin/name.go @@ -86,12 +86,13 @@ func (fn FullName) PluginVersionGreaterThan(right FullName) bool { rightVersion := right.PluginVersion() rightSemver, errRight := semver.NewVersion(rightVersion) - if errLeft != nil && errRight != nil { + switch { + case errLeft != nil && errRight != nil: // both are invalid semvers, compare as strings return leftVersion < rightVersion - } else if errRight != nil { + case errRight != nil: return true // right is an invalid semver, left is greater either way - } else if errLeft != nil { + case errLeft != nil: return false // left is an invalid semver, right is greater either way } From fd1c3325efe5748a97b2d063fef4f47c31fb791b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lovro=20Ma=C5=BEgon?= Date: Thu, 14 Mar 2024 14:21:49 +0100 Subject: [PATCH 3/3] fix comparison logic --- pkg/plugin/name.go | 13 ++++++------- pkg/plugin/name_test.go | 13 ++++++++++--- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/pkg/plugin/name.go b/pkg/plugin/name.go index bd204df5d..4b33ee7f9 100644 --- a/pkg/plugin/name.go +++ b/pkg/plugin/name.go @@ -73,13 +73,6 @@ func (fn FullName) PluginVersion() string { } func (fn FullName) PluginVersionGreaterThan(right FullName) bool { - if right == "" { - return true // right is empty, left is greater either way - } - if fn == "" { - return true // left is empty, right is greater either way - } - leftVersion := fn.PluginVersion() leftSemver, errLeft := semver.NewVersion(leftVersion) @@ -88,6 +81,12 @@ func (fn FullName) PluginVersionGreaterThan(right FullName) bool { switch { case errLeft != nil && errRight != nil: + switch { + case leftVersion == PluginVersionLatest: + return false // latest could be anything, we prioritize explicit versions + case rightVersion == PluginVersionLatest: + return true // latest could be anything, we prioritize explicit versions + } // both are invalid semvers, compare as strings return leftVersion < rightVersion case errRight != nil: diff --git a/pkg/plugin/name_test.go b/pkg/plugin/name_test.go index 4da83432d..a3828ddac 100644 --- a/pkg/plugin/name_test.go +++ b/pkg/plugin/name_test.go @@ -47,7 +47,7 @@ func TestFullName(t *testing.T) { } } -func TestFullName_PluginVersionGreaterThanP(t *testing.T) { +func TestFullName_PluginVersionGreaterThan(t *testing.T) { testCases := []struct { v1 string // left version v2 string // right version @@ -63,10 +63,11 @@ func TestFullName_PluginVersionGreaterThanP(t *testing.T) { {v1: "foo", v2: "bar", gt: false}, {v1: "bar", v2: "foo", gt: true}, {v1: "v0.0.1", v2: "v0.0.1-dirty", gt: true}, - {v1: "bar", v2: "", gt: true}, + {v1: "zoo", v2: "", gt: true}, {v1: "v1", v2: "", gt: true}, - {v1: "", v2: "bar", gt: false}, + {v1: "", v2: "zoo", gt: false}, {v1: "", v2: "v1", gt: false}, + {v1: "", v2: "", gt: false}, } for _, tc := range testCases { @@ -75,6 +76,12 @@ func TestFullName_PluginVersionGreaterThanP(t *testing.T) { v1 := NewFullName("builtin", "test", tc.v1) v2 := NewFullName("builtin", "test", tc.v2) is.Equal(v1.PluginVersionGreaterThan(v2), tc.gt) + + // even if plugin type and plugin name are empty, the version + // comparison should still work the same + v1 = NewFullName("", "", tc.v1) + v2 = NewFullName("", "", tc.v2) + is.Equal(v1.PluginVersionGreaterThan(v2), tc.gt) }) } }