Skip to content

Commit dec6749

Browse files
yasirfolio3yasirfolio3
authored
fix: check to see if prefix is before build and visa versa in isBuild isPr… (#293)
* fixing sem-versioning. * decreasing cyclo-complexity of compareVersion.
1 parent 2fe4d95 commit dec6749

File tree

2 files changed

+157
-18
lines changed

2 files changed

+157
-18
lines changed

‎pkg/decision/evaluator/matchers/semver.go‎

Lines changed: 51 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,12 @@ package matchers
1919

2020
import (
2121
"fmt"
22-
"github.com/optimizely/go-sdk/pkg/logging"
2322
"regexp"
2423
"strconv"
2524
"strings"
2625

26+
"github.com/optimizely/go-sdk/pkg/logging"
27+
2728
"github.com/optimizely/go-sdk/pkg/decision/reasons"
2829
"github.com/optimizely/go-sdk/pkg/entities"
2930

@@ -65,16 +66,12 @@ func (sv SemanticVersion) compareVersion(attribute string) (int, error) {
6566
return -1, nil
6667
case !sv.isNumber(versionParts[idx]):
6768
// Compare strings
68-
if versionParts[idx] < targetedVersionParts[idx] {
69-
return -1, nil
70-
} else if versionParts[idx] > targetedVersionParts[idx] {
71-
return 1, nil
69+
if result := sv.compareVersionStrings(versionParts[idx], targetedVersionParts[idx], attribute); result != 0 {
70+
return result, nil
7271
}
7372
case sv.isNumber(targetedVersionParts[idx]): // both targetedVersionParts and versionParts are digits
74-
if sv.toInt(versionParts[idx]) < sv.toInt(targetedVersionParts[idx]) {
75-
return -1, nil
76-
} else if sv.toInt(versionParts[idx]) > sv.toInt(targetedVersionParts[idx]) {
77-
return 1, nil
73+
if result := sv.compareVersionNumbers(versionParts[idx], targetedVersionParts[idx]); result != 0 {
74+
return result, nil
7875
}
7976
default:
8077
return -1, nil
@@ -94,17 +91,16 @@ func (sv SemanticVersion) splitSemanticVersion(targetedVersion string) ([]string
9491
return []string{}, errors.New(string(reasons.AttributeFormatInvalid))
9592
}
9693

97-
9894
targetPrefix := targetedVersion
9995
var targetSuffix []string
10096

101-
if sv.isPreRelease(targetedVersion) || sv.isBuild(targetedVersion){
97+
if sv.isPreRelease(targetedVersion) || sv.isBuild(targetedVersion) {
10298
sep := buildSeperator
10399
if sv.isPreRelease(targetedVersion) {
104100
sep = preReleaseSeperator
105101
}
106102
// this is going to slit with the first occurrence.
107-
targetParts := strings.Split(targetedVersion, sep)
103+
targetParts := strings.SplitN(targetedVersion, sep, 2)
108104
// in the case it is neither a build or pre release it will return the
109105
// original string as the first element in the array
110106
if len(targetParts) == 0 {
@@ -128,8 +124,8 @@ func (sv SemanticVersion) splitSemanticVersion(targetedVersion string) ([]string
128124
return []string{}, errors.New(string(reasons.AttributeFormatInvalid))
129125
}
130126

131-
for i := 0; i < len(targetedVersionParts);i++ {
132-
if !sv.isNumber(targetedVersionParts[i]) {
127+
for i := 0; i < len(targetedVersionParts); i++ {
128+
if !sv.isNumber(targetedVersionParts[i]) {
133129
return []string{}, errors.New(string(reasons.AttributeFormatInvalid))
134130
}
135131
}
@@ -138,6 +134,32 @@ func (sv SemanticVersion) splitSemanticVersion(targetedVersion string) ([]string
138134
return targetedVersionParts, nil
139135
}
140136

137+
// returns 1 if versionPart > targetedVersionPart, -1 if targetedVersionPart > versionPart, 0 otherwise
138+
func (sv SemanticVersion) compareVersionStrings(versionPart, targetedVersionPart, attribute string) int {
139+
if versionPart < targetedVersionPart {
140+
if sv.isPreRelease(sv.Condition) && !sv.isPreRelease(attribute) {
141+
return 1
142+
}
143+
return -1
144+
} else if versionPart > targetedVersionPart {
145+
if !sv.isPreRelease(sv.Condition) && sv.isPreRelease(attribute) {
146+
return -1
147+
}
148+
return 1
149+
}
150+
return 0
151+
}
152+
153+
// returns 1 if versionPart > targetedVersionPart, -1 if targetedVersionPart > versionPart, 0 otherwise
154+
func (sv SemanticVersion) compareVersionNumbers(versionPart, targetedVersionPart string) int {
155+
if sv.toInt(versionPart) < sv.toInt(targetedVersionPart) {
156+
return -1
157+
} else if sv.toInt(versionPart) > sv.toInt(targetedVersionPart) {
158+
return 1
159+
}
160+
return 0
161+
}
162+
141163
func (sv SemanticVersion) isNumber(str string) bool {
142164
return digitCheck.MatchString(str)
143165
}
@@ -151,15 +173,27 @@ func (sv SemanticVersion) toInt(str string) int {
151173
}
152174

153175
func (sv SemanticVersion) isPreRelease(str string) bool {
154-
return strings.Contains(str, preReleaseSeperator)
176+
if prIndex := strings.Index(str, preReleaseSeperator); prIndex > 0 {
177+
if buildIndex := strings.Index(str, buildSeperator); buildIndex > 0 {
178+
return prIndex < buildIndex
179+
}
180+
return true
181+
}
182+
return false
155183
}
156184

157185
func (sv SemanticVersion) isBuild(str string) bool {
158-
return strings.Contains(str, buildSeperator)
186+
if buildIndex := strings.Index(str, buildSeperator); buildIndex > 0 {
187+
if prIndex := strings.Index(str, preReleaseSeperator); prIndex > 0 {
188+
return buildIndex < prIndex
189+
}
190+
return true
191+
}
192+
return false
159193
}
160194

161195
func (sv SemanticVersion) hasWhiteSpace(str string) bool {
162-
return str == "" || strings.Contains(str, whiteSpace)
196+
return str == "" || strings.Contains(str, whiteSpace)
163197
}
164198

165199
// SemverEvaluator is a help function to wrap a common evaluation code

‎pkg/decision/evaluator/matchers/semver_test.go‎

Lines changed: 106 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,112 @@ func TestValidAttributesBetaToRelease(t *testing.T) {
178178
}
179179
}
180180

181+
func TestTargetBetaAndBetaComplex(t *testing.T) {
182+
testValues := []string{"2.1.3-beta+1", "2.1.3+build-1.2.3"}
183+
scenarios := []struct {
184+
matchType string
185+
version string
186+
expected bool
187+
}{
188+
{matchType: "semver_eq", version: "2.1.3-beta+1.2.3", expected: false},
189+
{matchType: "semver_eq", version: "2.1.3+build-1", expected: false},
190+
191+
{matchType: "semver_ge", version: "2.1.3-beta+1.2.3", expected: true},
192+
{matchType: "semver_ge", version: "2.1.3+build-1", expected: false},
193+
194+
{matchType: "semver_gt", version: "2.1.3-beta+1.2.3", expected: true},
195+
{matchType: "semver_gt", version: "2.1.3+build-1", expected: false},
196+
197+
{matchType: "semver_le", version: "2.1.3-beta+1.2.3", expected: false},
198+
{matchType: "semver_le", version: "2.1.3+build-1", expected: true},
199+
200+
{matchType: "semver_lt", version: "2.1.3-beta+1.2.3", expected: false},
201+
{matchType: "semver_lt", version: "2.1.3+build-1", expected: true},
202+
}
203+
204+
for index, scenario := range scenarios {
205+
condition := entities.Condition{
206+
Match: scenario.matchType,
207+
Value: testValues[index%len(testValues)],
208+
Name: "version",
209+
}
210+
211+
user := entities.UserContext{
212+
Attributes: map[string]interface{}{
213+
"version": scenario.version,
214+
},
215+
}
216+
217+
messageAndArgs := []interface{}{"matchType: %s, condition: %s, attribute: %s", scenario.matchType, condition.Value, scenario.version}
218+
219+
matcher, ok := Get(scenario.matchType)
220+
assert.True(t, ok, messageAndArgs...)
221+
222+
actual, err := matcher(condition, user, nil)
223+
assert.NoError(t, err, messageAndArgs...)
224+
225+
assert.Equal(t, scenario.expected, actual, messageAndArgs...)
226+
}
227+
}
228+
229+
func TestDifferentAttributeAgainstBuildAndPrerelease(t *testing.T) {
230+
testValues := []string{"3.7.0", "3.7.0", "3.7.0-prerelease", "3.7.0-prerelease+build"}
231+
scenarios := []struct {
232+
matchType string
233+
version string
234+
expected bool
235+
}{
236+
{matchType: "semver_eq", version: "3.7.0+build", expected: true},
237+
{matchType: "semver_eq", version: "3.7.0-prerelease", expected: false},
238+
{matchType: "semver_eq", version: "3.7.0+build", expected: false},
239+
{matchType: "semver_eq", version: "3.7.0-prerelease-prelrease+rc", expected: false},
240+
241+
{matchType: "semver_ge", version: "3.7.0+build", expected: true},
242+
{matchType: "semver_ge", version: "3.7.0-prerelease", expected: false},
243+
{matchType: "semver_ge", version: "3.7.0+build", expected: true},
244+
{matchType: "semver_ge", version: "3.7.0-prerelease-prelrease+rc", expected: true},
245+
246+
{matchType: "semver_gt", version: "3.7.0+build", expected: false},
247+
{matchType: "semver_gt", version: "3.7.0-prerelease", expected: false},
248+
{matchType: "semver_gt", version: "3.7.0+build", expected: true},
249+
{matchType: "semver_gt", version: "3.7.0-prerelease-prelrease+rc", expected: true},
250+
251+
{matchType: "semver_le", version: "3.7.0+build", expected: true},
252+
{matchType: "semver_le", version: "3.7.0-prerelease", expected: true},
253+
{matchType: "semver_le", version: "3.7.0+build", expected: false},
254+
{matchType: "semver_le", version: "3.7.0-prerelease-prelrease+rc", expected: false},
255+
256+
{matchType: "semver_lt", version: "3.7.0+build", expected: false},
257+
{matchType: "semver_lt", version: "3.7.0-prerelease", expected: true},
258+
{matchType: "semver_lt", version: "3.7.0+build", expected: false},
259+
{matchType: "semver_lt", version: "3.7.0-prerelease-prelrease+rc", expected: false},
260+
}
261+
262+
for index, scenario := range scenarios {
263+
condition := entities.Condition{
264+
Match: scenario.matchType,
265+
Value: testValues[index%len(testValues)],
266+
Name: "version",
267+
}
268+
269+
user := entities.UserContext{
270+
Attributes: map[string]interface{}{
271+
"version": scenario.version,
272+
},
273+
}
274+
275+
messageAndArgs := []interface{}{"matchType: %s, condition: %s, attribute: %s", scenario.matchType, condition.Value, scenario.version}
276+
277+
matcher, ok := Get(scenario.matchType)
278+
assert.True(t, ok, messageAndArgs...)
279+
280+
actual, err := matcher(condition, user, nil)
281+
assert.NoError(t, err, messageAndArgs...)
282+
283+
assert.Equal(t, scenario.expected, actual, messageAndArgs...)
284+
}
285+
}
286+
181287
func TestInvalidAttributes(t *testing.T) {
182288

183289
condition := entities.Condition{
@@ -198,7 +304,6 @@ func TestInvalidAttributes(t *testing.T) {
198304
true,
199305
37,
200306
nil,
201-
"",
202307
"-",
203308
".",
204309
"..",

0 commit comments

Comments
 (0)