Skip to content

Commit 7fad2d0

Browse files
vilmibmvilmibm
committed
switch to alternate method for determining if repo already forked
1 parent 115bf37 commit 7fad2d0

File tree

6 files changed

+51
-55
lines changed

6 files changed

+51
-55
lines changed

‎api/queries_repo.go‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,18 @@ import (
66
"fmt"
77
"sort"
88
"strings"
9+
"time"
910

1011
"github.com/cli/cli/internal/ghrepo"
1112
)
1213

1314
// Repository contains information about a GitHub repo
1415
type Repository struct {
15-
ID string
16-
Name string
17-
CloneURL string
18-
Owner RepositoryOwner
16+
ID string
17+
Name string
18+
CloneURL string
19+
CreatedAt time.Time
20+
Owner RepositoryOwner
1921

2022
IsPrivate bool
2123
HasIssuesEnabled bool
@@ -211,10 +213,11 @@ func RepoNetwork(client *Client, repos []ghrepo.Interface) (RepoNetworkResult, e
211213

212214
// repositoryV3 is the repository result from GitHub API v3
213215
type repositoryV3 struct {
214-
NodeID string
215-
Name string
216-
CloneURL string `json:"clone_url"`
217-
Owner struct {
216+
NodeID string
217+
Name string
218+
CreatedAt time.Time `json:"created_at"`
219+
CloneURL string `json:"clone_url"`
220+
Owner struct {
218221
Login string
219222
}
220223
}
@@ -230,9 +233,10 @@ func ForkRepo(client *Client, repo ghrepo.Interface) (*Repository, error) {
230233
}
231234

232235
return &Repository{
233-
ID: result.NodeID,
234-
Name: result.Name,
235-
CloneURL: result.CloneURL,
236+
ID: result.NodeID,
237+
Name: result.Name,
238+
CloneURL: result.CloneURL,
239+
CreatedAt: result.CreatedAt,
236240
Owner: RepositoryOwner{
237241
Login: result.Owner.Login,
238242
},

‎command/repo.go‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"net/url"
66
"os"
77
"strings"
8+
"time"
89

910
"github.com/cli/cli/api"
1011
"github.com/cli/cli/git"
@@ -84,6 +85,10 @@ func isURL(arg string) bool {
8485
return strings.HasPrefix(arg, "http:/") || strings.HasPrefix(arg, "https:/")
8586
}
8687

88+
var Since = func(t time.Time) time.Duration {
89+
return time.Since(t)
90+
}
91+
8792
func repoFork(cmd *cobra.Command, args []string) error {
8893
ctx := contextForCommand(cmd)
8994

@@ -148,23 +153,25 @@ func repoFork(cmd *cobra.Command, args []string) error {
148153
}
149154

150155
possibleFork := ghrepo.New(authLogin, toFork.RepoName())
151-
exists, err := api.GitHubRepoExists(apiClient, possibleFork)
156+
157+
forkedRepo, err := api.ForkRepo(apiClient, toFork)
152158
if err != nil {
153159
s.Stop()
154-
return fmt.Errorf("problem with API request: %w", err)
160+
return fmt.Errorf("failed to fork: %w", err)
155161
}
156162

157-
if exists {
163+
// This is weird. There is not an efficient way to determine via the GitHub API whether or not a
164+
// given user has forked a given repo. We noticed, also, that the create fork API endpoint just
165+
// returns the fork repo data even if it already exists -- with no change in status code or
166+
// anything. We thus check the created time to see if the repo is brand new or not; if it's not,
167+
// we assume the fork already existed and report an error.
168+
created_ago := Since(forkedRepo.CreatedAt)
169+
if created_ago > time.Minute {
158170
s.Stop()
159171
fmt.Fprintf(out, redX+" ")
160172
return fmt.Errorf("%s already exists", utils.Bold(ghrepo.FullName(possibleFork)))
161173
}
162174

163-
forkedRepo, err := api.ForkRepo(apiClient, toFork)
164-
if err != nil {
165-
s.Stop()
166-
return fmt.Errorf("failed to fork: %w", err)
167-
}
168175
s.Stop()
169176

170177
fmt.Fprintf(out, "%s Created fork %s\n", greenCheck, utils.Bold(ghrepo.FullName(forkedRepo)))

‎command/repo_test.go‎

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"regexp"
66
"strings"
77
"testing"
8+
"time"
89

910
"github.com/cli/cli/context"
1011
"github.com/cli/cli/utils"
@@ -23,7 +24,7 @@ func TestRepoFork_already_forked(t *testing.T) {
2324
}
2425
http := initFakeHTTP()
2526
http.StubRepoResponse("OWNER", "REPO")
26-
defer http.StubWithFixture(200, "repo.json")()
27+
defer http.StubWithFixture(200, "forkResult.json")()
2728

2829
_, err := RunCommand(repoForkCmd, "repo fork --remote=false")
2930
if err == nil {
@@ -34,11 +35,21 @@ func TestRepoFork_already_forked(t *testing.T) {
3435
}
3536
}
3637

38+
func stubSince(d time.Duration) func() {
39+
originalSince := Since
40+
Since = func(t time.Time) time.Duration {
41+
return d
42+
}
43+
return func() {
44+
Since = originalSince
45+
}
46+
}
47+
3748
func TestRepoFork_in_parent(t *testing.T) {
3849
initBlankContext("OWNER/REPO", "master")
50+
defer stubSince(2 * time.Second)()
3951
http := initFakeHTTP()
4052
http.StubRepoResponse("OWNER", "REPO")
41-
defer http.StubWithFixture(200, "repoNotFound.json")()
4253
defer http.StubWithFixture(200, "forkResult.json")()
4354

4455
output, err := RunCommand(repoForkCmd, "repo fork --remote=false")
@@ -75,8 +86,8 @@ func TestRepoFork_outside(t *testing.T) {
7586
}
7687
for _, tt := range tests {
7788
t.Run(tt.name, func(t *testing.T) {
89+
defer stubSince(2 * time.Second)()
7890
http := initFakeHTTP()
79-
defer http.StubWithFixture(200, "repoNotFound.json")()
8091
defer http.StubWithFixture(200, "forkResult.json")()
8192

8293
output, err := RunCommand(repoForkCmd, tt.args)
@@ -101,9 +112,9 @@ func TestRepoFork_outside(t *testing.T) {
101112

102113
func TestRepoFork_in_parent_yes(t *testing.T) {
103114
initBlankContext("OWNER/REPO", "master")
115+
defer stubSince(2 * time.Second)()
104116
http := initFakeHTTP()
105117
http.StubRepoResponse("OWNER", "REPO")
106-
defer http.StubWithFixture(200, "repoNotFound.json")()
107118
defer http.StubWithFixture(200, "forkResult.json")()
108119

109120
var seenCmds []*exec.Cmd
@@ -141,8 +152,8 @@ func TestRepoFork_in_parent_yes(t *testing.T) {
141152
}
142153

143154
func TestRepoFork_outside_yes(t *testing.T) {
155+
defer stubSince(2 * time.Second)()
144156
http := initFakeHTTP()
145-
defer http.StubWithFixture(200, "repoNotFound.json")()
146157
defer http.StubWithFixture(200, "forkResult.json")()
147158

148159
var seenCmd *exec.Cmd
@@ -173,8 +184,8 @@ func TestRepoFork_outside_yes(t *testing.T) {
173184
}
174185

175186
func TestRepoFork_outside_survey_yes(t *testing.T) {
187+
defer stubSince(2 * time.Second)()
176188
http := initFakeHTTP()
177-
defer http.StubWithFixture(200, "repoNotFound.json")()
178189
defer http.StubWithFixture(200, "forkResult.json")()
179190

180191
var seenCmd *exec.Cmd
@@ -212,8 +223,8 @@ func TestRepoFork_outside_survey_yes(t *testing.T) {
212223
}
213224

214225
func TestRepoFork_outside_survey_no(t *testing.T) {
226+
defer stubSince(2 * time.Second)()
215227
http := initFakeHTTP()
216-
defer http.StubWithFixture(200, "repoNotFound.json")()
217228
defer http.StubWithFixture(200, "forkResult.json")()
218229

219230
cmdRun := false
@@ -251,9 +262,9 @@ func TestRepoFork_outside_survey_no(t *testing.T) {
251262

252263
func TestRepoFork_in_parent_survey_yes(t *testing.T) {
253264
initBlankContext("OWNER/REPO", "master")
265+
defer stubSince(2 * time.Second)()
254266
http := initFakeHTTP()
255267
http.StubRepoResponse("OWNER", "REPO")
256-
defer http.StubWithFixture(200, "repoNotFound.json")()
257268
defer http.StubWithFixture(200, "forkResult.json")()
258269

259270
var seenCmds []*exec.Cmd
@@ -299,9 +310,9 @@ func TestRepoFork_in_parent_survey_yes(t *testing.T) {
299310

300311
func TestRepoFork_in_parent_survey_no(t *testing.T) {
301312
initBlankContext("OWNER/REPO", "master")
313+
defer stubSince(2 * time.Second)()
302314
http := initFakeHTTP()
303315
http.StubRepoResponse("OWNER", "REPO")
304-
defer http.StubWithFixture(200, "repoNotFound.json")()
305316
defer http.StubWithFixture(200, "forkResult.json")()
306317

307318
cmdRun := false

‎test/fixtures/forkResult.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
"node_id": "123",
33
"name": "REPO",
44
"clone_url": "https://github.com/someone/repo.git",
5+
"created_at": "2011-01-26T19:01:12Z",
56
"owner": {
67
"login": "someone"
78
}

‎test/fixtures/repo.json‎

Lines changed: 0 additions & 7 deletions
This file was deleted.

‎test/fixtures/repoNotFound.json‎

Lines changed: 0 additions & 20 deletions
This file was deleted.

0 commit comments

Comments
 (0)