Skip to content

Commit

Permalink
#1877 URL checking fixed
Browse files Browse the repository at this point in the history
  • Loading branch information
yegor256 committed Feb 27, 2024
1 parent 78ffea4 commit e88ecab
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 38 deletions.
25 changes: 12 additions & 13 deletions src/main/java/com/rultor/agents/github/IssueUrl.java
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,13 @@
@Immutable
@ToString
@EqualsAndHashCode(of = "url")
@SuppressWarnings("PMD.ConstructorShouldDoInitialization")
final class IssueUrl {

/**
* Pattern for issue Url.
* Pattern for issue URL.
*/
private final Pattern correct =
Pattern.compile(".*/(?:issues|pull)/(\\d+)(?:/|$).*");
private static final Pattern CORRECT =
Pattern.compile("https://api.github.com/repos/[^/]+/[^/]+/(?:issues|pull)/(\\d+).*");

/**
* Url.
Expand All @@ -61,11 +60,7 @@ final class IssueUrl {
* Ctor.
* @param url Issue url.
*/
@SuppressWarnings("PMD.ConstructorOnlyInitializesOrCallOtherConstructors")
IssueUrl(final String url) {
if (url == null || url.isEmpty()) {
throw new IllegalArgumentException("URL should not be empty");
}
this.url = url;
}

Expand All @@ -74,14 +69,13 @@ final class IssueUrl {
* @return Issue id
* @checkstyle MethodNameCheck (10 lines)
*/
@SuppressWarnings("PMD.ShortMethodName")
public int id() {
final Matcher matcher = this.correct.matcher(this.url);
public int uid() {
final Matcher matcher = IssueUrl.CORRECT.matcher(this.url);
if (matcher.matches()) {
return Integer.parseInt(matcher.group(1));
}
throw new IllegalStateException(
String.format("Url %s is not valid issue url", this.url)
String.format("URL %s is not valid issue url", this.url)
);
}

Expand All @@ -90,6 +84,11 @@ public int id() {
* @return True if valid
*/
public boolean valid() {
return this.correct.matcher(this.url).matches();
if (this.url == null || this.url.isEmpty()) {
throw new IllegalArgumentException(
"URL should not be empty"
);
}
return IssueUrl.CORRECT.matcher(this.url).matches();
}
}
7 changes: 2 additions & 5 deletions src/main/java/com/rultor/agents/github/StartsTalks.java
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,7 @@ public void execute(final Talks talks) throws IOException {
}
if (!new IssueUrl(url).valid()) {
++skipped;
Logger.info(
this, "Skipped, since not valid URL at %s",
reason, url
);
Logger.info(this, "Skipped, since not valid URL at %s", url);
continue;
}
names.add(this.activate(talks, event));
Expand Down Expand Up @@ -144,7 +141,7 @@ private String activate(final Talks talks, final JsonObject event)
throws IOException {
final Coordinates coords = this.coords(event);
final Issue issue = this.github.repos().get(coords).issues().get(
new IssueUrl(event.getJsonObject("subject").getString("url")).id()
new IssueUrl(event.getJsonObject("subject").getString("url")).uid()
);
final String name = String.format("%s#%d", coords, issue.number());
if (!talks.exists(name)) {
Expand Down
24 changes: 4 additions & 20 deletions src/test/java/com/rultor/agents/github/IssueUrlTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ void pullRequestIdShouldBeReturned() {
"https://api.github.com/repos/USER/REPO/pull/5186"
);
MatcherAssert.assertThat(
issue.id(),
issue.uid(),
Matchers.is(5186)
);
}
Expand All @@ -100,7 +100,7 @@ void issueIdShouldBeReturned() {
"https://api.github.com/repos/USER/REPO/issues/5782"
);
MatcherAssert.assertThat(
issue.id(),
issue.uid(),
Matchers.is(5782)
);
}
Expand All @@ -111,7 +111,7 @@ void pullRequestIdFromReviewUrlShouldBeValid() {
"https://api.github.com/repos/USER/REPO/pull/5886/files#r123"
);
MatcherAssert.assertThat(
issue.id(),
issue.uid(),
Matchers.is(5886)
);
}
Expand All @@ -123,23 +123,7 @@ void commitIdShouldNotBeReturned() {
);
Assertions.assertThrows(
IllegalStateException.class,
issue::id
);
}

@Test
void urlShouldNotBeEmpty() {
Assertions.assertThrows(
IllegalArgumentException.class,
() -> new IssueUrl("")
);
}

@Test
void urlShouldNotBeNull() {
Assertions.assertThrows(
IllegalArgumentException.class,
() -> new IssueUrl(null)
issue::uid
);
}
}

0 comments on commit e88ecab

Please sign in to comment.