Merge pull request #1315 from sahansera/refactor-search-forks

Refactor GHContentSearchBuilder to allow a Fork type

Author: bitwiseman

src/main/java/org/kohsuke/github/GHContentSearchBuilder.java

@@ -12,13 +12,23 @@ public class GHContentSearchBuilder extends GHSearchBuilder<GHContent> {
     }
 
     /**
-     * Search terms.
+     * {@inheritDoc}
      */
+    @Override
     public GHContentSearchBuilder q(String term) {
         super.q(term);
         return this;
     }
 
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    GHContentSearchBuilder q(String qualifier, String value) {
+        super.q(qualifier, value);
+        return this;
+    }
+
     /**
      * In gh content search builder.
      *
@@ -47,9 +57,27 @@ public GHContentSearchBuilder language(String v) {
      * @param v
      *            the v
      * @return the gh content search builder
+     * @deprecated use {@link #fork(GHFork)}.
      */
+    @Deprecated
     public GHContentSearchBuilder fork(String v) {
-        return q("fork:" + v);
+        return q("fork", v);
+    }
+
+    /**
+     * Fork gh content search builder.
+     *
+     * @param fork
+     *            search mode for forks
+     *
+     * @return the gh content search builder
+     *
+     * @see <a href=
+     *      "https://docs.github.com/en/github/searching-for-information-on-github/searching-on-github/searching-in-forks">Searching
+     *      in forks</a>
+     */
+    public GHContentSearchBuilder fork(GHFork fork) {
+        return q("fork", fork.toString());
     }
 
     /**

src/main/java/org/kohsuke/github/GHFork.java

@@ -0,0 +1,38 @@
+package org.kohsuke.github;
+
+/**
+ * The enum for Fork search mode
+ */
+public enum GHFork {
+
+    /**
+     * Search in the parent repository and in forks with more stars than the parent repository.
+     *
+     * Forks with the same or fewer stars than the parent repository are still ignored.
+     */
+    PARENT_AND_FORKS("true"),
+
+    /**
+     * Search only in forks with more stars than the parent repository.
+     *
+     * The parent repository is ignored. If no forks have more stars than the parent, no results will be returned.
+     */
+    FORKS_ONLY("only"),
+
+    /**
+     * (Default) Search only the parent repository.
+     *
+     * Forks are ignored.
+     */
+    PARENT_ONLY("");
+
+    private String filterMode;
+    GHFork(final String mode) {
+        this.filterMode = mode;
+    }
+
+    @Override
+    public String toString() {
+        return filterMode;
+    }
+}

src/main/java/org/kohsuke/github/GHRepositorySearchBuilder.java

@@ -12,13 +12,23 @@ public class GHRepositorySearchBuilder extends GHSearchBuilder<GHRepository> {
     }
 
     /**
-     * Search terms.
+     * {@inheritDoc}
      */
+    @Override
     public GHRepositorySearchBuilder q(String term) {
         super.q(term);
         return this;
     }
 
+    /**
+     * {@inheritDoc}
+     */
+    @Override
+    GHRepositorySearchBuilder q(String qualifier, String value) {
+        super.q(qualifier, value);
+        return this;
+    }
+
     /**
      * In gh repository search builder.
      *
@@ -47,9 +57,11 @@ public GHRepositorySearchBuilder size(String v) {
      * @param v
      *            the v
      * @return the gh repository search builder
+     * @deprecated use {@link #fork(GHFork)} instead.
      */
+    @Deprecated
     public GHRepositorySearchBuilder forks(String v) {
-        return q("forks:" + v);
+        return q("fork", v);
     }
 
     /**
@@ -74,15 +86,39 @@ public GHRepositorySearchBuilder forks(String v) {
      * @see <a href=
      *      "https://docs.github.com/en/github/searching-for-information-on-github/searching-on-github/searching-in-forks">Searching
      *      in forks</a>
-     *
+     * @deprecated use {@link #fork(GHFork)} instead.
      */
