Fix qualifier comparison

The old code doesn't take milestone and release candidate versions
into account: M1 and M2 would be equal.
The new code deals with that.

Closes gh-1899

Author: mhalbritter

start-client/src/components/utils/Version.js

@@ -1,14 +1,42 @@
 const strictRange = /\[(.*),(.*)\]/
 const halfopenRightRange = /\[(.*),(.*)\)/
 const halfopenLeftRange = /\((.*),(.*)\]/
-const qualifiers = ['M', 'RC', 'BUILD-SNAPSHOT', 'RELEASE']
 
 export const parseQualifier = version => {
-  const qual = (version || '')
-    .replace(/\d+/g, '')
-    .replace(/\./g, ' ')
-    .replace(/\s/g, '')
-  return qualifiers.indexOf(qual) > -1 ? qual : 'RELEASE'
+  const splitted = (version || '').split('.')
+  if (splitted.length < 3) {
+    return 'RELEASE'
+  }
+  if (splitted.length === 4) {
+    return splitted[3]
+  }
+  const last = splitted[2]
+  const dash = last.indexOf('-')
+  if (dash !== -1) {
+    return last.substring(dash + 1)
+  }
+  return 'RELEASE'
+}
+
+export const RELEASE_ORDER = 300000
+const SNAPSHOT_ORDER = 200000
+const RELEASE_CANDIDATE_ORDER = 100000
+const MILESTONE_ORDER = 0
+
+const getQualifierOrder = qualifier => {
+  if (qualifier === 'RELEASE') {
+    return RELEASE_ORDER
+  }
+  if (qualifier === 'SNAPSHOT' || qualifier === 'BUILD-SNAPSHOT') {
+    return SNAPSHOT_ORDER
+  }
+  if (qualifier.startsWith('RC')) {
+    return RELEASE_CANDIDATE_ORDER + parseInt(qualifier.substring(2), 10)
+  }
+  if (qualifier.startsWith('M')) {
+    return MILESTONE_ORDER + parseInt(qualifier.substring(1), 10)
+  }
+  return RELEASE_ORDER
 }
 
 export const parseVersion = version => {
@@ -22,7 +50,7 @@ export const parseVersion = version => {
     version,
     short: `${r[0]}.${r[1]}.${r[2]}`,
     major: `${r[0]}.${r[1]}.x`,
-    qualify: qualifiers.indexOf(parseQualifier(version)),
+    qualify: getQualifierOrder(parseQualifier(version)),
     minor: +r[2],
   }
 }
@@ -43,7 +71,7 @@ export const compare = (a, b) => {
       return result
     }
   }
-  const qualify = version => qualifiers.indexOf(parseQualifier(version))
+  const qualify = version => getQualifierOrder(parseQualifier(version))
   result = qualify(a) - qualify(b)
   if (result !== 0) {
     return result
@@ -53,20 +81,19 @@ export const compare = (a, b) => {
 
 export const parseReleases = releases => {
   return releases.map(release => {
-    const version = parseVersion(release.key)
-    return version
+    return parseVersion(release.key)
   })
 }
 
 export const isInRange = (version, range) => {
   if (!range) {
     return true
   }
-  const strickMatch = range.match(strictRange)
-  if (strickMatch) {
+  const strictMatch = range.match(strictRange)
+  if (strictMatch) {
     return (
-      compare(strickMatch[1], version) <= 0 &&
-      compare(strickMatch[2], version) >= 0
+      compare(strictMatch[1], version) <= 0 &&
+      compare(strictMatch[2], version) >= 0
     )
   }
   const horMatch = range.match(halfopenRightRange)

start-client/src/components/utils/__tests__/Version.js

@@ -8,6 +8,7 @@ import {
   parseReleases,
   parseVersion,
   rangeToText,
+  RELEASE_ORDER,
 } from '../Version'
 
 /**
@@ -24,9 +25,9 @@ describe('parseQualifier', () => {
   })
   it('should return a specific qualifier', () => {
     let release = parseQualifier('1.0.0.M2')
-    expect(release).toBe('M')
+    expect(release).toBe('M2')
     release = parseQualifier('1.0.0.RC1')
-    expect(release).toBe('RC')
+    expect(release).toBe('RC1')
     release = parseQualifier('1.0.0.BUILD-SNAPSHOT')
     expect(release).toBe('BUILD-SNAPSHOT')
   })
@@ -41,7 +42,7 @@ describe('parseVersion', () => {
     expect(get(version, 'version')).toBe('1.0.1.RELEASE')
     expect(get(version, 'short')).toBe('1.0.1')
     expect(get(version, 'major')).toBe('1.0.x')
-    expect(get(version, 'qualify')).toBe(3)
+    expect(get(version, 'qualify')).toBe(RELEASE_ORDER)
     expect(get(version, 'minor')).toBe(1)
   })
 
@@ -89,7 +90,7 @@ describe('parseReleases', () => {
     expect(get(result[0], 'version')).toBe('1.0.1.RELEASE')
     expect(get(result[0], 'short')).toBe('1.0.1')
     expect(get(result[0], 'major')).toBe('1.0.x')
-    expect(get(result[0], 'qualify')).toBe(3)
+    expect(get(result[0], 'qualify')).toBe(RELEASE_ORDER)
     expect(get(result[0], 'minor')).toBe(1)
   })
 })
@@ -112,6 +113,26 @@ describe('isInRange', () => {
     result = isInRange('2.0.0.M1', '[2.0.0.RELEASE,2.2.0.M1)')
     expect(result).toBe(false)
   })
+  it('should match backend logic', () => {
+    const rangeInclusive = '[3.4.0,4.0.0-M2]'
+    const rangeExclusive = '[3.4.0,4.0.0-M3)'
+    expect(isInRange('3.4.9', rangeInclusive)).toBe(true)
+    expect(isInRange('3.4.9', rangeExclusive)).toBe(true)
+    expect(isInRange('3.4.10-SNAPSHOT', rangeInclusive)).toBe(true)
+    expect(isInRange('3.4.10-SNAPSHOT', rangeExclusive)).toBe(true)
+    expect(isInRange('3.5.5', rangeInclusive)).toBe(true)
+    expect(isInRange('3.5.5', rangeExclusive)).toBe(true)
+    expect(isInRange('3.5.6-SNAPSHOT', rangeInclusive)).toBe(true)
+    expect(isInRange('3.5.6-SNAPSHOT', rangeExclusive)).toBe(true)
+    expect(isInRange('4.0.0-M1', rangeInclusive)).toBe(true)
+    expect(isInRange('4.0.0-M1', rangeExclusive)).toBe(true)
+    expect(isInRange('4.0.0-M2', rangeInclusive)).toBe(true)
+    expect(isInRange('4.0.0-M2', rangeExclusive)).toBe(true)
+    expect(isInRange('4.0.0-M3', rangeInclusive)).toBe(false)
+    expect(isInRange('4.0.0-M3', rangeExclusive)).toBe(false)
+    expect(isInRange('4.0.0-SNAPSHOT', rangeInclusive)).toBe(false)
+    expect(isInRange('4.0.0-SNAPSHOT', rangeExclusive)).toBe(false)
+  })
 })
 
 /**