Skip to content

Commit 0fad40d

Browse files
wxiaoguangwxiaoguang
authored
Fix package error handling and npm meta and empty repo guide (#33112)
1 parent e637008 commit 0fad40d

File tree

10 files changed

+69
-49
lines changed

10 files changed

+69
-49
lines changed

‎modules/packages/npm/creator.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ type PackageMetadataVersion struct {
8181
BundleDependencies []string `json:"bundleDependencies,omitempty"`
8282
DevDependencies map[string]string `json:"devDependencies,omitempty"`
8383
PeerDependencies map[string]string `json:"peerDependencies,omitempty"`
84+
PeerDependenciesMeta map[string]any `json:"peerDependenciesMeta,omitempty"`
8485
Bin map[string]string `json:"bin,omitempty"`
8586
OptionalDependencies map[string]string `json:"optionalDependencies,omitempty"`
8687
Readme string `json:"readme,omitempty"`
@@ -222,6 +223,7 @@ func ParsePackage(r io.Reader) (*Package, error) {
222223
BundleDependencies: meta.BundleDependencies,
223224
DevelopmentDependencies: meta.DevDependencies,
224225
PeerDependencies: meta.PeerDependencies,
226+
PeerDependenciesMeta: meta.PeerDependenciesMeta,
225227
OptionalDependencies: meta.OptionalDependencies,
226228
Bin: meta.Bin,
227229
Readme: meta.Readme,

‎modules/packages/npm/metadata.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ type Metadata struct {
1919
BundleDependencies []string `json:"bundleDependencies,omitempty"`
2020
DevelopmentDependencies map[string]string `json:"development_dependencies,omitempty"`
2121
PeerDependencies map[string]string `json:"peer_dependencies,omitempty"`
22+
PeerDependenciesMeta map[string]any `json:"peer_dependencies_meta,omitempty"`
2223
OptionalDependencies map[string]string `json:"optional_dependencies,omitempty"`
2324
Bin map[string]string `json:"bin,omitempty"`
2425
Readme string `json:"readme,omitempty"`

‎routers/api/packages/container/blob.go

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -111,12 +111,11 @@ func getOrCreateUploadVersion(ctx context.Context, pi *packages_service.PackageI
111111
}
112112
var err error
113113
if p, err = packages_model.TryInsertPackage(ctx, p); err != nil {
114-
if err == packages_model.ErrDuplicatePackage {
115-
created = false
116-
} else {
114+
if !errors.Is(err, packages_model.ErrDuplicatePackage) {
117115
log.Error("Error inserting package: %v", err)
118116
return err
119117
}
118+
created = false
120119
}
121120

122121
if created {
@@ -135,7 +134,7 @@ func getOrCreateUploadVersion(ctx context.Context, pi *packages_service.PackageI
135134
MetadataJSON: "null",
136135
}
137136
if pv, err = packages_model.GetOrInsertVersion(ctx, pv); err != nil {
138-
if err != packages_model.ErrDuplicatePackageVersion {
137+
if !errors.Is(err, packages_model.ErrDuplicatePackageVersion) {
139138
log.Error("Error inserting package: %v", err)
140139
return err
141140
}
@@ -161,7 +160,7 @@ func createFileForBlob(ctx context.Context, pv *packages_model.PackageVersion, p
161160
}
162161
var err error
163162
if pf, err = packages_model.TryInsertFile(ctx, pf); err != nil {
164-
if err == packages_model.ErrDuplicatePackageFile {
163+
if errors.Is(err, packages_model.ErrDuplicatePackageFile) {
165164
return nil
166165
}
167166
log.Error("Error inserting package file: %v", err)

‎routers/api/packages/container/container.go

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ func GetUploadBlob(ctx *context.Context) {
324324

325325
upload, err := packages_model.GetBlobUploadByID(ctx, uuid)
326326
if err != nil {
327-
if err == packages_model.ErrPackageBlobUploadNotExist {
327+
if errors.Is(err, packages_model.ErrPackageBlobUploadNotExist) {
328328
apiErrorDefined(ctx, errBlobUploadUnknown)
329329
} else {
330330
apiError(ctx, http.StatusInternalServerError, err)
@@ -345,7 +345,7 @@ func UploadBlob(ctx *context.Context) {
345345

346346
uploader, err := container_service.NewBlobUploader(ctx, ctx.PathParam("uuid"))
347347
if err != nil {
348-
if err == packages_model.ErrPackageBlobUploadNotExist {
348+
if errors.Is(err, packages_model.ErrPackageBlobUploadNotExist) {
349349
apiErrorDefined(ctx, errBlobUploadUnknown)
350350
} else {
351351
apiError(ctx, http.StatusInternalServerError, err)
@@ -396,7 +396,7 @@ func EndUploadBlob(ctx *context.Context) {
396396

397397
uploader, err := container_service.NewBlobUploader(ctx, ctx.PathParam("uuid"))
398398
if err != nil {
399-
if err == packages_model.ErrPackageBlobUploadNotExist {
399+
if errors.Is(err, packages_model.ErrPackageBlobUploadNotExist) {
400400
apiErrorDefined(ctx, errBlobUploadUnknown)
401401
} else {
402402
apiError(ctx, http.StatusInternalServerError, err)
@@ -465,7 +465,7 @@ func CancelUploadBlob(ctx *context.Context) {
465465

466466
_, err := packages_model.GetBlobUploadByID(ctx, uuid)
467467
if err != nil {
468-
if err == packages_model.ErrPackageBlobUploadNotExist {
468+
if errors.Is(err, packages_model.ErrPackageBlobUploadNotExist) {
469469
apiErrorDefined(ctx, errBlobUploadUnknown)
470470
} else {
471471
apiError(ctx, http.StatusInternalServerError, err)
@@ -501,7 +501,7 @@ func getBlobFromContext(ctx *context.Context) (*packages_model.PackageFileDescri
501501
func HeadBlob(ctx *context.Context) {
502502
blob, err := getBlobFromContext(ctx)
503503
if err != nil {
504-
if err == container_model.ErrContainerBlobNotExist {
504+
if errors.Is(err, container_model.ErrContainerBlobNotExist) {
505505
apiErrorDefined(ctx, errBlobUnknown)
506506
} else {
507507
apiError(ctx, http.StatusInternalServerError, err)
@@ -520,7 +520,7 @@ func HeadBlob(ctx *context.Context) {
520520
func GetBlob(ctx *context.Context) {
521521
blob, err := getBlobFromContext(ctx)
522522
if err != nil {
523-
if err == container_model.ErrContainerBlobNotExist {
523+
if errors.Is(err, container_model.ErrContainerBlobNotExist) {
524524
apiErrorDefined(ctx, errBlobUnknown)
525525
} else {
526526
apiError(ctx, http.StatusInternalServerError, err)
@@ -639,7 +639,7 @@ func getManifestFromContext(ctx *context.Context) (*packages_model.PackageFileDe
639639
func HeadManifest(ctx *context.Context) {
640640
manifest, err := getManifestFromContext(ctx)
641641
if err != nil {
642-
if err == container_model.ErrContainerBlobNotExist {
642+
if errors.Is(err, container_model.ErrContainerBlobNotExist) {
643643
apiErrorDefined(ctx, errManifestUnknown)
644644
} else {
645645
apiError(ctx, http.StatusInternalServerError, err)
@@ -659,7 +659,7 @@ func HeadManifest(ctx *context.Context) {
659659
func GetManifest(ctx *context.Context) {
660660
manifest, err := getManifestFromContext(ctx)
661661
if err != nil {
662-
if err == container_model.ErrContainerBlobNotExist {
662+
if errors.Is(err, container_model.ErrContainerBlobNotExist) {
663663
apiErrorDefined(ctx, errManifestUnknown)
664664
} else {
665665
apiError(ctx, http.StatusInternalServerError, err)
@@ -739,7 +739,7 @@ func GetTagList(ctx *context.Context) {
739739
image := ctx.PathParam("image")
740740

741741
if _, err := packages_model.GetPackageByName(ctx, ctx.Package.Owner.ID, packages_model.TypeContainer, image); err != nil {
742-
if err == packages_model.ErrPackageNotExist {
742+
if errors.Is(err, packages_model.ErrPackageNotExist) {
743743
apiErrorDefined(ctx, errNameUnknown)
744744
} else {
745745
apiError(ctx, http.StatusInternalServerError, err)

‎routers/api/packages/container/manifest.go

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ func processImageManifestIndex(ctx context.Context, mci *manifestCreationInfo, b
240240
IsManifest: true,
241241
})
242242
if err != nil {
243-
if err == container_model.ErrContainerBlobNotExist {
243+
if errors.Is(err, container_model.ErrContainerBlobNotExist) {
244244
return errManifestBlobUnknown
245245
}
246246
return err
@@ -321,12 +321,11 @@ func createPackageAndVersion(ctx context.Context, mci *manifestCreationInfo, met
321321
}
322322
var err error
323323
if p, err = packages_model.TryInsertPackage(ctx, p); err != nil {
324-
if err == packages_model.ErrDuplicatePackage {
325-
created = false
326-
} else {
324+
if !errors.Is(err, packages_model.ErrDuplicatePackage) {
327325
log.Error("Error inserting package: %v", err)
328326
return nil, err
329327
}
328+
created = false
330329
}
331330

332331
if created {
@@ -352,21 +351,23 @@ func createPackageAndVersion(ctx context.Context, mci *manifestCreationInfo, met
352351
}
353352
var pv *packages_model.PackageVersion
354353
if pv, err = packages_model.GetOrInsertVersion(ctx, _pv); err != nil {
355-
if err == packages_model.ErrDuplicatePackageVersion {
356-
if err := packages_service.DeletePackageVersionAndReferences(ctx, pv); err != nil {
357-
return nil, err
358-
}
354+
if !errors.Is(err, packages_model.ErrDuplicatePackageVersion) {
355+
log.Error("Error inserting package: %v", err)
356+
return nil, err
357+
}
359358

360-
// keep download count on overwrite
361-
_pv.DownloadCount = pv.DownloadCount
359+
if err = packages_service.DeletePackageVersionAndReferences(ctx, pv); err != nil {
360+
return nil, err
361+
}
362+
363+
// keep download count on overwrite
364+
_pv.DownloadCount = pv.DownloadCount
362365

363-
if pv, err = packages_model.GetOrInsertVersion(ctx, _pv); err != nil {
366+
if pv, err = packages_model.GetOrInsertVersion(ctx, _pv); err != nil {
367+
if !errors.Is(err, packages_model.ErrDuplicatePackageVersion) {
364368
log.Error("Error inserting package: %v", err)
365369
return nil, err
366370
}
367-
} else {
368-
log.Error("Error inserting package: %v", err)
369-
return nil, err
370371
}
371372
}
372373

@@ -417,7 +418,7 @@ func createFileFromBlobReference(ctx context.Context, pv, uploadVersion *package
417418
}
418419
var err error
419420
if pf, err = packages_model.TryInsertFile(ctx, pf); err != nil {
420-
if err == packages_model.ErrDuplicatePackageFile {
421+
if errors.Is(err, packages_model.ErrDuplicatePackageFile) {
421422
// Skip this blob because the manifest contains the same filesystem layer multiple times.
422423
return nil
423424
}

‎routers/api/packages/npm/api.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ func createPackageMetadataVersion(registryURL string, pd *packages_model.Package
6767
BundleDependencies: metadata.BundleDependencies,
6868
DevDependencies: metadata.DevelopmentDependencies,
6969
PeerDependencies: metadata.PeerDependencies,
70+
PeerDependenciesMeta: metadata.PeerDependenciesMeta,
7071
OptionalDependencies: metadata.OptionalDependencies,
7172
Readme: metadata.Readme,
7273
Bin: metadata.Bin,

‎routers/web/repo/view_home.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,7 @@ func handleRepoEmptyOrBroken(ctx *context.Context) {
250250
showEmpty = true // it is not really empty, but there is no branch
251251
// at the moment, other repo units like "actions" are not able to handle such case,
252252
// so we just mark the repo as empty to prevent from displaying these units.
253+
ctx.Data["RepoHasContentsWithoutBranch"] = true
253254
updateContextRepoEmptyAndStatus(ctx, true, repo_model.RepositoryReady)
254255
} else {
255256
// the repo is actually not empty and has branches, need to update the database later

‎templates/repo/empty.tmpl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717

1818
{{if .Repository.IsBroken}}
1919
<div class="ui segment center">{{ctx.Locale.Tr "repo.broken_message"}}</div>
20-
{{else if .Repository.IsEmpty}}
20+
{{else if .RepoHasContentsWithoutBranch}}
2121
<div class="ui segment center">{{ctx.Locale.Tr "repo.no_branch"}}</div>
2222
{{else if .CanWriteCode}}
2323
<h4 class="ui top attached header">{{ctx.Locale.Tr "repo.quick_guide"}}</h4>

‎tests/integration/api_packages_npm_test.go

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -52,24 +52,33 @@ func TestPackageNpm(t *testing.T) {
5252
"` + packageTag + `": "` + version + `"
5353
},
5454
"versions": {
55-
"` + version + `": {
56-
"name": "` + packageName + `",
57-
"version": "` + version + `",
58-
"description": "` + packageDescription + `",
59-
"author": {
60-
"name": "` + packageAuthor + `"
61-
},
62-
"bin": {
63-
"` + packageBinName + `": "` + packageBinPath + `"
64-
},
65-
"dist": {
66-
"integrity": "sha512-yA4FJsVhetynGfOC1jFf79BuS+jrHbm0fhh+aHzCQkOaOBXKf9oBnC4a6DnLLnEsHQDRLYd00cwj8sCXpC+wIg==",
67-
"shasum": "aaa7eaf852a948b0aa05afeda35b1badca155d90"
68-
},
69-
"repository": {
70-
"type": "` + repoType + `",
71-
"url": "` + repoURL + `"
72-
}
55+
"` + version + `": {
56+
"name": "` + packageName + `",
57+
"version": "` + version + `",
58+
"description": "` + packageDescription + `",
59+
"author": {
60+
"name": "` + packageAuthor + `"
61+
},
62+
"bin": {
63+
"` + packageBinName + `": "` + packageBinPath + `"
64+
},
65+
"dist": {
66+
"integrity": "sha512-yA4FJsVhetynGfOC1jFf79BuS+jrHbm0fhh+aHzCQkOaOBXKf9oBnC4a6DnLLnEsHQDRLYd00cwj8sCXpC+wIg==",
67+
"shasum": "aaa7eaf852a948b0aa05afeda35b1badca155d90"
68+
},
69+
"repository": {
70+
"type": "` + repoType + `",
71+
"url": "` + repoURL + `"
72+
},
73+
"peerDependencies": {
74+
"tea": "2.x",
75+
"soy-milk": "1.2"
76+
},
77+
"peerDependenciesMeta": {
78+
"soy-milk": {
79+
"optional": true
80+
}
81+
}
7382
}
7483
},
7584
"_attachments": {
@@ -178,6 +187,8 @@ func TestPackageNpm(t *testing.T) {
178187
assert.Equal(t, fmt.Sprintf("%s%s/-/%s/%s", setting.AppURL, root[1:], packageVersion, filename), pmv.Dist.Tarball)
179188
assert.Equal(t, repoType, result.Repository.Type)
180189
assert.Equal(t, repoURL, result.Repository.URL)
190+
assert.Equal(t, map[string]string{"tea": "2.x", "soy-milk": "1.2"}, pmv.PeerDependencies)
191+
assert.Equal(t, map[string]any{"soy-milk": map[string]any{"optional": true}}, pmv.PeerDependenciesMeta)
181192
})
182193

183194
t.Run("AddTag", func(t *testing.T) {

‎tests/integration/empty_repo_test.go

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,12 @@ func TestEmptyRepoAddFile(t *testing.T) {
5858
defer tests.PrepareTestEnv(t)()
5959

6060
session := loginUser(t, "user30")
61-
req := NewRequest(t, "GET", "/user30/empty/_new/"+setting.Repository.DefaultBranch)
61+
req := NewRequest(t, "GET", "/user30/empty")
6262
resp := session.MakeRequest(t, req, http.StatusOK)
63+
assert.Contains(t, resp.Body.String(), "empty-repo-guide")
64+
65+
req = NewRequest(t, "GET", "/user30/empty/_new/"+setting.Repository.DefaultBranch)
66+
resp = session.MakeRequest(t, req, http.StatusOK)
6367
doc := NewHTMLParser(t, resp.Body).Find(`input[name="commit_choice"]`)
6468
assert.Empty(t, doc.AttrOr("checked", "_no_"))
6569
req = NewRequestWithValues(t, "POST", "/user30/empty/_new/"+setting.Repository.DefaultBranch, map[string]string{

0 commit comments

Comments
 (0)