-
Notifications
You must be signed in to change notification settings - Fork 9
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.
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
feat: add ApiVersion parser and comparison #189
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,120 @@ | ||
package com.google.docfx.doclet; | ||
|
||
import com.google.common.base.MoreObjects; | ||
import java.util.Objects; | ||
import java.util.Optional; | ||
import java.util.regex.Matcher; | ||
import java.util.regex.Pattern; | ||
import javax.annotation.Nullable; | ||
|
||
public class ApiVersion implements Comparable<ApiVersion> { | ||
public static ApiVersion NONE = new ApiVersion(0, 0, null, 0); | ||
|
||
private static final Pattern VALID_VERSION_REGEX = | ||
Pattern.compile("^v(\\d+)p?(\\d+)?(alpha|beta)?(\\d+)?"); | ||
|
||
/** | ||
* Creates an ApiVersion instance, if the given input matches the correct format. | ||
* | ||
* <p>Supported Format: | ||
* | ||
* <pre> | ||
* v1p2beta3 | ||
* │└┤└──┤│ | ||
* │ │ ││ | ||
* │ │ │└─── Optional: Prerelease major version | ||
* │ │ └──── Optional: Stability level. See <a href="https://google.aip.dev/181">AIP 181</a> | ||
* │ └──────── Optional: Minor version | ||
* └────────── Required: Major version | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. really helpful description! |
||
* </pre> | ||
* | ||
* @return Optional.empty() if the given input doesn't match the version pattern | ||
*/ | ||
public static Optional<ApiVersion> parse(@Nullable String input) { | ||
if (input != null) { | ||
Matcher matcher = VALID_VERSION_REGEX.matcher(input); | ||
if (matcher.matches()) { | ||
return Optional.of( | ||
new ApiVersion( | ||
safeParseInt(matcher.group(1)), | ||
safeParseInt(matcher.group(2)), | ||
matcher.group(3), | ||
safeParseInt(matcher.group(4)))); | ||
} | ||
} | ||
return Optional.empty(); | ||
} | ||
|
||
private static int safeParseInt(@Nullable String input) { | ||
if (input == null) { | ||
return 0; | ||
} | ||
return Integer.parseInt(input); | ||
} | ||
|
||
private final int major; | ||
private final int minor; | ||
private final String stability; | ||
private final int prerelease; | ||
|
||
private ApiVersion(int major, int minor, String stability, int prerelease) { | ||
this.major = major; | ||
this.minor = minor; | ||
this.stability = stability; | ||
this.prerelease = prerelease; | ||
} | ||
|
||
public boolean isStable() { | ||
return stability == null; | ||
} | ||
|
||
@Override | ||
public int compareTo(ApiVersion other) { | ||
if (major != other.major) { | ||
return major - other.major; | ||
} | ||
if (minor != other.minor) { | ||
return minor - other.minor; | ||
} | ||
if (!Objects.equals(stability, other.stability)) { | ||
if (stability == null) { | ||
return 1; // Other is a prerelease version, but this is not. | ||
} | ||
if (other.stability == null) { | ||
return -1; // This is a prerelease version, but the other is not. | ||
} | ||
return stability.compareTo(other.stability); // Based on alphabetical order | ||
} | ||
return prerelease - other.prerelease; | ||
} | ||
|
||
@Override | ||
public boolean equals(Object o) { | ||
if (this == o) { | ||
return true; | ||
} | ||
if (o == null || getClass() != o.getClass()) { | ||
return false; | ||
} | ||
ApiVersion other = (ApiVersion) o; | ||
return major == other.major | ||
&& minor == other.minor | ||
&& prerelease == other.prerelease | ||
&& Objects.equals(stability, other.stability); | ||
} | ||
|
||
@Override | ||
public int hashCode() { | ||
return Objects.hash(major, minor, stability, prerelease); | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return MoreObjects.toStringHelper(ApiVersion.class) | ||
.add("major", major) | ||
.add("minor", minor) | ||
.add("stability", stability) | ||
.add("prerelease", prerelease) | ||
.toString(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
package com.microsoft.util; | ||
|
||
import static com.google.common.truth.Truth.assertThat; | ||
import static org.junit.Assert.assertFalse; | ||
import static org.junit.Assert.assertTrue; | ||
|
||
import com.google.docfx.doclet.ApiVersion; | ||
import org.junit.Test; | ||
|
||
public class ApiVersionTest { | ||
|
||
@Test | ||
public void badFormat() { | ||
assertFalse(ApiVersion.parse("badFormat").isPresent()); | ||
assertFalse(ApiVersion.parse("1p1").isPresent()); | ||
assertFalse(ApiVersion.parse("v1p2p3").isPresent()); | ||
assertFalse(ApiVersion.parse("v1a").isPresent()); | ||
assertFalse(ApiVersion.parse("v1beta2p3").isPresent()); | ||
} | ||
|
||
@Test | ||
public void goodFormat() { | ||
assertTrue(ApiVersion.parse("v99").isPresent()); | ||
assertTrue(ApiVersion.parse("v1p2").isPresent()); | ||
assertTrue(ApiVersion.parse("v2alpha").isPresent()); | ||
assertTrue(ApiVersion.parse("v3beta").isPresent()); | ||
assertTrue(ApiVersion.parse("v3beta1").isPresent()); | ||
assertTrue(ApiVersion.parse("v3beta1").isPresent()); | ||
assertTrue(ApiVersion.parse("v3p1beta2").isPresent()); | ||
} | ||
|
||
@Test | ||
public void testAisGreaterThanB() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assume this use case would be used when determining which version to recommend? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Partially, yes. The exact logic is now available in PR #190 's ApiVersion#getRecommended method: https://github.com/googleapis/java-docfx-doclet/pull/190/files#diff-e1cb1f1754754c8682e5d8cee8c20a16573edf80ce3c7f7b4f2d08446f51511cR57-R70 |
||
String[][] comparisons = { | ||
{"v3", "v2"}, | ||
{"v1p1", "v1"}, | ||
{"v1", "v1alpha"}, | ||
{"v1", "v1beta"}, | ||
{"v1beta", "v1alpha"}, | ||
{"v1beta", "v1alpha2"}, | ||
{"v1beta1", "v1alpha"}, | ||
{"v1beta1", "v1beta"}, | ||
{"v1beta2", "v1beta1"}, | ||
{"v2p1alpha0", "v1p1alpha0"}, | ||
}; | ||
|
||
for (String[] testcase : comparisons) { | ||
ApiVersion a = parse(testcase[0]); | ||
ApiVersion b = parse(testcase[1]); | ||
assertThat(a).isGreaterThan(b); | ||
} | ||
} | ||
|
||
@Test | ||
public void stability() { | ||
assertTrue(parse("v12").isStable()); | ||
assertTrue(parse("v1p3").isStable()); | ||
|
||
assertFalse(parse("v1alpha").isStable()); | ||
assertFalse(parse("v1p1beta1").isStable()); | ||
} | ||
|
||
@Test | ||
public void equals() { | ||
assertThat(parse("v1")).isEqualTo(parse("v1p0")); | ||
assertThat(parse("v1p0")).isEqualTo(parse("v1")); | ||
assertThat(parse("v2p1beta")).isEqualTo(parse("v2p1beta0")); | ||
} | ||
|
||
private ApiVersion parse(String s) { | ||
return ApiVersion.parse(s).orElseThrow(() -> new IllegalStateException("Unable to parse " + s)); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know we use Guava widely in other projects so it might not apply here, but wondering if we want to highlight this additional dependency through the new dependency governance process? Or is this already a considered a "safe" dependency that any project is free to import?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CC'ing @suztomo to verify. Guava is an approved dependency to be used in sdk-platform-java, so I've used the same (latest) version here. This project doesn't use java-shared-dependencies... but perhaps we should change that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This java-docfx-doclet doesn't appear in library users class path. So its dependencies do not matter. I don't see a problem using the shared dependencies.