[Feature] Private README.md for organization (#32872)

Implemented #29503

---------

Co-authored-by: Ben Chang <[email protected]>
Co-authored-by: wxiaoguang <[email protected]>

Author: 3 people

modules/gitrepo/gitrepo.go

@@ -43,19 +43,20 @@ type contextKey struct {
 }
 
 // RepositoryFromContextOrOpen attempts to get the repository from the context or just opens it
+// The caller must call "defer gitRepo.Close()"
 func RepositoryFromContextOrOpen(ctx context.Context, repo Repository) (*git.Repository, io.Closer, error) {
-	ds := reqctx.GetRequestDataStore(ctx)
-	if ds != nil {
-		gitRepo, err := RepositoryFromRequestContextOrOpen(ctx, ds, repo)
+	reqCtx := reqctx.FromContext(ctx)
+	if reqCtx != nil {
+		gitRepo, err := RepositoryFromRequestContextOrOpen(reqCtx, repo)
 		return gitRepo, util.NopCloser{}, err
 	}
 	gitRepo, err := OpenRepository(ctx, repo)
 	return gitRepo, gitRepo, err
 }
 
-// RepositoryFromRequestContextOrOpen opens the repository at the given relative path in the provided request context
-// The repo will be automatically closed when the request context is done
-func RepositoryFromRequestContextOrOpen(ctx context.Context, ds reqctx.RequestDataStore, repo Repository) (*git.Repository, error) {
+// RepositoryFromRequestContextOrOpen opens the repository at the given relative path in the provided request context.
+// Caller shouldn't close the git repo manually, the git repo will be automatically closed when the request context is done.
+func RepositoryFromRequestContextOrOpen(ctx reqctx.RequestContext, repo Repository) (*git.Repository, error) {
 	ck := contextKey{repoPath: repoPath(repo)}
 	if gitRepo, ok := ctx.Value(ck).(*git.Repository); ok {
 		return gitRepo, nil
@@ -64,7 +65,7 @@ func RepositoryFromRequestContextOrOpen(ctx context.Context, ds reqctx.RequestDa
 	if err != nil {
 		return nil, err
 	}
-	ds.AddCloser(gitRepo)
-	ds.SetContextValue(ck, gitRepo)
+	ctx.AddCloser(gitRepo)
+	ctx.SetContextValue(ck, gitRepo)
 	return gitRepo, nil
 }

modules/reqctx/datastore.go

@@ -88,6 +88,21 @@ func (r *requestDataStore) cleanUp() {
 	}
 }
 
+type RequestContext interface {
+	context.Context
+	RequestDataStore
+}
+
+func FromContext(ctx context.Context) RequestContext {
+	// here we must use the current ctx and the underlying store
+	// the current ctx guarantees that the ctx deadline/cancellation/values are respected
+	// the underlying store guarantees that the request-specific data is available
+	if store := GetRequestDataStore(ctx); store != nil {
+		return &requestContext{Context: ctx, RequestDataStore: store}
+	}
+	return nil
+}
+
 func GetRequestDataStore(ctx context.Context) RequestDataStore {
 	if req, ok := ctx.Value(RequestDataStoreKey).(*requestDataStore); ok {
 		return req
@@ -97,27 +112,28 @@ func GetRequestDataStore(ctx context.Context) RequestDataStore {
 
 type requestContext struct {
 	context.Context
-	dataStore *requestDataStore
+	RequestDataStore
 }
 
 func (c *requestContext) Value(key any) any {
-	if v := c.dataStore.GetContextValue(key); v != nil {
+	if v := c.GetContextValue(key); v != nil {
 		return v
 	}
 	return c.Context.Value(key)
 }
 
 func NewRequestContext(parentCtx context.Context, profDesc string) (_ context.Context, finished func()) {
 	ctx, _, processFinished := process.GetManager().AddTypedContext(parentCtx, profDesc, process.RequestProcessType, true)
-	reqCtx := &requestContext{Context: ctx, dataStore: &requestDataStore{values: make(map[any]any)}}
+	store := &requestDataStore{values: make(map[any]any)}
+	reqCtx := &requestContext{Context: ctx, RequestDataStore: store}
 	return reqCtx, func() {
-		reqCtx.dataStore.cleanUp()
+		store.cleanUp()
 		processFinished()
 	}
 }
 
 // NewRequestContextForTest creates a new RequestContext for testing purposes
 // It doesn't add the context to the process manager, nor do cleanup
 func NewRequestContextForTest(parentCtx context.Context) context.Context {
-	return &requestContext{Context: parentCtx, dataStore: &requestDataStore{values: make(map[any]any)}}
+	return &requestContext{Context: parentCtx, RequestDataStore: &requestDataStore{values: make(map[any]any)}}
 }

modules/templates/helper.go

@@ -264,22 +264,42 @@ func userThemeName(user *user_model.User) string {
 	return setting.UI.DefaultTheme
 }
 
+func isQueryParamEmpty(v any) bool {
+	return v == nil || v == false || v == 0 || v == int64(0) || v == ""
+}
+
 // QueryBuild builds a query string from a list of key-value pairs.
-// It omits the nil and empty strings, but it doesn't omit other zero values,
-// because the zero value of number types may have a meaning.
+// It omits the nil, false, zero int/int64 and empty string values,
+// because they are default empty values for "ctx.FormXxx" calls.
+// If 0 or false need to be included, use string values: "0" and "false".
+// Build rules:
+// * Even parameters: always build as query string: a=b&c=d
+// * Odd parameters:
+// * * {"/anything", param-pairs...} => "/?param-paris"
+// * * {"anything?old-params", new-param-pairs...} => "anything?old-params&new-param-paris"
+// * * Otherwise: {"old&params", new-param-pairs...} => "old&params&new-param-paris"
+// * * Other behaviors are undefined yet.
 func QueryBuild(a ...any) template.URL {
-	var s string
+	var reqPath, s string
+	hasTrailingSep := false
 	if len(a)%2 == 1 {
 		if v, ok := a[0].(string); ok {
-			if v == "" || (v[0] != '?' && v[0] != '&') {
-				panic("QueryBuild: invalid argument")
-			}
 			s = v
 		} else if v, ok := a[0].(template.URL); ok {
 			s = string(v)
 		} else {
 			panic("QueryBuild: invalid argument")
 		}
+		hasTrailingSep = s != "&" && strings.HasSuffix(s, "&")
+		if strings.HasPrefix(s, "/") || strings.Contains(s, "?") {
+			if s1, s2, ok := strings.Cut(s, "?"); ok {
+				reqPath = s1 + "?"
+				s = s2
+			} else {
+				reqPath += s + "?"
+				s = ""
+			}
+		}
 	}
 	for i := len(a) % 2; i < len(a); i += 2 {
 		k, ok := a[i].(string)
@@ -290,19 +310,16 @@ func QueryBuild(a ...any) template.URL {
 		if va, ok := a[i+1].(string); ok {
 			v = va
 		} else if a[i+1] != nil {
-			v = fmt.Sprint(a[i+1])
+			if !isQueryParamEmpty(a[i+1]) {
+				v = fmt.Sprint(a[i+1])
+			}
 		}
 		// pos1 to pos2 is the "k=v&" part, "&" is optional
 		pos1 := strings.Index(s, "&"+k+"=")
 		if pos1 != -1 {
 			pos1++
-		} else {
-			pos1 = strings.Index(s, "?"+k+"=")
-			if pos1 != -1 {
-				pos1++
-			} else if strings.HasPrefix(s, k+"=") {
-				pos1 = 0
-			}
+		} else if strings.HasPrefix(s, k+"=") {
+			pos1 = 0
 		}
 		pos2 := len(s)
 		if pos1 == -1 {
@@ -315,7 +332,7 @@ func QueryBuild(a ...any) template.URL {
 		}
 		if v != "" {
 			sep := ""
-			hasPrefixSep := pos1 == 0 || (pos1 <= len(s) && (s[pos1-1] == '?' || s[pos1-1] == '&'))
+			hasPrefixSep := pos1 == 0 || (pos1 <= len(s) && s[pos1-1] == '&')
 			if !hasPrefixSep {
 				sep = "&"
 			}
@@ -324,9 +341,22 @@ func QueryBuild(a ...any) template.URL {
 			s = s[:pos1] + s[pos2:]
 		}
 	}
-	if s != "" && s != "&" && s[len(s)-1] == '&' {
+	if s != "" && s[len(s)-1] == '&' && !hasTrailingSep {
 		s = s[:len(s)-1]
 	}
+	if reqPath != "" {
+		if s == "" {
+			s = reqPath
+			if s != "?" {
+				s = s[:len(s)-1]
+			}
+		} else {
+			if s[0] == '&' {
+				s = s[1:]
+			}
+			s = reqPath + s
+		}
+	}
 	return template.URL(s)
 }
 

modules/templates/helper_test.go

@@ -118,3 +118,58 @@ func TestTemplateEscape(t *testing.T) {
 		assert.Equal(t, `<a k="&#34;">&lt;&gt;</a>`, actual)
 	})
 }
+
+func TestQueryBuild(t *testing.T) {
+	t.Run("construct", func(t *testing.T) {
+		assert.Equal(t, "", string(QueryBuild()))
+		assert.Equal(t, "", string(QueryBuild("a", nil, "b", false, "c", 0, "d", "")))
+		assert.Equal(t, "a=1&b=true", string(QueryBuild("a", 1, "b", "true")))
+
+		// path with query parameters
+		assert.Equal(t, "/?k=1", string(QueryBuild("/", "k", 1)))
+		assert.Equal(t, "/", string(QueryBuild("/?k=a", "k", 0)))
+
+		// no path but question mark with query parameters
+		assert.Equal(t, "?k=1", string(QueryBuild("?", "k", 1)))
+		assert.Equal(t, "?", string(QueryBuild("?", "k", 0)))
+		assert.Equal(t, "path?k=1", string(QueryBuild("path?", "k", 1)))
+		assert.Equal(t, "path", string(QueryBuild("path?", "k", 0)))
+
+		// only query parameters
+		assert.Equal(t, "&k=1", string(QueryBuild("&", "k", 1)))
+		assert.Equal(t, "", string(QueryBuild("&", "k", 0)))
+		assert.Equal(t, "", string(QueryBuild("&k=a", "k", 0)))
+		assert.Equal(t, "", string(QueryBuild("k=a&", "k", 0)))
+		assert.Equal(t, "a=1&b=2", string(QueryBuild("a=1", "b", 2)))
+		assert.Equal(t, "&a=1&b=2", string(QueryBuild("&a=1", "b", 2)))
+		assert.Equal(t, "a=1&b=2&", string(QueryBuild("a=1&", "b", 2)))
+	})
+
+	t.Run("replace", func(t *testing.T) {
+		assert.Equal(t, "a=1&c=d&e=f", string(QueryBuild("a=b&c=d&e=f", "a", 1)))
+		assert.Equal(t, "a=b&c=1&e=f", string(QueryBuild("a=b&c=d&e=f", "c", 1)))
+		assert.Equal(t, "a=b&c=d&e=1", string(QueryBuild("a=b&c=d&e=f", "e", 1)))
+		assert.Equal(t, "a=b&c=d&e=f&k=1", string(QueryBuild("a=b&c=d&e=f", "k", 1)))
+	})
+
+	t.Run("replace-&", func(t *testing.T) {
+		assert.Equal(t, "&a=1&c=d&e=f", string(QueryBuild("&a=b&c=d&e=f", "a", 1)))
+		assert.Equal(t, "&a=b&c=1&e=f", string(QueryBuild("&a=b&c=d&e=f", "c", 1)))
+		assert.Equal(t, "&a=b&c=d&e=1", string(QueryBuild("&a=b&c=d&e=f", "e", 1)))
+		assert.Equal(t, "&a=b&c=d&e=f&k=1", string(QueryBuild("&a=b&c=d&e=f", "k", 1)))
+	})
+
+	t.Run("delete", func(t *testing.T) {
+		assert.Equal(t, "c=d&e=f", string(QueryBuild("a=b&c=d&e=f", "a", "")))
+		assert.Equal(t, "a=b&e=f", string(QueryBuild("a=b&c=d&e=f", "c", "")))
+		assert.Equal(t, "a=b&c=d", string(QueryBuild("a=b&c=d&e=f", "e", "")))
+		assert.Equal(t, "a=b&c=d&e=f", string(QueryBuild("a=b&c=d&e=f", "k", "")))
+	})
+
+	t.Run("delete-&", func(t *testing.T) {
+		assert.Equal(t, "&c=d&e=f", string(QueryBuild("&a=b&c=d&e=f", "a", "")))
+		assert.Equal(t, "&a=b&e=f", string(QueryBuild("&a=b&c=d&e=f", "c", "")))
+		assert.Equal(t, "&a=b&c=d", string(QueryBuild("&a=b&c=d&e=f", "e", "")))
+		assert.Equal(t, "&a=b&c=d&e=f", string(QueryBuild("&a=b&c=d&e=f", "k", "")))
+	})
+}

modules/util/path.go

@@ -203,6 +203,7 @@ func statDir(dirPath, recPath string, includeDir, isDirOnly, followSymlinks bool
 //
 // Slice does not include given path itself.
 // If subdirectories is enabled, they will have suffix '/'.
+// FIXME: it doesn't like dot-files, for example: "owner/.profile.git"
 func StatDir(rootPath string, includeDir ...bool) ([]string, error) {
 	if isDir, err := IsDir(rootPath); err != nil {
 		return nil, err

options/locale/locale_en-US.ini

@@ -1015,7 +1015,9 @@ new_repo_helper = A repository contains all project files, including revision hi
 owner = Owner
 owner_helper = Some organizations may not show up in the dropdown due to a maximum repository count limit.
 repo_name = Repository Name
-repo_name_helper = Good repository names use short, memorable and unique keywords.
+repo_name_profile_public_hint= .profile is a special repository that you can use to add README.md to your public organization profile, visible to anyone. Make sure it’s public and initialize it with a README in the profile directory to get started.
+repo_name_profile_private_hint = .profile-private is a special repository that you can use to add a README.md to your organization member profile, visible only to organization members. Make sure it’s private and initialize it with a README in the profile directory to get started.
+repo_name_helper = Good repository names use short, memorable and unique keywords. A repository named '.profile' or '.profile-private' could be used to add a README.md for the user/organization profile.
 repo_size = Repository Size
 template = Template
 template_select = Select a template.
@@ -2862,6 +2864,10 @@ teams.invite.title = You have been invited to join team <strong>%s</strong> in o
 teams.invite.by = Invited by %s
 teams.invite.description = Please click the button below to join the team.
 
+view_as_role = View as: %s
+view_as_public_hint = You are viewing the README a public user.
+view_as_member_hint = You are viewing the README a member of this organization.
+
 [admin]
 maintenance = Maintenance
 dashboard = Dashboard

routers/api/v1/repo/branch.go

@@ -729,7 +729,7 @@ func CreateBranchProtection(ctx *context.APIContext) {
 	} else {
 		if !isPlainRule {
 			if ctx.Repo.GitRepo == nil {
-				ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx, ctx.Repo.Repository)
+				ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx.Repo.Repository)
 				if err != nil {
 					ctx.Error(http.StatusInternalServerError, "OpenRepository", err)
 					return
@@ -1057,7 +1057,7 @@ func EditBranchProtection(ctx *context.APIContext) {
 	} else {
 		if !isPlainRule {
 			if ctx.Repo.GitRepo == nil {
-				ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx, ctx.Repo.Repository)
+				ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx.Repo.Repository)
 				if err != nil {
 					ctx.Error(http.StatusInternalServerError, "OpenRepository", err)
 					return

routers/api/v1/repo/compare.go

@@ -45,7 +45,7 @@ func CompareDiff(ctx *context.APIContext) {
 
 	if ctx.Repo.GitRepo == nil {
 		var err error
-		ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx, ctx.Repo.Repository)
+		ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx.Repo.Repository)
 		if err != nil {
 			ctx.Error(http.StatusInternalServerError, "OpenRepository", err)
 			return

routers/api/v1/repo/download.go

@@ -29,7 +29,7 @@ func DownloadArchive(ctx *context.APIContext) {
 
 	if ctx.Repo.GitRepo == nil {
 		var err error
-		ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx, ctx.Repo.Repository)
+		ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx.Repo.Repository)
 		if err != nil {
 			ctx.Error(http.StatusInternalServerError, "OpenRepository", err)
 			return

routers/api/v1/repo/file.go

@@ -282,7 +282,7 @@ func GetArchive(ctx *context.APIContext) {
 
 	if ctx.Repo.GitRepo == nil {
 		var err error
-		ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx, ctx.Repo.Repository)
+		ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx.Repo.Repository)
 		if err != nil {
 			ctx.Error(http.StatusInternalServerError, "OpenRepository", err)
 			return

routers/api/v1/repo/repo.go

@@ -726,7 +726,7 @@ func updateBasicProperties(ctx *context.APIContext, opts api.EditRepoOption) err
 
 	if ctx.Repo.GitRepo == nil && !repo.IsEmpty {
 		var err error
-		ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx, repo)
+		ctx.Repo.GitRepo, err = gitrepo.RepositoryFromRequestContextOrOpen(ctx, repo)
 		if err != nil {
 			ctx.Error(http.StatusInternalServerError, "Unable to OpenRepository", err)
 			return err

routers/private/internal_repo.go

@@ -27,7 +27,7 @@ func RepoAssignment(ctx *gitea_context.PrivateContext) {
 		return
 	}
 
-	gitRepo, err := gitrepo.RepositoryFromRequestContextOrOpen(ctx, ctx, repo)
+	gitRepo, err := gitrepo.RepositoryFromRequestContextOrOpen(ctx, repo)
 	if err != nil {
 		log.Error("Failed to open repository: %s/%s Error: %v", ownerName, repoName, err)
 		ctx.JSON(http.StatusInternalServerError, private.Response{