Skip to content

Commit e5c576e

Browse files
wxiaoguangwxiaoguang
authored
Refactor maven package registry (#33049)
Close #33036
1 parent 54bd220 commit e5c576e

File tree

5 files changed

+143
-60
lines changed

5 files changed

+143
-60
lines changed

‎models/packages/package.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -248,6 +248,18 @@ func GetPackageByID(ctx context.Context, packageID int64) (*Package, error) {
248248
return p, nil
249249
}
250250

251+
// UpdatePackageNameByID updates the package's name, it is only for internal usage, for example: rename some legacy packages
252+
func UpdatePackageNameByID(ctx context.Context, ownerID int64, packageType Type, packageID int64, name string) error {
253+
var cond builder.Cond = builder.Eq{
254+
"package.id": packageID,
255+
"package.owner_id": ownerID,
256+
"package.type": packageType,
257+
"package.is_internal": false,
258+
}
259+
_, err := db.GetEngine(ctx).Where(cond).Update(&Package{Name: name, LowerName: strings.ToLower(name)})
260+
return err
261+
}
262+
251263
// GetPackageByName gets a package by name
252264
func GetPackageByName(ctx context.Context, ownerID int64, packageType Type, name string) (*Package, error) {
253265
var cond builder.Cond = builder.Eq{

‎modules/web/router_test.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,11 @@ func TestRouter(t *testing.T) {
183183
pathParams: map[string]string{"username": "the-user", "reponame": "the-repo", "*": "d1/d2/fn", "dir": "d1/d2", "file": "fn"},
184184
handlerMark: "match-path",
185185
})
186+
testRoute(t, "GET /api/v1/repos/the-user/the-repo/branches/d1%2fd2/fn", resultStruct{
187+
method: "GET",
188+
pathParams: map[string]string{"username": "the-user", "reponame": "the-repo", "*": "d1%2fd2/fn", "dir": "d1%2fd2", "file": "fn"},
189+
handlerMark: "match-path",
190+
})
186191
testRoute(t, "GET /api/v1/repos/the-user/the-repo/branches/d1/d2/000", resultStruct{
187192
method: "GET",
188193
pathParams: map[string]string{"reponame": "the-repo", "username": "the-user", "*": "d1/d2/000"},

‎routers/api/packages/api.go‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -430,8 +430,6 @@ func CommonRoutes() *web.Router {
430430
r.Post("/api/charts", reqPackageAccess(perm.AccessModeWrite), helm.UploadPackage)
431431
}, reqPackageAccess(perm.AccessModeRead))
432432
r.Group("/maven", func() {
433-
// FIXME: this path design is not right.
434-
// It should be `/.../{groupId}/{artifactId}/{version}`, but not `/.../{groupId}-{artifactId}/{version}`
435433
r.Put("/*", reqPackageAccess(perm.AccessModeWrite), maven.UploadPackageFile)
436434
r.Get("/*", maven.DownloadPackageFile)
437435
r.Head("/*", maven.ProvidePackageFileHeader)

‎routers/api/packages/maven/maven.go‎

Lines changed: 56 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import (
1313
"errors"
1414
"io"
1515
"net/http"
16-
"path/filepath"
16+
"path"
1717
"regexp"
1818
"sort"
1919
"strconv"
@@ -25,6 +25,7 @@ import (
2525
"code.gitea.io/gitea/modules/log"
2626
packages_module "code.gitea.io/gitea/modules/packages"
2727
maven_module "code.gitea.io/gitea/modules/packages/maven"
28+
"code.gitea.io/gitea/modules/util"
2829
"code.gitea.io/gitea/routers/api/packages/helper"
2930
"code.gitea.io/gitea/services/context"
3031
packages_service "code.gitea.io/gitea/services/packages"
@@ -44,7 +45,7 @@ const (
4445

4546
var (
4647
errInvalidParameters = errors.New("request parameters are invalid")
47-
illegalCharacters = regexp.MustCompile(`[\\/:"<>|?\*]`)
48+
illegalCharacters = regexp.MustCompile(`[\\/:"<>|?*]`)
4849
)
4950

5051
func apiError(ctx *context.Context, status int, obj any) {
@@ -85,8 +86,10 @@ func handlePackageFile(ctx *context.Context, serveContent bool) {
8586
func serveMavenMetadata(ctx *context.Context, params parameters) {
8687
// /com/foo/project/maven-metadata.xml[.md5/.sha1/.sha256/.sha512]
8788

88-
packageName := params.GroupID + "-" + params.ArtifactID
89-
pvs, err := packages_model.GetVersionsByPackageName(ctx, ctx.Package.Owner.ID, packages_model.TypeMaven, packageName)
89+
pvs, err := packages_model.GetVersionsByPackageName(ctx, ctx.Package.Owner.ID, packages_model.TypeMaven, params.toInternalPackageName())
90+
if errors.Is(err, util.ErrNotExist) {
91+
pvs, err = packages_model.GetVersionsByPackageName(ctx, ctx.Package.Owner.ID, packages_model.TypeMaven, params.toInternalPackageNameLegacy())
92+
}
9093
if err != nil {
9194
apiError(ctx, http.StatusInternalServerError, err)
9295
return
@@ -116,10 +119,10 @@ func serveMavenMetadata(ctx *context.Context, params parameters) {
116119

117120
latest := pds[len(pds)-1]
118121
// http.TimeFormat required a UTC time, refer to https://pkg.go.dev/net/http#TimeFormat
119-
lastModifed := latest.Version.CreatedUnix.AsTime().UTC().Format(http.TimeFormat)
120-
ctx.Resp.Header().Set("Last-Modified", lastModifed)
122+
lastModified := latest.Version.CreatedUnix.AsTime().UTC().Format(http.TimeFormat)
123+
ctx.Resp.Header().Set("Last-Modified", lastModified)
121124

122-
ext := strings.ToLower(filepath.Ext(params.Filename))
125+
ext := strings.ToLower(path.Ext(params.Filename))
123126
if isChecksumExtension(ext) {
124127
var hash []byte
125128
switch ext {
@@ -147,11 +150,12 @@ func serveMavenMetadata(ctx *context.Context, params parameters) {
147150
}
148151

149152
func servePackageFile(ctx *context.Context, params parameters, serveContent bool) {
150-
packageName := params.GroupID + "-" + params.ArtifactID
151-
152-
pv, err := packages_model.GetVersionByNameAndVersion(ctx, ctx.Package.Owner.ID, packages_model.TypeMaven, packageName, params.Version)
153+
pv, err := packages_model.GetVersionByNameAndVersion(ctx, ctx.Package.Owner.ID, packages_model.TypeMaven, params.toInternalPackageName(), params.Version)
154+
if errors.Is(err, util.ErrNotExist) {
155+
pv, err = packages_model.GetVersionByNameAndVersion(ctx, ctx.Package.Owner.ID, packages_model.TypeMaven, params.toInternalPackageNameLegacy(), params.Version)
156+
}
153157
if err != nil {
154-
if err == packages_model.ErrPackageNotExist {
158+
if errors.Is(err, packages_model.ErrPackageNotExist) {
155159
apiError(ctx, http.StatusNotFound, err)
156160
} else {
157161
apiError(ctx, http.StatusInternalServerError, err)
@@ -161,14 +165,14 @@ func servePackageFile(ctx *context.Context, params parameters, serveContent bool
161165

162166
filename := params.Filename
163167

164-
ext := strings.ToLower(filepath.Ext(filename))
168+
ext := strings.ToLower(path.Ext(filename))
165169
if isChecksumExtension(ext) {
166170
filename = filename[:len(filename)-len(ext)]
167171
}
168172

169173
pf, err := packages_model.GetFileForVersionByName(ctx, pv.ID, filename, packages_model.EmptyFileKey)
170174
if err != nil {
171-
if err == packages_model.ErrPackageFileNotExist {
175+
if errors.Is(err, packages_model.ErrPackageFileNotExist) {
172176
apiError(ctx, http.StatusNotFound, err)
173177
} else {
174178
apiError(ctx, http.StatusInternalServerError, err)
@@ -238,15 +242,17 @@ func UploadPackageFile(ctx *context.Context) {
238242
return
239243
}
240244

241-
log.Trace("Parameters: %+v", params)
242-
243245
// Ignore the package index /<name>/maven-metadata.xml
244246
if params.IsMeta && params.Version == "" {
245247
ctx.Status(http.StatusOK)
246248
return
247249
}
248250

249-
packageName := params.GroupID + "-" + params.ArtifactID
251+
packageName := params.toInternalPackageName()
252+
if ctx.FormBool("use_legacy_package_name") {
253+
// for testing purpose only
254+
packageName = params.toInternalPackageNameLegacy()
255+
}
250256

251257
// for the same package, only one upload at a time
252258
releaser, err := globallock.Lock(ctx, mavenPkgNameKey(packageName))
@@ -274,13 +280,26 @@ func UploadPackageFile(ctx *context.Context) {
274280
Creator: ctx.Doer,
275281
}
276282

277-
ext := filepath.Ext(params.Filename)
283+
// old maven package uses "groupId-artifactId" as package name, so we need to update to the new format "groupId:artifactId"
284+
legacyPackage, err := packages_model.GetPackageByName(ctx, ctx.Package.Owner.ID, packages_model.TypeMaven, params.toInternalPackageNameLegacy())
285+
if err != nil && !errors.Is(err, packages_model.ErrPackageNotExist) {
286+
apiError(ctx, http.StatusInternalServerError, err)
287+
return
288+
} else if legacyPackage != nil {
289+
err = packages_model.UpdatePackageNameByID(ctx, ctx.Package.Owner.ID, packages_model.TypeMaven, legacyPackage.ID, packageName)
290+
if err != nil {
291+
apiError(ctx, http.StatusInternalServerError, err)
292+
return
293+
}
294+
}
295+
296+
ext := path.Ext(params.Filename)
278297

279298
// Do not upload checksum files but compare the hashes.
280299
if isChecksumExtension(ext) {
281300
pv, err := packages_model.GetVersionByNameAndVersion(ctx, pvci.Owner.ID, pvci.PackageType, pvci.Name, pvci.Version)
282301
if err != nil {
283-
if err == packages_model.ErrPackageNotExist {
302+
if errors.Is(err, packages_model.ErrPackageNotExist) {
284303
apiError(ctx, http.StatusNotFound, err)
285304
return
286305
}
@@ -289,7 +308,7 @@ func UploadPackageFile(ctx *context.Context) {
289308
}
290309
pf, err := packages_model.GetFileForVersionByName(ctx, pv.ID, params.Filename[:len(params.Filename)-len(ext)], packages_model.EmptyFileKey)
291310
if err != nil {
292-
if err == packages_model.ErrPackageFileNotExist {
311+
if errors.Is(err, packages_model.ErrPackageFileNotExist) {
293312
apiError(ctx, http.StatusNotFound, err)
294313
return
295314
}
@@ -343,7 +362,7 @@ func UploadPackageFile(ctx *context.Context) {
343362

344363
if pvci.Metadata != nil {
345364
pv, err := packages_model.GetVersionByNameAndVersion(ctx, pvci.Owner.ID, pvci.PackageType, pvci.Name, pvci.Version)
346-
if err != nil && err != packages_model.ErrPackageNotExist {
365+
if err != nil && !errors.Is(err, packages_model.ErrPackageNotExist) {
347366
apiError(ctx, http.StatusInternalServerError, err)
348367
return
349368
}
@@ -399,9 +418,26 @@ type parameters struct {
399418
IsMeta bool
400419
}
401420

421+
func (p *parameters) toInternalPackageName() string {
422+
// there cuold be 2 choices: "/" or ":"
423+
// Maven says: "groupId:artifactId:version" in their document: https://maven.apache.org/pom.html#Maven_Coordinates
424+
// but it would be slightly ugly in URL: "/-/packages/maven/group-id%3Aartifact-id"
425+
return p.GroupID + ":" + p.ArtifactID
426+
}
427+
428+
func (p *parameters) toInternalPackageNameLegacy() string {
429+
return p.GroupID + "-" + p.ArtifactID
430+
}
431+
402432
func extractPathParameters(ctx *context.Context) (parameters, error) {
403433
parts := strings.Split(ctx.PathParam("*"), "/")
404434

435+
// formats:
436+
// * /com/group/id/artifactId/maven-metadata.xml[.md5|.sha1|.sha256|.sha512]
437+
// * /com/group/id/artifactId/version-SNAPSHOT/maven-metadata.xml[.md5|.sha1|.sha256|.sha512]
438+
// * /com/group/id/artifactId/version/any-file
439+
// * /com/group/id/artifactId/version-SNAPSHOT/any-file
440+
405441
p := parameters{
406442
Filename: parts[len(parts)-1],
407443
}

0 commit comments

Comments
 (0)