Refactor fetch_metadata

shard.lock

@@ -32,3 +32,7 @@ shards:
  github: crystal-lang/shards
  commit: 5666481ea1b304944adcf9eabce0b24ff9badeb9
 
+ webmock:
+ github: manastech/webmock.cr
+ commit: 78bb0e3b5850c700da0e7fbdd2d6c180cc4a061b
+

shard.yml

@@ -25,3 +25,8 @@ dependencies:
  github: smacker/libgit2.cr
  github-cr:
  github: arnavb/github-cr
+
+development_dependencies:
+ webmock:
+ github: manastech/webmock.cr
+ branch: master

spec/service/fetch_metadata_spec.cr

@@ -0,0 +1,60 @@
+require "spec"
+require "../../src/service/fetch_metadata"
+require "webmock"
+
+describe Service::FetchMetadata do
+ describe "#fetch_metadata" do
+ it "returns nil for git" do
+ service = Service::FetchMetadata.new(Repo::Ref.new("git", "foo"))
+ service.fetch_metadata.should be_nil
+ end
+
+ it "queries github graphql" do
+ WebMock.wrap do
+ WebMock.stub(:post, "https://api.github.com/graphql").to_return do |request|
+ body = request.body.not_nil!.gets_to_end
+ body.should contain %("variables":{"owner":"foo","name":"bar"})
+
+ HTTP::Client::Response.new(:ok, <<-JSON)
+ {
+ "data": {
+ "repository": {
+ "description": "foo bar baz"
+ }
+ }
+ }
+ JSON
+ end
+ service = Service::FetchMetadata.new(Repo::Ref.new("github", "foo/bar"))
+ service.api_token = ""
+ service.fetch_metadata.should eq Repo::Metadata.new(description: "foo bar baz")
+ end
+ end
+
+ it "raises when endpoint unavailable" do
+ WebMock.wrap do
+ WebMock.stub(:post, "https://api.github.com/graphql").to_return(status: 401)
+
+ service = Service::FetchMetadata.new(Repo::Ref.new("github", "foo/bar"))
+ service.api_token = ""
+
+ expect_raises(Service::FetchMetadata::Error, "Repository unavailable") do
+ service.fetch_metadata
+ end
+ end
+ end
+
+ it "raises when response is invalid" do
+ WebMock.wrap do
+ WebMock.stub(:post, "https://api.github.com/graphql").to_return(status: 200)
+
+ service = Service::FetchMetadata.new(Repo::Ref.new("github", "foo/bar"))
+ service.api_token = ""
+
+ expect_raises(Service::FetchMetadata::Error, "Invalid response") do
+ service.fetch_metadata
+ end
+ end
+ end
+ end
+end

spec/service/sync_repo_spec.cr

@@ -187,18 +187,25 @@ describe Service::SyncRepo do
  repo_ref = Repo::Ref.new("git", "foo")
  service = Service::SyncRepo.new(db, repo_ref)
 
- mock_resolver = MockResolver.new(metadata: Repo::Metadata.new(forks_count: 42))
- resolver = Repo::Resolver.new(mock_resolver, repo_ref)
  repo = Repo.new(repo_ref, nil, id: repo_id)
- service.sync_metadata(resolver, repo)
+ service.sync_metadata(repo, fetch_service: MockFetchMetadata.new(nil))
+ results = db.connection.query_all <<-SQL, as: {JSON::Any, Bool, Bool}
+ SELECT
+ metadata, synced_at > NOW() - interval '1s', sync_failed_at IS NOT NULL
+ FROM repos
+ SQL
+
+ results.should eq [{JSON.parse(%({})), false, true}]
+
+ service.sync_metadata(repo, fetch_service: MockFetchMetadata.new(Repo::Metadata.new(forks_count: 42)))
 
- results = db.connection.query_all <<-SQL, as: {JSON::Any, Bool, Time?}
+ results = db.connection.query_all <<-SQL, as: {JSON::Any, Bool, Bool}
  SELECT
- metadata, synced_at > NOW() - interval '1s', sync_failed_at
+ metadata, synced_at > NOW() - interval '1s', sync_failed_at IS NOT NULL
  FROM repos
  SQL
 
- results.should eq [{JSON.parse(%({"forks_count": 42})), true, nil}]
+ results.should eq [{JSON.parse(%({"forks_count": 42})), true, false}]
  end
  end
 end

spec/support/mock_resolver.cr

@@ -11,11 +11,11 @@ class MockResolver
 
  property? resolvable : Bool = true
 
- def self.new(versions : Hash(String, MockEntry) = {} of String => MockEntry, metadata : Repo::Metadata = Repo::Metadata.new)
- new(versions.transform_keys { |key| Shards::Version.new(key) }, metadata)
+ def self.new(versions : Hash(String, MockEntry) = {} of String => MockEntry)
+ new(versions.transform_keys { |key| Shards::Version.new(key) })
  end
 
- def initialize(@versions : Hash(Shards::Version, MockEntry), @metadata : Repo::Metadata = Repo::Metadata.new)
+ def initialize(@versions : Hash(Shards::Version, MockEntry))
  end
 
  def self.unresolvable
@@ -52,11 +52,6 @@ class MockResolver
  @versions[version].revision_info
  end
 
- def fetch_metadata
- raise Repo::Resolver::RepoUnresolvableError.new unless resolvable?
- @metadata
- end
-
  def latest_version_for_ref(ref)
  raise Repo::Resolver::RepoUnresolvableError.new unless resolvable?
  @versions.keys.last?
