Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix version helper classes #1532

Closed
wants to merge 5 commits into from
Closed

Fix version helper classes #1532

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
44b16bc
fix version helper classes
gcx-seb Nov 3, 2023
30e11e7
implement PR feedback
gcx-seb Nov 6, 2023
6702066
remove duplicated test case
gcx-seb Nov 6, 2023
ea109ce
cleanup
gcx-seb Nov 7, 2023
c309c31
correct spelling
gcx-seb Nov 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 42 additions & 34 deletions leshan-core/src/main/java/org/eclipse/leshan/core/LwM2m.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,9 @@ public interface LwM2m {
*/
public class LwM2mVersion extends Version {

public static LwM2mVersion V1_0 = new LwM2mVersion("1.0", true);
public static LwM2mVersion V1_1 = new LwM2mVersion("1.1", true);
private static LwM2mVersion[] supportedVersions = new LwM2mVersion[] { V1_0, V1_1 };
public static final LwM2mVersion V1_0 = new LwM2mVersion("1.0", true);
public static final LwM2mVersion V1_1 = new LwM2mVersion("1.1", true);
private static final LwM2mVersion[] supportedVersions = new LwM2mVersion[] { V1_0, V1_1 };

private final boolean supported;

Expand Down Expand Up @@ -91,35 +91,49 @@ public boolean equals(Object obj) {
*/
public class Version implements Comparable<Version> {

public static Version V1_0 = new Version("1.0");
public static final Version MAX = new Version(Short.MAX_VALUE, Short.MIN_VALUE);
public static final Version V1_0 = new Version("1.0");
public static final Version MAX = new Version(Short.MAX_VALUE, Short.MAX_VALUE);

protected final Short major;
protected final Short minor;
private static short toShortExact(int value, String format, Object... args) {
if ((short) value != value) {
throw new IllegalArgumentException(String.format(format, args));
}
return (short) value;
}

protected final short major;
protected final short minor;

public Version(int major, int minor) {
this.major = (short) major;
this.minor = (short) minor;
this(
toShortExact(major, "version (%d.%d) major part (%d) is not a valid short",
major, minor, major),
toShortExact(minor, "version (%d.%d) minor part (%d) is not a valid short",
major, minor, minor)
);
}

public Version(Short major, Short minor) {
public Version(short major, short minor) {
this.major = major;
if (this.major < 0) {
throw new IllegalArgumentException(String.format("version (%d.%d) major part (%d) must not be negative",
major, minor, major));
}
this.minor = minor;
if (this.minor < 0) {
throw new IllegalArgumentException(String.format("version (%d.%d) minor part (%d) must not be negative",
major, minor, minor));
}
}

public Version(String version) {
try {
String[] versionPart = version.split("\\.");
this.major = Short.parseShort(versionPart[0]);
this.minor = Short.parseShort(versionPart[1]);
} catch (RuntimeException e) {
String err = Version.validate(version);
if (err != null) {
throw new IllegalArgumentException(err);
} else {
throw e;
}
String err = Version.validate(version);
if (err != null) {
throw new IllegalArgumentException(err);
}
String[] versionPart = version.split("\\.");
this.major = Short.parseShort(versionPart[0]);
this.minor = Short.parseShort(versionPart[1]);
}

@Override
Expand All @@ -133,16 +147,16 @@ public static Version getDefault() {

public static String validate(String version) {
if (version == null || version.isEmpty())
return "version MUST NOT be null or empty";
return "version MUST NOT be null or empty";
String[] versionPart = version.split("\\.");
if (versionPart.length != 2) {
return String.format("version (%s) MUST be composed of 2 parts", version);
}
for (int i = 0; i < 2; i++) {
try {
Short parsedShort = Short.parseShort(versionPart[i]);
short parsedShort = Short.parseShort(versionPart[i]);
if (parsedShort < 0) {
return String.format("version (%s) part %d (%s) is not a valid short", version, i + 1,
return String.format("version (%s) part %d (%s) must not be negative", version, i + 1,
versionPart[i]);
}
} catch (Exception e) {
Expand All @@ -157,8 +171,8 @@ public static String validate(String version) {
public int hashCode() {
final int prime = 31;
int result = 1;
result = prime * result + ((major == null) ? 0 : major.hashCode());
result = prime * result + ((minor == null) ? 0 : minor.hashCode());
result = prime * result + major;
result = prime * result + minor;
return result;
}

Expand All @@ -171,15 +185,9 @@ public boolean equals(Object obj) {
if (getClass() != obj.getClass())
return false;
Version other = (Version) obj;
if (major == null) {
if (other.major != null)
return false;
} else if (!major.equals(other.major))
if (major != other.major)
return false;
if (minor == null) {
if (other.minor != null)
return false;
} else if (!minor.equals(other.minor))
if (minor != other.minor)
return false;
return true;
}
Expand Down
67 changes: 64 additions & 3 deletions leshan-core/src/test/java/org/eclipse/leshan/core/VersionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,16 @@
*******************************************************************************/
package org.eclipse.leshan.core;

import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;

import org.eclipse.leshan.core.LwM2m.LwM2mVersion;
import org.eclipse.leshan.core.LwM2m.Version;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.function.Executable;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import java.util.stream.Stream;

import static org.junit.jupiter.api.Assertions.*;

public class VersionTest {
@Test
Expand All @@ -35,9 +39,66 @@ public void is_supported_tests() {
@Test
public void compare_tests() {
assertTrue(new Version("1.0").compareTo(new Version("1.2")) < 0);
assertTrue(new Version("1.0").olderThan(new Version("1.2")));
assertNotEquals(new Version("1.0").hashCode(), new Version("1.2").hashCode());
sbernard31 marked this conversation as resolved.
Show resolved Hide resolved

assertTrue(new Version("0.9").compareTo(new Version("1.2")) < 0);
assertTrue(new Version("0.9").olderThan(new Version("1.2")));
assertNotEquals(new Version("0.9"), new Version("1.2"));

assertTrue(new Version("128.0").compareTo(new Version("128.2")) < 0);
assertTrue(new Version("128.0").olderThan(new Version("128.2")));
assertNotEquals(new Version("128.0"), new Version("128.2"));

assertTrue(new Version("1.2").compareTo(new Version("1.2")) == 0);
assertEquals(new Version("1.2"), new Version("1.2"));
assertEquals(new Version("1.2").hashCode(), new Version("1.2").hashCode());

assertTrue(new Version("128.0").compareTo(new Version("128.0")) == 0);
assertEquals(new Version("128.0"), new Version("128.0"));
assertEquals(new Version("128.0").hashCode(), new Version("128.0").hashCode());

assertTrue(new Version("1.3").compareTo(new Version("1.2")) > 0);
assertTrue(new Version("1.3").newerThan(new Version("1.2")));

assertTrue(new Version("2.0").compareTo(new Version("1.2")) > 0);
assertTrue(new Version("2.0").newerThan(new Version("1.2")));

assertTrue(new Version("128.2").compareTo(new Version("128.0")) > 0);
assertTrue(new Version("128.2").newerThan(new Version("128.0")));
assertNotEquals(new Version("128.2"), new Version("128.0"));
}

@ParameterizedTest
@MethodSource("illegal_arguments")
public void illegal_argument_tests(Executable executable, String expectedMessage) {
IllegalArgumentException e = assertThrows(IllegalArgumentException.class, executable);
assertEquals(expectedMessage, e.getMessage());
}

private static Stream<Arguments> illegal_arguments() {
return Stream.of(
args(() -> new Version(null), "version MUST NOT be null or empty"),
args(() -> new Version(""), "version MUST NOT be null or empty"),
args(() -> new Version("foo"), "version (foo) MUST be composed of 2 parts"),
args(() -> new Version("1.0.0"), "version (1.0.0) MUST be composed of 2 parts"),
args(() -> new Version("-1.0"), "version (-1.0) part 1 (-1) must not be negative"),
args(() -> new Version("1.-1"), "version (1.-1) part 2 (-1) must not be negative"),
args(() -> new Version("a.0"), "version (a.0) part 1 (a) is not a valid short"),
args(() -> new Version("32768.32767"), "version (32768.32767) part 1 (32768) is not a valid short"),
args(() -> new Version("32767.32768"), "version (32767.32768) part 2 (32768) is not a valid short"),
args(() -> new Version(-32769, -32768), "version (-32769.-32768) major part (-32769) is not a valid short"),
args(() -> new Version(-32768, -32769), "version (-32768.-32769) minor part (-32769) is not a valid short"),
args(() -> new Version(32768, 32767), "version (32768.32767) major part (32768) is not a valid short"),
args(() -> new Version(32767, 32768), "version (32767.32768) minor part (32768) is not a valid short"),
args(() -> new Version(-1, 0), "version (-1.0) major part (-1) must not be negative"),
args(() -> new Version(1, -1), "version (1.-1) minor part (-1) must not be negative"),
args(() -> new Version((short) -1, (short) 0), "version (-1.0) major part (-1) must not be negative"),
args(() -> new Version((short) 1, (short) -1), "version (1.-1) minor part (-1) must not be negative")
);
}

private static Arguments args(Executable executable, String expectedMessage) {
return Arguments.of(executable, expectedMessage);
}
}