Skip to content

Commit cb204b2

Browse files
gcx-sebsbernard31
gcx-seb
authored and
sbernard31
committed
GH-1532: Some Fix/Enhancement about Version class
1 parent 7829f0a commit cb204b2

File tree

2 files changed

+105
-36
lines changed

2 files changed

+105
-36
lines changed

‎leshan-core/src/main/java/org/eclipse/leshan/core/LwM2m.java‎

Lines changed: 38 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,9 @@ public interface LwM2m {
2222
*/
2323
public class LwM2mVersion extends Version {
2424

25-
public static LwM2mVersion V1_0 = new LwM2mVersion("1.0", true);
26-
public static LwM2mVersion V1_1 = new LwM2mVersion("1.1", true);
27-
private static LwM2mVersion[] supportedVersions = new LwM2mVersion[] { V1_0, V1_1 };
25+
public static final LwM2mVersion V1_0 = new LwM2mVersion("1.0", true);
26+
public static final LwM2mVersion V1_1 = new LwM2mVersion("1.1", true);
27+
private static final LwM2mVersion[] supportedVersions = new LwM2mVersion[] { V1_0, V1_1 };
2828

2929
private final boolean supported;
3030

@@ -91,35 +91,45 @@ public boolean equals(Object obj) {
9191
*/
9292
public class Version implements Comparable<Version> {
9393

94-
public static Version V1_0 = new Version("1.0");
95-
public static final Version MAX = new Version(Short.MAX_VALUE, Short.MIN_VALUE);
94+
public static final Version V1_0 = new Version("1.0");
95+
public static final Version MAX = new Version(Short.MAX_VALUE, Short.MAX_VALUE);
9696

97-
protected final Short major;
98-
protected final Short minor;
97+
private static short toShortExact(int value, String format, Object... args) {
98+
if ((short) value != value) {
99+
throw new IllegalArgumentException(String.format(format, args));
100+
}
101+
return (short) value;
102+
}
103+
104+
protected final short major;
105+
protected final short minor;
99106

100107
public Version(int major, int minor) {
101-
this.major = (short) major;
102-
this.minor = (short) minor;
108+
this(toShortExact(major, "version (%d.%d) major part (%d) is not a valid short", major, minor, major),
109+
toShortExact(minor, "version (%d.%d) minor part (%d) is not a valid short", major, minor, minor));
103110
}
104111

105-
public Version(Short major, Short minor) {
112+
public Version(short major, short minor) {
106113
this.major = major;
114+
if (this.major < 0) {
115+
throw new IllegalArgumentException(
116+
String.format("version (%d.%d) major part (%d) must not be negative", major, minor, major));
117+
}
107118
this.minor = minor;
119+
if (this.minor < 0) {
120+
throw new IllegalArgumentException(
121+
String.format("version (%d.%d) minor part (%d) must not be negative", major, minor, minor));
122+
}
108123
}
109124

110125
public Version(String version) {
111-
try {
112-
String[] versionPart = version.split("\\.");
113-
this.major = Short.parseShort(versionPart[0]);
114-
this.minor = Short.parseShort(versionPart[1]);
115-
} catch (RuntimeException e) {
116-
String err = Version.validate(version);
117-
if (err != null) {
118-
throw new IllegalArgumentException(err);
119-
} else {
120-
throw e;
121-
}
126+
String err = Version.validate(version);
127+
if (err != null) {
128+
throw new IllegalArgumentException(err);
122129
}
130+
String[] versionPart = version.split("\\.");
131+
this.major = Short.parseShort(versionPart[0]);
132+
this.minor = Short.parseShort(versionPart[1]);
123133
}
124134

125135
@Override
@@ -133,16 +143,16 @@ public static Version getDefault() {
133143

134144
public static String validate(String version) {
135145
if (version == null || version.isEmpty())
136-
return "version MUST NOT be null or empty";
146+
return "version MUST NOT be null or empty";
137147
String[] versionPart = version.split("\\.");
138148
if (versionPart.length != 2) {
139149
return String.format("version (%s) MUST be composed of 2 parts", version);
140150
}
141151
for (int i = 0; i < 2; i++) {
142152
try {
143-
Short parsedShort = Short.parseShort(versionPart[i]);
153+
short parsedShort = Short.parseShort(versionPart[i]);
144154
if (parsedShort < 0) {
145-
return String.format("version (%s) part %d (%s) is not a valid short", version, i + 1,
155+
return String.format("version (%s) part %d (%s) must not be negative", version, i + 1,
146156
versionPart[i]);
147157
}
148158
} catch (Exception e) {
@@ -157,8 +167,8 @@ public static String validate(String version) {
157167
public int hashCode() {
158168
final int prime = 31;
159169
int result = 1;
160-
result = prime * result + ((major == null) ? 0 : major.hashCode());
161-
result = prime * result + ((minor == null) ? 0 : minor.hashCode());
170+
result = prime * result + major;
171+
result = prime * result + minor;
162172
return result;
163173
}
164174

@@ -171,15 +181,9 @@ public boolean equals(Object obj) {
171181
if (getClass() != obj.getClass())
172182
return false;
173183
Version other = (Version) obj;
174-
if (major == null) {
175-
if (other.major != null)
176-
return false;
177-
} else if (!major.equals(other.major))
184+
if (major != other.major)
178185
return false;
179-
if (minor == null) {
180-
if (other.minor != null)
181-
return false;
182-
} else if (!minor.equals(other.minor))
186+
if (minor != other.minor)
183187
return false;
184188
return true;
185189
}
@@ -188,7 +192,6 @@ public boolean equals(Object obj) {
188192
public int compareTo(Version other) {
189193
if (major != other.major)
190194
return major - other.major;
191-
192195
return minor - other.minor;
193196
}
194197

‎leshan-core/src/test/java/org/eclipse/leshan/core/VersionTest.java‎

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,22 @@
1515
*******************************************************************************/
1616
package org.eclipse.leshan.core;
1717

18+
import static org.junit.jupiter.api.Assertions.assertEquals;
1819
import static org.junit.jupiter.api.Assertions.assertFalse;
20+
import static org.junit.jupiter.api.Assertions.assertThrows;
1921
import static org.junit.jupiter.api.Assertions.assertTrue;
2022

23+
import java.util.stream.Stream;
24+
2125
import org.eclipse.leshan.core.LwM2m.LwM2mVersion;
2226
import org.eclipse.leshan.core.LwM2m.Version;
2327
import org.junit.jupiter.api.Test;
28+
import org.junit.jupiter.api.function.Executable;
29+
import org.junit.jupiter.params.ParameterizedTest;
30+
import org.junit.jupiter.params.provider.Arguments;
31+
import org.junit.jupiter.params.provider.MethodSource;
32+
33+
import nl.jqno.equalsverifier.EqualsVerifier;
2434

2535
public class VersionTest {
2636
@Test
@@ -33,11 +43,67 @@ public void is_supported_tests() {
3343
}
3444

3545
@Test
36-
public void compare_tests() {
46+
public void compare_to_tests() {
3747
assertTrue(new Version("1.0").compareTo(new Version("1.2")) < 0);
3848
assertTrue(new Version("0.9").compareTo(new Version("1.2")) < 0);
49+
assertTrue(new Version("128.0").compareTo(new Version("128.2")) < 0);
3950
assertTrue(new Version("1.2").compareTo(new Version("1.2")) == 0);
51+
assertTrue(new Version("128.0").compareTo(new Version("128.0")) == 0);
4052
assertTrue(new Version("1.3").compareTo(new Version("1.2")) > 0);
4153
assertTrue(new Version("2.0").compareTo(new Version("1.2")) > 0);
54+
assertTrue(new Version("128.2").compareTo(new Version("128.0")) > 0);
55+
}
56+
57+
@Test
58+
public void assertEqualsHashcode() {
59+
// TODO we should not use EqualsVerifier.simple()
60+
// but implement a right hashcode/equals way
61+
// see : https://github.com/eclipse-leshan/leshan/issues/1504
62+
EqualsVerifier.simple().forClass(Version.class).verify();
63+
}
64+
65+
@Test
66+
public void older_than_tests() {
67+
assertTrue(new Version("1.0").olderThan(new Version("1.1")));
68+
assertTrue(new Version("0.9").olderThan(new Version("1.1")));
69+
}
70+
71+
@Test
72+
public void newer_than_tests() {
73+
assertTrue(new Version("1.1").newerThan(new Version("1.0")));
74+
assertTrue(new Version("1.0").newerThan(new Version("0.9")));
75+
}
76+
77+
@ParameterizedTest
78+
@MethodSource("illegal_arguments")
79+
public void illegal_argument_tests(Executable executable, String expectedMessage) {
80+
IllegalArgumentException e = assertThrows(IllegalArgumentException.class, executable);
81+
assertEquals(expectedMessage, e.getMessage());
82+
}
83+
84+
private static Stream<Arguments> illegal_arguments() {
85+
return Stream.of(args(() -> new Version(null), "version MUST NOT be null or empty"),
86+
args(() -> new Version(""), "version MUST NOT be null or empty"),
87+
args(() -> new Version("foo"), "version (foo) MUST be composed of 2 parts"),
88+
args(() -> new Version("1.0.0"), "version (1.0.0) MUST be composed of 2 parts"),
89+
args(() -> new Version("-1.0"), "version (-1.0) part 1 (-1) must not be negative"),
90+
args(() -> new Version("1.-1"), "version (1.-1) part 2 (-1) must not be negative"),
91+
args(() -> new Version("a.0"), "version (a.0) part 1 (a) is not a valid short"),
92+
args(() -> new Version("32768.32767"), "version (32768.32767) part 1 (32768) is not a valid short"),
93+
args(() -> new Version("32767.32768"), "version (32767.32768) part 2 (32768) is not a valid short"),
94+
args(() -> new Version(-32769, -32768),
95+
"version (-32769.-32768) major part (-32769) is not a valid short"),
96+
args(() -> new Version(-32768, -32769),
97+
"version (-32768.-32769) minor part (-32769) is not a valid short"),
98+
args(() -> new Version(32768, 32767), "version (32768.32767) major part (32768) is not a valid short"),
99+
args(() -> new Version(32767, 32768), "version (32767.32768) minor part (32768) is not a valid short"),
100+
args(() -> new Version(-1, 0), "version (-1.0) major part (-1) must not be negative"),
101+
args(() -> new Version(1, -1), "version (1.-1) minor part (-1) must not be negative"),
102+
args(() -> new Version((short) -1, (short) 0), "version (-1.0) major part (-1) must not be negative"),
103+
args(() -> new Version((short) 1, (short) -1), "version (1.-1) minor part (-1) must not be negative"));
104+
}
105+
106+
private static Arguments args(Executable executable, String expectedMessage) {
107+
return Arguments.of(executable, expectedMessage);
42108
}
43109
}

0 commit comments

Comments
 (0)