@@ -67,3 +62,16 @@ class MockResolver
  @versions[version].files[path]?
  end
 end
+
+struct MockFetchMetadata
+ def initialize(@metadata : Repo::Metadata?)
+ end
+
+ def fetch_metadata
+ if metadata = @metadata
+ metadata
+ else
+ raise Service::FetchMetadata::Error.new("Repo unavailable")
+ end
+ end
+end

src/repo/ref.cr

@@ -86,6 +86,12 @@ struct Repo::Ref
  File.basename(uri.path).rchop('/').rchop(".git")
  end
 
+ def owner
+ if provider_resolver?
+ Path.posix(url).dirname
+ end
+ end
+
  def nice_url
  return url if provider_resolver? || !resolvable?
 

src/repo/resolver.cr

@@ -1,7 +1,6 @@
 require "shards/logger"
 require "shards/package"
 require "../ext/shards/resolvers/git"
-require "../ext/shards/resolvers/github"
 require "../release"
 
 class Repo
@@ -48,12 +47,6 @@ class Repo
  @resolver.revision_info(Shards::Version.new(version))
  end
 
- def fetch_metadata : Repo::Metadata?
- if (resolver = @resolver).responds_to?(:fetch_metadata)
- resolver.fetch_metadata
- end
- end
-
  def latest_version_for_ref(ref) : String?
  @resolver.latest_version_for_ref(ref).try &.value
  end

src/ext/shards/resolvers/github.cr → src/service/fetch_metadata.cr

@@ -1,36 +1,43 @@
 require "json"
 require "http/client"
-require "../../../repo"
+require "../repo"
 
-class Shards::GithubResolver
- @@graphql_client : HTTP::Client?
-
- def self.graphql_client
- @@graphql_client ||= HTTP::Client.new("api.github.com", 443, true)
+struct Service::FetchMetadata
+ class Error < Exception
  end
 
- def self.api_token
- @@api_token ||= ENV["GITHUB_TOKEN"]
+ getter graphql_client : HTTP::Client { HTTP::Client.new("api.github.com", 443, true) }
+ property api_token : String { ENV["GITHUB_TOKEN"] }
+
+ private getter graphql_query : String = begin
+ {{ read_file("#{__DIR__}/fetch_metadata-github.graphql") }}
  end
 
- def fetch_metadata : Repo::Metadata
- self.class.fetch_metadata(dependency["github"])
+ def initialize(@repo_ref : Repo::Ref)
  end
 
- private def self.graphql_query : String
- {{ read_file("#{__DIR__}/github-repo-metadata.graphql") }}
+ def fetch_metadata
+ case @repo_ref.resolver
+ when "github"
+ fetch_metadata_github(@repo_ref.owner, @repo_ref.name)
+ else
+ nil
+ end
  end
 
- def self.fetch_metadata(path)
- owner, name = path.split("/")
+ def fetch_metadata_github(owner, name)
  body = {query: graphql_query, variables: {owner: owner, name: name}}
  response = graphql_client.post "/graphql", body: body.to_json, headers: HTTP::Headers{"Authorization" => "bearer #{api_token}"}
 
- raise Shards::Error.new("Repository unavailable") unless response.status_code == 200
+ raise Error.new("Repository unavailable") unless response.status_code == 200
 
- metadata = Repo::Metadata.from_github_graphql(response.body)
+ begin
+ metadata = Repo::Metadata.from_github_graphql(response.body)
+ rescue exc : JSON::ParseException
+ raise Error.new("Invalid response", cause: exc)
+ end
 
- raise Shards::Error.new("Invalid response") unless metadata
+ raise Error.new("Invalid response") unless metadata
 
  metadata
  end
@@ -46,7 +53,7 @@ struct Repo::Metadata
  pull.read_object do |key|
  if key == "repository"
  pull.read_null_or do
- metadata = new(github_pull: pull)
+ metadata = Repo::Metadata.new(github_pull: pull)
  end
  else
  pull.skip
@@ -59,7 +66,7 @@ struct Repo::Metadata
  errors << pull.read_string
  end
  end
- raise Shards::Error.new("Repository error: #{errors.join(", ")}")
+ raise Service::FetchMetadata::Error.new("Repository error: #{errors.join(", ")}")
  else
  pull.skip
  end

src/service/sync_repo.cr

@@ -2,6 +2,7 @@ require "../ext/yaml/any"
 require "../repo/resolver"
 require "./sync_release"
 require "./order_releases"
+require "./fetch_metadata"
 
 # This service synchronizes the information about a repository in the database.
 struct Service::SyncRepo
@@ -41,7 +42,7 @@ struct Service::SyncRepo
  end
  end
 
- sync_metadata(resolver, repo)
+ sync_metadata(repo)
  end
 
  def sync_releases(resolver, shard_id)
@@ -105,10 +106,10 @@ struct Service::SyncRepo
  end
  end
 
- def sync_metadata(resolver, repo : Repo)
+ def sync_metadata(repo : Repo, *, fetch_service = Service::FetchMetadata.new(repo.ref))
  begin
- metadata = resolver.fetch_metadata
- rescue exc : Shards::Error
+ metadata = fetch_service.fetch_metadata
+ rescue exc : Service::FetchMetadata::Error
  SyncRepo.sync_failed(@db, repo, "fetch_metadata_failed", exc)
 
  return