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.

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

Merged
merged 3 commits into from
Aug 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions third_party/docfx-doclet-143274/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,11 @@
<artifactId>jsoup</artifactId>
<version>1.16.1</version>
</dependency>
<dependency>
<groupId>com.google.guava</groupId>
<artifactId>guava</artifactId>
<version>32.1.1-jre</version>
</dependency>
Copy link
Contributor

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?

Copy link
Member Author

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?

Copy link
Member

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.


<!-- Test dependencies -->
<dependency>
Expand Down
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
Copy link
Contributor

Choose a reason for hiding this comment

The 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() {
Copy link
Contributor

Choose a reason for hiding this comment

The 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?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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));
}
}