+    @Deprecated
     public GHRepositorySearchBuilder fork(Fork fork) {
-        if (Fork.PARENT_ONLY.equals(fork)) {
-            this.terms.removeIf(term -> term.contains("fork:"));
-            return this;
-        }
+        return q("fork", fork.toString());
+    }
 
-        return q("fork:" + fork);
+    /**
+     * Searching in forks
+     *
+     * The default search mode is {@link Fork#PARENT_ONLY}. In that mode, forks are not included in search results.
+     *
+     * <p>
+     * Passing {@link Fork#PARENT_AND_FORKS} or {@link Fork#FORKS_ONLY} will show results from forks, but only if they
+     * have more stars than the parent repository.
+     *
+     * <p>
+     * IMPORTANT: Regardless of this setting, no search results will ever be returned for forks with equal or fewer
+     * stars than the parent repository. Forks with less stars than the parent repository are not included in the index
+     * for code searching.
+     *
+     * @param fork
+     *            search mode for forks
+     *
+     * @return the gh repository search builder
+     *
+     * @see <a href=
+     *      "https://docs.github.com/en/github/searching-for-information-on-github/searching-on-github/searching-in-forks">Searching
+     *      in forks</a>
+     *
+     */
+    public GHRepositorySearchBuilder fork(GHFork fork) {
+        return q("fork", fork.toString());
     }
 
     /**
@@ -217,8 +253,11 @@ public enum Sort {
     }
 
     /**
-     * The enum for Fork search mode
+     * The enum for Fork search mode.
+     *
+     * @deprecated Kept for backward compatibility. Use {@link GHFork} instead.
      */
