Skip to content

Add Search.repository endpoint#25

Merged
ChristopherDavenport merged 6 commits intodavenverse:masterfrom
timo-schmid:add-repo-search
Oct 2, 2019
Merged

Add Search.repository endpoint#25
ChristopherDavenport merged 6 commits intodavenverse:masterfrom
timo-schmid:add-repo-search

Conversation

@timo-schmid
Copy link
Copy Markdown
Contributor

@timo-schmid timo-schmid commented Oct 2, 2019

Adds a new endpoint to search for repositories.

Some test code I've used to test:

package io.chrisdavenport.github

import cats.Show
import cats.effect.{ExitCode, IO, IOApp}
import cats.implicits._
import io.chrisdavenport.github.data.Repositories.Repo
import io.chrisdavenport.github.data.SearchResult
import io.chrisdavenport.github.data.SearchResult.Order.{Ascending, Descending}
import io.chrisdavenport.github.data.SearchResult.Sort.{Stars, Updated}
import io.chrisdavenport.github.data.SearchResult.{Order, Sort}
import org.http4s.client.Client
import org.http4s.client.blaze.BlazeClientBuilder

object Search extends IOApp {

  val auth: Option[Auth] = Some(OAuth("[insert-api-key]"))

  override def run(args: List[String]): IO[ExitCode] =
    BlazeClientBuilder[IO](scala.concurrent.ExecutionContext.global)
      .resource
      .use(testRepositorySearch) *> ExitCode.Success.pure[IO]

  private def testRepositorySearch(client: Client[IO]): IO[Unit] =
    List[(String, Option[Sort], Option[Order])](
      ("svalidate user:timo-schmid", None, None),
      ("user:timo-schmid", Some(Stars), Some(Descending)),
      ("user:timo-schmid", Some(Updated), Some(Ascending))
    ).traverse_(repositorySearch(client))

  private def repositorySearch(client: Client[IO])(params: (String, Option[Sort], Option[Order])): IO[Unit] =
    endpoints.Search
      .repository[IO](params._1, params._2, params._3, auth)
      .run(client)
      .flatMap { result =>
        IO(println(result.show))
      }

  private implicit val showRepo: Show[Repo] =
    repo => s"- ${repo.updatedAt.get} ${repo.stargazersCount}${repo.owner.login}/${repo.name}"

  implicit def showSearchResult[A](implicit showA: Show[A]): Show[SearchResult[A]] =
    searchResult =>
      (List(
        "Search Results:",
        s"- total_count: ${searchResult.totalCount}",
        s"- incomplete_results: ${searchResult.incompleteResults}",
      ) ++ searchResult.items.map(_.show)).mkString("\n")

}

Co-Authored-By: Christopher Davenport <chris@christopherdavenport.tech>

object Search {

def repository[F[_]: Sync](
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can you link to the appropriate endpoint documentation for easy debugging in the future?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yep, will do!

@ChristopherDavenport
Copy link
Copy Markdown
Collaborator

I would love a unit test on the endpoint generating the request you expect, but understand and don't always live up to that myself.

@timo-schmid
Copy link
Copy Markdown
Contributor Author

Makes sense, I'll add one!


"Search" should {

"return a valid search result" in {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hope this is ok. It's just testing the parsing, but not the passed params etc.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Something is better to nothing.

.stream

private val GITHUB_URI: Uri = uri"https://api.github.com"
private val GITHUB_URI: Uri = uri"https://api.github.com/"
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Turns out this didn't work with relative urls anymore (probably due to the missing slash).
Adding the slash at the end fixes it.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

kk. Expect that will then be required as well for enterprise versions of that uri. Things to note when we get a bug report.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, in fact I remember I had to add the / at the end of the uri in the enterprise version to make it work.

@ChristopherDavenport ChristopherDavenport merged commit 6db0b11 into davenverse:master Oct 2, 2019
@timo-schmid timo-schmid mentioned this pull request Oct 2, 2019
@timo-schmid timo-schmid deleted the add-repo-search branch October 2, 2019 21:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants