diff --git a/api/queries_repo.go b/api/queries_repo.go index 913307765..3b31d8efe 100644 --- a/api/queries_repo.go +++ b/api/queries_repo.go @@ -6,16 +6,18 @@ import ( "fmt" "sort" "strings" + "time" "github.com/cli/cli/internal/ghrepo" ) // Repository contains information about a GitHub repo type Repository struct { - ID string - Name string - CloneURL string - Owner RepositoryOwner + ID string + Name string + CloneURL string + CreatedAt time.Time + Owner RepositoryOwner IsPrivate bool HasIssuesEnabled bool @@ -211,10 +213,11 @@ func RepoNetwork(client *Client, repos []ghrepo.Interface) (RepoNetworkResult, e // repositoryV3 is the repository result from GitHub API v3 type repositoryV3 struct { - NodeID string - Name string - CloneURL string `json:"clone_url"` - Owner struct { + NodeID string + Name string + CreatedAt time.Time `json:"created_at"` + CloneURL string `json:"clone_url"` + Owner struct { Login string } } @@ -230,9 +233,10 @@ func ForkRepo(client *Client, repo ghrepo.Interface) (*Repository, error) { } return &Repository{ - ID: result.NodeID, - Name: result.Name, - CloneURL: result.CloneURL, + ID: result.NodeID, + Name: result.Name, + CloneURL: result.CloneURL, + CreatedAt: result.CreatedAt, Owner: RepositoryOwner{ Login: result.Owner.Login, }, diff --git a/command/repo.go b/command/repo.go index 98b589b1b..c8db87266 100644 --- a/command/repo.go +++ b/command/repo.go @@ -5,6 +5,7 @@ import ( "net/url" "os" "strings" + "time" "github.com/cli/cli/api" "github.com/cli/cli/git" @@ -84,6 +85,10 @@ func isURL(arg string) bool { return strings.HasPrefix(arg, "http:/") || strings.HasPrefix(arg, "https:/") } +var Since = func(t time.Time) time.Duration { + return time.Since(t) +} + func repoFork(cmd *cobra.Command, args []string) error { ctx := contextForCommand(cmd) @@ -148,23 +153,25 @@ func repoFork(cmd *cobra.Command, args []string) error { } possibleFork := ghrepo.New(authLogin, toFork.RepoName()) - exists, err := api.GitHubRepoExists(apiClient, possibleFork) - if err != nil { - s.Stop() - return fmt.Errorf("problem with API request: %w", err) - } - - if exists { - s.Stop() - fmt.Fprintf(out, redX+" ") - return fmt.Errorf("%s already exists", utils.Bold(ghrepo.FullName(possibleFork))) - } forkedRepo, err := api.ForkRepo(apiClient, toFork) if err != nil { s.Stop() return fmt.Errorf("failed to fork: %w", err) } + + // This is weird. There is not an efficient way to determine via the GitHub API whether or not a + // given user has forked a given repo. We noticed, also, that the create fork API endpoint just + // returns the fork repo data even if it already exists -- with no change in status code or + // anything. We thus check the created time to see if the repo is brand new or not; if it's not, + // we assume the fork already existed and report an error. + created_ago := Since(forkedRepo.CreatedAt) + if created_ago > time.Minute { + s.Stop() + fmt.Fprintf(out, redX+" ") + return fmt.Errorf("%s already exists", utils.Bold(ghrepo.FullName(possibleFork))) + } + s.Stop() fmt.Fprintf(out, "%s Created fork %s\n", greenCheck, utils.Bold(ghrepo.FullName(forkedRepo))) diff --git a/command/repo_test.go b/command/repo_test.go index 402c27aba..0a72dd2bd 100644 --- a/command/repo_test.go +++ b/command/repo_test.go @@ -5,6 +5,7 @@ import ( "regexp" "strings" "testing" + "time" "github.com/cli/cli/context" "github.com/cli/cli/utils" @@ -23,7 +24,7 @@ func TestRepoFork_already_forked(t *testing.T) { } http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") - defer http.StubWithFixture(200, "repo.json")() + defer http.StubWithFixture(200, "forkResult.json")() _, err := RunCommand(repoForkCmd, "repo fork --remote=false") if err == nil { @@ -34,11 +35,21 @@ func TestRepoFork_already_forked(t *testing.T) { } } +func stubSince(d time.Duration) func() { + originalSince := Since + Since = func(t time.Time) time.Duration { + return d + } + return func() { + Since = originalSince + } +} + func TestRepoFork_in_parent(t *testing.T) { initBlankContext("OWNER/REPO", "master") + defer stubSince(2 * time.Second)() http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") - defer http.StubWithFixture(200, "repoNotFound.json")() defer http.StubWithFixture(200, "forkResult.json")() output, err := RunCommand(repoForkCmd, "repo fork --remote=false") @@ -75,8 +86,8 @@ func TestRepoFork_outside(t *testing.T) { } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + defer stubSince(2 * time.Second)() http := initFakeHTTP() - defer http.StubWithFixture(200, "repoNotFound.json")() defer http.StubWithFixture(200, "forkResult.json")() output, err := RunCommand(repoForkCmd, tt.args) @@ -101,9 +112,9 @@ func TestRepoFork_outside(t *testing.T) { func TestRepoFork_in_parent_yes(t *testing.T) { initBlankContext("OWNER/REPO", "master") + defer stubSince(2 * time.Second)() http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") - defer http.StubWithFixture(200, "repoNotFound.json")() defer http.StubWithFixture(200, "forkResult.json")() var seenCmds []*exec.Cmd @@ -141,8 +152,8 @@ func TestRepoFork_in_parent_yes(t *testing.T) { } func TestRepoFork_outside_yes(t *testing.T) { + defer stubSince(2 * time.Second)() http := initFakeHTTP() - defer http.StubWithFixture(200, "repoNotFound.json")() defer http.StubWithFixture(200, "forkResult.json")() var seenCmd *exec.Cmd @@ -173,8 +184,8 @@ func TestRepoFork_outside_yes(t *testing.T) { } func TestRepoFork_outside_survey_yes(t *testing.T) { + defer stubSince(2 * time.Second)() http := initFakeHTTP() - defer http.StubWithFixture(200, "repoNotFound.json")() defer http.StubWithFixture(200, "forkResult.json")() var seenCmd *exec.Cmd @@ -212,8 +223,8 @@ func TestRepoFork_outside_survey_yes(t *testing.T) { } func TestRepoFork_outside_survey_no(t *testing.T) { + defer stubSince(2 * time.Second)() http := initFakeHTTP() - defer http.StubWithFixture(200, "repoNotFound.json")() defer http.StubWithFixture(200, "forkResult.json")() cmdRun := false @@ -251,9 +262,9 @@ func TestRepoFork_outside_survey_no(t *testing.T) { func TestRepoFork_in_parent_survey_yes(t *testing.T) { initBlankContext("OWNER/REPO", "master") + defer stubSince(2 * time.Second)() http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") - defer http.StubWithFixture(200, "repoNotFound.json")() defer http.StubWithFixture(200, "forkResult.json")() var seenCmds []*exec.Cmd @@ -299,9 +310,9 @@ func TestRepoFork_in_parent_survey_yes(t *testing.T) { func TestRepoFork_in_parent_survey_no(t *testing.T) { initBlankContext("OWNER/REPO", "master") + defer stubSince(2 * time.Second)() http := initFakeHTTP() http.StubRepoResponse("OWNER", "REPO") - defer http.StubWithFixture(200, "repoNotFound.json")() defer http.StubWithFixture(200, "forkResult.json")() cmdRun := false diff --git a/test/fixtures/forkResult.json b/test/fixtures/forkResult.json index 3b6ab671b..e3f885642 100644 --- a/test/fixtures/forkResult.json +++ b/test/fixtures/forkResult.json @@ -2,6 +2,7 @@ "node_id": "123", "name": "REPO", "clone_url": "https://github.com/someone/repo.git", + "created_at": "2011-01-26T19:01:12Z", "owner": { "login": "someone" } diff --git a/test/fixtures/repo.json b/test/fixtures/repo.json deleted file mode 100644 index 893c1f2c2..000000000 --- a/test/fixtures/repo.json +++ /dev/null @@ -1,7 +0,0 @@ -{ - "data": { - "repository": { - "id": "MDEwOlJlcG9zaXRvcnkyMTI2MTMwNDk=" - } - } -} diff --git a/test/fixtures/repoNotFound.json b/test/fixtures/repoNotFound.json deleted file mode 100644 index ac6ed2adc..000000000 --- a/test/fixtures/repoNotFound.json +++ /dev/null @@ -1,20 +0,0 @@ -{ - "data": { - "repository": null - }, - "errors": [ - { - "type": "NOT_FOUND", - "path": [ - "repository" - ], - "locations": [ - { - "line": 2, - "column": 3 - } - ], - "message": "Could not resolve to a Repository with the name 'REPO'." - } - ] -}