+    @Deprecated
     public enum Fork {
 
         /**
@@ -243,10 +282,11 @@ public enum Fork {
         PARENT_ONLY("");
 
         private String filterMode;
-        Fork(String mode) {
+        Fork(final String mode) {
             this.filterMode = mode;
         }
 
+        @Override
         public String toString() {
             return filterMode;
         }

src/main/java/org/kohsuke/github/GHSearchBuilder.java

@@ -5,6 +5,9 @@
 import java.util.ArrayList;
 import java.util.List;
 
+import javax.annotation.CheckForNull;
+import javax.annotation.Nonnull;
+
 /**
  * Base class for various search builders.
  *
@@ -39,6 +42,30 @@ public GHQueryBuilder<T> q(String term) {
         return this;
     }
 
+    /**
+     * Add a search term with qualifier.
+     *
+     * If {@code value} is empty or {@code null}, all terms with the current qualifier will be removed.
+     *
+     * @param qualifier
+     *            the qualifier for this term
+     * @param value
+     *            the value for this term. If empty or null, all terms with the current qualifier will be removed.
+     * @return the gh query builder
+     */
+    GHQueryBuilder<T> q(@Nonnull final String qualifier, @CheckForNull final String value) {
+        if (StringUtils.isEmpty(qualifier)) {
+            throw new IllegalArgumentException("qualifier cannot be null or empty");
+        }
+        if (StringUtils.isEmpty(value)) {
+            final String removeQualifier = qualifier + ":";
+            terms.removeIf(term -> term.startsWith(removeQualifier));
+        } else {
+            terms.add(qualifier + ":" + value);
+        }
+        return this;
+    }
+
     /**
      * Performs the search.
      */

src/test/java/org/kohsuke/github/GHRepositoryTest.java

@@ -424,7 +424,7 @@ public void searchAllPublicAndForkedRepos() throws IOException {
         PagedSearchIterable<GHRepository> list = gitHub.searchRepositories()
                 .user("t0m4uk1991")
                 .visibility(GHRepository.Visibility.PUBLIC)
-                .fork(GHRepositorySearchBuilder.Fork.PARENT_AND_FORKS)
+                .fork(GHFork.PARENT_AND_FORKS)
                 .list();
         List<GHRepository> u = list.toList();
         assertThat(u.size(), is(14));
@@ -437,7 +437,7 @@ public void searchForPublicForkedOnlyRepos() throws IOException {
         PagedSearchIterable<GHRepository> list = gitHub.searchRepositories()
                 .user("t0m4uk1991")
                 .visibility(GHRepository.Visibility.PUBLIC)
-                .fork(GHRepositorySearchBuilder.Fork.FORKS_ONLY)
+                .fork(GHFork.FORKS_ONLY)
                 .list();
         List<GHRepository> u = list.toList();
         assertThat(u.size(), is(2));
@@ -467,6 +467,22 @@ public void ghRepositorySearchBuilderIgnoresUnknownVisibility() {
     @Test
     public void ghRepositorySearchBuilderForkDefaultResetForksSearchTerms() {
         GHRepositorySearchBuilder ghRepositorySearchBuilder = new GHRepositorySearchBuilder(gitHub);
+
+        ghRepositorySearchBuilder = ghRepositorySearchBuilder.fork(GHFork.PARENT_AND_FORKS);
+        assertThat(ghRepositorySearchBuilder.terms.stream().filter(item -> item.contains("fork:true")).count(), is(1L));
+        assertThat(ghRepositorySearchBuilder.terms.stream().filter(item -> item.contains("fork:")).count(), is(1L));
+
+        ghRepositorySearchBuilder = ghRepositorySearchBuilder.fork(GHFork.FORKS_ONLY);
+        assertThat(ghRepositorySearchBuilder.terms.stream().filter(item -> item.contains("fork:only")).count(), is(1L));
+        assertThat(ghRepositorySearchBuilder.terms.stream().filter(item -> item.contains("fork:")).count(), is(2L));
+
+        ghRepositorySearchBuilder = ghRepositorySearchBuilder.fork(GHFork.PARENT_ONLY);
+        assertThat(ghRepositorySearchBuilder.terms.stream().filter(item -> item.contains("fork:")).count(), is(0L));
+    }
+
+    @Test
+    public void ghRepositorySearchBuilderForkDeprecatedEnum() {
+        GHRepositorySearchBuilder ghRepositorySearchBuilder = new GHRepositorySearchBuilder(gitHub);
         ghRepositorySearchBuilder = ghRepositorySearchBuilder.fork(GHRepositorySearchBuilder.Fork.PARENT_AND_FORKS);
         assertThat(ghRepositorySearchBuilder.terms.stream().filter(item -> item.contains("fork:true")).count(), is(1L));
         assertThat(ghRepositorySearchBuilder.terms.stream().filter(item -> item.contains("fork:")).count(), is(1L));
@@ -479,6 +495,21 @@ public void ghRepositorySearchBuilderForkDefaultResetForksSearchTerms() {
         assertThat(ghRepositorySearchBuilder.terms.stream().filter(item -> item.contains("fork:")).count(), is(0L));
     }
 
+    @Test
+    public void ghRepositorySearchBuilderForkDeprecatedString() {
+        GHRepositorySearchBuilder ghRepositorySearchBuilder = new GHRepositorySearchBuilder(gitHub);
+        ghRepositorySearchBuilder = ghRepositorySearchBuilder.forks(GHFork.PARENT_AND_FORKS.toString());
+        assertThat(ghRepositorySearchBuilder.terms.stream().filter(item -> item.contains("fork:true")).count(), is(1L));
+        assertThat(ghRepositorySearchBuilder.terms.stream().filter(item -> item.contains("fork:")).count(), is(1L));
+
+        ghRepositorySearchBuilder = ghRepositorySearchBuilder.forks(GHFork.FORKS_ONLY.toString());
+        assertThat(ghRepositorySearchBuilder.terms.stream().filter(item -> item.contains("fork:only")).count(), is(1L));
+        assertThat(ghRepositorySearchBuilder.terms.stream().filter(item -> item.contains("fork:")).count(), is(2L));
+
+        ghRepositorySearchBuilder = ghRepositorySearchBuilder.forks(null);
+        assertThat(ghRepositorySearchBuilder.terms.stream().filter(item -> item.contains("fork:")).count(), is(0L));
+    }
+
     @Test
     public void listCommitCommentsSomeComments() throws IOException {
         List<GHCommitComment> commitComments = getRepository()

src/test/java/org/kohsuke/github/GitHubTest.java

@@ -1,6 +1,7 @@
 package org.kohsuke.github;
 
 import com.google.common.collect.Iterables;
+import org.junit.Assert;
 import org.junit.Test;
 import org.kohsuke.github.example.dataobject.ReadOnlyObjects;
 
@@ -138,20 +139,69 @@ public void searchContent() throws Exception {
         assertThat(c3.getPath(), not(equalTo(c2.getPath())));
         assertThat(r3.getTotalCount(), equalTo(r2.getTotalCount()));
 
-        PagedSearchIterable<GHContent> r4 = gitHub.searchContent()
+        GHContentSearchBuilder searchBuilder = gitHub.searchContent()
                 .q("addClass")
                 .in("file")
                 .language("js")
                 .repo("jquery/jquery")
                 .sort(GHContentSearchBuilder.Sort.INDEXED)
-                .order(GHDirection.DESC)
-                .list();
+                .order(GHDirection.DESC);
+
+        PagedSearchIterable<GHContent> r4 = searchBuilder.list();
 
         GHContent c4 = r4.iterator().next();
         assertThat(c4.getPath(), not(equalTo(c2.getPath())));
         assertThat(c4.getPath(), not(equalTo(c3.getPath())));
         assertThat(r4.getTotalCount(), equalTo(r2.getTotalCount()));
 
+        // Verify qualifier not allowed to be empty
+        IllegalArgumentException e = Assert.assertThrows(IllegalArgumentException.class,
+                () -> searchBuilder.q("", "not valid"));
+        assertThat(e.getMessage(), equalTo("qualifier cannot be null or empty"));
+    }
+
+    @Test
+    public void searchContentWithForks() {
+        final PagedSearchIterable<GHContent> results = gitHub.searchContent()
+                .q("addClass")
+                .language("js")
+                .sort(GHContentSearchBuilder.Sort.INDEXED)
+                .order(GHDirection.DESC)
+                .fork(GHFork.PARENT_ONLY)
+                .list();
+
+        final PagedSearchIterable<GHContent> resultsWithForks = gitHub.searchContent()
+                .q("addClass")
+                .language("js")
+                .sort(GHContentSearchBuilder.Sort.INDEXED)
+                .order(GHDirection.DESC)
+                .fork(GHFork.PARENT_AND_FORKS)
+                .list();
+
+        assertThat(results.getTotalCount(), lessThan(resultsWithForks.getTotalCount()));
+
+        // Do not record these.
+        // This will verify that the queries for the deprecated path are identical to the ones above.
+        if (!mockGitHub.isTakeSnapshot()) {
+            final PagedSearchIterable<GHContent> resultsDeprecated = gitHub.searchContent()
+                    .q("addClass")
+                    .language("js")
+                    .sort(GHContentSearchBuilder.Sort.INDEXED)
+                    .order(GHDirection.DESC)
+                    .fork(GHFork.PARENT_ONLY.toString())
+                    .list();
+
+            final PagedSearchIterable<GHContent> resultsWithForksDeprecated = gitHub.searchContent()
+                    .q("addClass")
+                    .language("js")
+                    .sort(GHContentSearchBuilder.Sort.INDEXED)
+                    .order(GHDirection.DESC)
+                    .fork(GHFork.PARENT_AND_FORKS.toString())
+                    .list();
+
+            assertThat(resultsDeprecated.getTotalCount(), equalTo(results.getTotalCount()));
+            assertThat(resultsWithForksDeprecated.getTotalCount(), equalTo(resultsWithForks.getTotalCount()));
+        }
     }
 
     @Test