Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Repository avatars #6986

Merged
merged 38 commits into from
May 30, 2019
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
a2b9436
Repository avatars
sergey-dryabzhinsky May 19, 2019
c94c201
Add changed index.css, remove unused template name
sergey-dryabzhinsky May 19, 2019
259c995
Update en-us doc about configuration options
sergey-dryabzhinsky May 19, 2019
8ad6526
Add comments to new functions, add new option to docker app.ini
sergey-dryabzhinsky May 19, 2019
903c8ec
Add comment for lint
sergey-dryabzhinsky May 19, 2019
a16f49e
Remove variable, not needed
sergey-dryabzhinsky May 19, 2019
e8367ac
Fix formatting
sergey-dryabzhinsky May 19, 2019
822e33f
Update swagger api template
sergey-dryabzhinsky May 19, 2019
984199a
Check if avatar exists
sergey-dryabzhinsky May 19, 2019
253e7d1
Fix avatar link/path checks
sergey-dryabzhinsky May 19, 2019
53379c6
Typo
sergey-dryabzhinsky May 19, 2019
ef07e9b
TEXT column can't have a default value
sergey-dryabzhinsky May 19, 2019
3ccee72
Fixes:
sergey-dryabzhinsky May 19, 2019
52f877b
Fix fmt check
sergey-dryabzhinsky May 19, 2019
0a84f5c
Generate PNG instead of "static" GIF
sergey-dryabzhinsky May 19, 2019
a280eeb
More informative comment
sergey-dryabzhinsky May 19, 2019
b9e3514
Fix error message
sergey-dryabzhinsky May 19, 2019
32cc0f9
Update avatar upload checks:
sergey-dryabzhinsky May 19, 2019
b660e55
Fixes:
sergey-dryabzhinsky May 19, 2019
82b1333
Fix formatting
sergey-dryabzhinsky May 19, 2019
d71e535
Update comments
sergey-dryabzhinsky May 19, 2019
1ef191b
Update log message
sergey-dryabzhinsky May 19, 2019
4519847
Removed wrong style - not needed
sergey-dryabzhinsky May 19, 2019
025b66b
Use Sync2 to migrate
sergey-dryabzhinsky May 19, 2019
0c45c04
Update repos list view
sergey-dryabzhinsky May 20, 2019
89f9938
A little adjust avatar size
sergey-dryabzhinsky May 20, 2019
b2daa63
Use small icons for explore/repo list
sergey-dryabzhinsky May 24, 2019
1ddcd54
Merge branch 'gitea-master' into issue-694-repository-avatars
sergey-dryabzhinsky May 24, 2019
e023259
Merge branch 'gitea-master' into issue-694-repository-avatars
sergey-dryabzhinsky May 25, 2019
d3ea244
Use new cool avatar preparation func by @lafriks
sergey-dryabzhinsky May 25, 2019
30c0aab
Missing changes for new function
sergey-dryabzhinsky May 25, 2019
30bbe9f
Remove unused import, move imports
sergey-dryabzhinsky May 25, 2019
c3f386c
Missed new option definition in app.ini
sergey-dryabzhinsky May 26, 2019
4ae9383
Use smaller field length for Avatar
sergey-dryabzhinsky May 28, 2019
c007a9a
Use session to update repo DB data, update DeleteAvatar - use session…
sergey-dryabzhinsky May 29, 2019
01c3ef8
Fix err variable definition
sergey-dryabzhinsky May 29, 2019
f53758a
As suggested @lafriks - return as soon as possible, code readability
sergey-dryabzhinsky May 29, 2019
141d542
Merge branch 'master' into issue-694-repository-avatars
zeripath May 29, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions custom/conf/app.ini.sample
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,7 @@ SESSION_LIFE_TIME = 86400

[picture]
AVATAR_UPLOAD_PATH = data/avatars
REPOSITORY_AVATAR_UPLOAD_PATH = data/repo-avatars
lafriks marked this conversation as resolved.
Show resolved Hide resolved
; Max Width and Height of uploaded avatars. This is to limit the amount of RAM
; used when resizing the image.
AVATAR_MAX_WIDTH = 4096
Expand Down
1 change: 1 addition & 0 deletions docker/root/etc/templates/app.ini
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ PROVIDER_CONFIG = /data/gitea/sessions

[picture]
AVATAR_UPLOAD_PATH = /data/gitea/avatars
REPOSITORY_AVATAR_UPLOAD_PATH = /data/gitea/repo-avatars

[attachment]
PATH = /data/gitea/attachments
Expand Down
3 changes: 2 additions & 1 deletion docs/content/doc/advanced/config-cheat-sheet.en-us.md
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,8 @@ Values containing `#` or `;` must be quoted using `` ` `` or `"""`.
- `DISABLE_GRAVATAR`: **false**: Enable this to use local avatars only.
- `ENABLE_FEDERATED_AVATAR`: **false**: Enable support for federated avatars (see
[http://www.libravatar.org](http://www.libravatar.org)).
- `AVATAR_UPLOAD_PATH`: **data/avatars**: Path to store local and cached files.
- `AVATAR_UPLOAD_PATH`: **data/avatars**: Path to store user avatar image files.
- `REPOSITORY_AVATAR_UPLOAD_PATH`: **data/repo-avatars**: Path to store repository avatar image files.

## Attachment (`attachment`)

Expand Down
2 changes: 2 additions & 0 deletions models/migrations/migrations.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,8 @@ var migrations = []Migration{
NewMigration("hash application token", hashAppToken),
// v86 -> v87
NewMigration("add http method to webhook", addHTTPMethodToWebhook),
// v87 -> v88
NewMigration("add avatar field to repository", addAvatarFieldToRepository),
}

// Migrate database to current version
Expand Down
42 changes: 42 additions & 0 deletions models/migrations/v87.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Copyright 2017 Gitea. All rights reserved.
// Use of this source code is governed by a MIT-style
// license that can be found in the LICENSE file.

package migrations

import (
"fmt"

"github.com/go-xorm/xorm"
)

func addAvatarFieldToRepository(x *xorm.Engine) (err error) {
lunny marked this conversation as resolved.
Show resolved Hide resolved

sess := x.NewSession()
defer sess.Close()

if err := sess.Begin(); err != nil {
return err
}

dialect := x.Dialect().DriverName()

switch dialect {
case "mysql":
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I need to use case, Xorm can handle this. You can use Sync2 method

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

_, err = sess.Exec("ALTER TABLE repository ADD COLUMN `avatar` TEXT")
case "postgres":
_, err = sess.Exec("ALTER TABLE repository ADD COLUMN \"avatar\" VARCHAR")
case "tidb":
_, err = sess.Exec("ALTER TABLE repository ADD `avatar` TEXT")
case "mssql":
_, err = sess.Exec("ALTER TABLE repository ADD \"avatar\" VARCHAR")
case "sqlite3":
_, err = sess.Exec("ALTER TABLE repository ADD COLUMN `avatar` TEXT")
}

if err != nil {
return fmt.Errorf("Error changing mirror interval column type: %v", err)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we do need this migration you need to change this line.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

}

return sess.Commit()
}
132 changes: 132 additions & 0 deletions models/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,11 @@ package models

import (
"bytes"
"crypto/md5"
"errors"
"fmt"
"html/template"
"image"
"io/ioutil"
"net/url"
"os"
Expand All @@ -21,6 +23,11 @@ import (
"strings"
"time"

// Needed for jpeg support
_ "image/jpeg"
"image/png"

"code.gitea.io/gitea/modules/avatar"
"code.gitea.io/gitea/modules/git"
"code.gitea.io/gitea/modules/log"
"code.gitea.io/gitea/modules/markup"
Expand All @@ -34,6 +41,7 @@ import (
"github.com/Unknwon/com"
"github.com/go-xorm/builder"
"github.com/go-xorm/xorm"
"github.com/nfnt/resize"
ini "gopkg.in/ini.v1"
)

Expand Down Expand Up @@ -166,6 +174,9 @@ type Repository struct {
CloseIssuesViaCommitInAnyBranch bool `xorm:"NOT NULL DEFAULT false"`
Topics []string `xorm:"TEXT JSON"`

// Avatar
Avatar string `xorm:"VARCHAR(2048)"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does the full URL need to be stored in DB? For user avatars this makes sense due to gravatar (and other external avatar services), but for repo avatars it follows the following patter setting.AppSubURL + "/repo-avatars/" + repo.Avatar

Copy link
Contributor Author

@sergey-dryabzhinsky sergey-dryabzhinsky May 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean that it easily fits into 250 symbols?
ID+MD5 ~ (10-20) + 32.
64 symbols must be enough I think.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, rather than storing the full URL, just the filename could be stored.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 4ae9383


CreatedUnix util.TimeStamp `xorm:"INDEX created"`
UpdatedUnix util.TimeStamp `xorm:"INDEX updated"`
}
Expand Down Expand Up @@ -290,6 +301,7 @@ func (repo *Repository) innerAPIFormat(e Engine, mode AccessMode, isParent bool)
Created: repo.CreatedUnix.AsTime(),
Updated: repo.UpdatedUnix.AsTime(),
Permissions: permission,
AvatarURL: repo.AvatarLink(),
}
}

Expand Down Expand Up @@ -1869,6 +1881,15 @@ func DeleteRepository(doer *User, uid, repoID int64) error {
go HookQueue.Add(repo.ID)
}

if len(repo.Avatar) > 0 {
avatarPath := repo.CustomAvatarPath()
if com.IsExist(avatarPath) {
if err := os.Remove(avatarPath); err != nil {
return fmt.Errorf("Failed to remove %s: %v", avatarPath, err)
}
}
}

DeleteRepoFromIndexer(repo)
return nil
}
Expand Down Expand Up @@ -2452,3 +2473,114 @@ func (repo *Repository) GetUserFork(userID int64) (*Repository, error) {
}
return &forkedRepo, nil
}

// CustomAvatarPath returns repository custom avatar file path.
func (repo *Repository) CustomAvatarPath() string {
// Avatar empty by default
if len(repo.Avatar) <= 0 {
return ""
}
return filepath.Join(setting.RepositoryAvatarUploadPath, repo.Avatar)
}

// RelAvatarLink returns a relative link to the user's avatar.
// The link a sub-URL to this site
// Since Gravatar support not needed here - just check for image path.
func (repo *Repository) RelAvatarLink() string {
// If no avatar - path is empty
avatarPath := repo.CustomAvatarPath()
if len(avatarPath) <= 0 {
return ""
}
if !com.IsFile(avatarPath) {
return ""
}
return setting.AppSubURL + "/repo-avatars/" + repo.Avatar
}

// AvatarLink returns user avatar absolute link.
func (repo *Repository) AvatarLink() string {
link := repo.RelAvatarLink()
// link may be empty!
if len(link) > 0 {
if link[0] == '/' && link[1] != '/' {
return setting.AppURL + strings.TrimPrefix(link, setting.AppSubURL)[1:]
}
}
return link
}

// UploadAvatar saves custom avatar for user.
// FIXME: split uploads to different subdirs in case we have massive users.
func (repo *Repository) UploadAvatar(data []byte) error {
imgCfg, _, err := image.DecodeConfig(bytes.NewReader(data))
if err != nil {
return fmt.Errorf("DecodeConfig: %v", err)
}
if imgCfg.Width > setting.AvatarMaxWidth {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please reuse same code as recently added in user avatar upload

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated to latest master. That code not changed.
Снимок экрана_2019-05-24_21-30-39

Can you point me to the desired code?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, sorry I thought I merged #7025 already. It still needs @lunny approval

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now it's merged

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

return fmt.Errorf("Image width is to large: %d > %d", imgCfg.Width, setting.AvatarMaxWidth)
}
if imgCfg.Height > setting.AvatarMaxHeight {
return fmt.Errorf("Image height is to large: %d > %d", imgCfg.Height, setting.AvatarMaxHeight)
}

img, _, err := image.Decode(bytes.NewReader(data))
if err != nil {
return fmt.Errorf("Decode: %v", err)
}

m := resize.Resize(avatar.AvatarSize, avatar.AvatarSize, img, resize.NearestNeighbor)

sess := x.NewSession()
defer sess.Close()
if err = sess.Begin(); err != nil {
return err
}

oldAvatarPath := repo.CustomAvatarPath()

// Users can upload the same image to other repo - prefix it with ID
// Then repo will be removed - only it avatar file will be removed
repo.Avatar = fmt.Sprintf("%d-%x", repo.ID, md5.Sum(data))
if _, err := x.ID(repo.ID).Cols("avatar").Update(repo); err != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sess. not x.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in c007a9a

return fmt.Errorf("UpdateRepository: %v", err)
}

if err := os.MkdirAll(setting.RepositoryAvatarUploadPath, os.ModePerm); err != nil {
return fmt.Errorf("Failed to create dir %s: %v", setting.RepositoryAvatarUploadPath, err)
}

fw, err := os.Create(repo.CustomAvatarPath())
if err != nil {
return fmt.Errorf("Create: %v", err)
}
defer fw.Close()

if err = png.Encode(fw, m); err != nil {
return fmt.Errorf("Encode: %v", err)
}

if len(oldAvatarPath) > 0 && oldAvatarPath != repo.CustomAvatarPath() {
if err := os.Remove(oldAvatarPath); err != nil {
return fmt.Errorf("Failed to remove old %s: %v", oldAvatarPath, err)
}
}

return sess.Commit()
}

// DeleteAvatar deletes the user's custom avatar.
func (repo *Repository) DeleteAvatar() error {
log.Trace("DeleteAvatar[%d]: %s", repo.ID, repo.CustomAvatarPath())
if len(repo.Avatar) > 0 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please return as soon as possible for code readability:

Suggested change
if len(repo.Avatar) > 0 {
if len(repo.Avatar) == 0 {
return nil
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Point taken.
Done in f53758a

if err := os.Remove(repo.CustomAvatarPath()); err != nil {
return fmt.Errorf("Failed to remove %s: %v", repo.CustomAvatarPath(), err)
}
}

repo.Avatar = ""
if _, err := x.ID(repo.ID).Cols("avatar").Update(repo); err != nil {
return fmt.Errorf("UpdateRepository: %v", err)
}
return nil
}
39 changes: 39 additions & 0 deletions models/repo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@
package models

import (
"bytes"
"crypto/md5"
"fmt"
"image"
"image/png"
"testing"

"code.gitea.io/gitea/modules/markup"
Expand Down Expand Up @@ -158,3 +163,37 @@ func TestTransferOwnership(t *testing.T) {

CheckConsistencyFor(t, &Repository{}, &User{}, &Team{})
}

func TestUploadAvatar(t *testing.T) {

// Generate image
myImage := image.NewRGBA(image.Rect(0, 0, 1, 1))
var buff bytes.Buffer
png.Encode(&buff, myImage)

assert.NoError(t, PrepareTestDatabase())
repo := AssertExistsAndLoadBean(t, &Repository{ID: 10}).(*Repository)

err := repo.UploadAvatar(buff.Bytes())
assert.NoError(t, err)
assert.Equal(t, fmt.Sprintf("%d-%x", 10, md5.Sum(buff.Bytes())), repo.Avatar)
}

func TestDeleteAvatar(t *testing.T) {

// Generate image
myImage := image.NewRGBA(image.Rect(0, 0, 1, 1))
var buff bytes.Buffer
png.Encode(&buff, myImage)

assert.NoError(t, PrepareTestDatabase())
repo := AssertExistsAndLoadBean(t, &Repository{ID: 10}).(*Repository)

err := repo.UploadAvatar(buff.Bytes())
assert.NoError(t, err)

err = repo.DeleteAvatar()
assert.NoError(t, err)

assert.Equal(t, "", repo.Avatar)
}
22 changes: 14 additions & 8 deletions modules/setting/setting.go
Original file line number Diff line number Diff line change
Expand Up @@ -250,14 +250,15 @@ var (
}

// Picture settings
AvatarUploadPath string
AvatarMaxWidth int
AvatarMaxHeight int
GravatarSource string
GravatarSourceURL *url.URL
DisableGravatar bool
EnableFederatedAvatar bool
LibravatarService *libravatar.Libravatar
AvatarUploadPath string
AvatarMaxWidth int
AvatarMaxHeight int
GravatarSource string
GravatarSourceURL *url.URL
DisableGravatar bool
EnableFederatedAvatar bool
LibravatarService *libravatar.Libravatar
RepositoryAvatarUploadPath string

// Log settings
LogLevel string
Expand Down Expand Up @@ -835,6 +836,11 @@ func NewContext() {
if !filepath.IsAbs(AvatarUploadPath) {
AvatarUploadPath = path.Join(AppWorkPath, AvatarUploadPath)
}
RepositoryAvatarUploadPath = sec.Key("REPOSITORY_AVATAR_UPLOAD_PATH").MustString(path.Join(AppDataPath, "repo-avatars"))
forcePathSeparator(RepositoryAvatarUploadPath)
if !filepath.IsAbs(RepositoryAvatarUploadPath) {
RepositoryAvatarUploadPath = path.Join(AppWorkPath, RepositoryAvatarUploadPath)
}
AvatarMaxWidth = sec.Key("AVATAR_MAX_WIDTH").MustInt(4096)
AvatarMaxHeight = sec.Key("AVATAR_MAX_HEIGHT").MustInt(3072)
switch source := sec.Key("GRAVATAR_SOURCE").MustString("gravatar"); source {
Expand Down
1 change: 1 addition & 0 deletions modules/structs/repo.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ type Repository struct {
// swagger:strfmt date-time
Updated time.Time `json:"updated_at"`
Permissions *Permission `json:"permissions,omitempty"`
AvatarURL string `json:"avatar_url"`
}

// CreateRepoOption options when creating repository
Expand Down
1 change: 1 addition & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -1309,6 +1309,7 @@ settings.unarchive.header = Un-Archive This Repo
settings.unarchive.text = Un-Archiving the repo will restore its ability to recieve commits and pushes, as well as new issues and pull-requests.
settings.unarchive.success = The repo was successfully un-archived.
settings.unarchive.error = An error occured while trying to un-archive the repo. See the log for more details.
settings.update_avatar_success = The repository avatar has been updated.

diff.browse_source = Browse Source
diff.parent = parent
Expand Down
1 change: 1 addition & 0 deletions public/css/index.css
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,7 @@ footer .ui.left,footer .ui.right{line-height:40px}
.repository .repo-header .ui.compact.menu{margin-left:1rem}
.repository .repo-header .ui.header{margin-top:0}
.repository .repo-header .mega-octicon{width:30px;font-size:30px}
.repository .repo-header #repo-avatar{width:100px;height:100px;margin-right:15px}
.repository .repo-header .ui.huge.breadcrumb{font-weight:400;font-size:1.5rem}
.repository .repo-header .fork-flag{margin-left:36px;margin-top:3px;display:block;font-size:12px;white-space:nowrap}
.repository .repo-header .octicon.octicon-repo-forked{margin-top:-1px;font-size:15px}
Expand Down
6 changes: 6 additions & 0 deletions public/less/_repository.less
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@
font-size: 30px;
}

#repo-avatar {
width: 100px;
height: 100px;
margin-right: 15px;
}

.ui.huge.breadcrumb {
font-weight: 400;
font-size: 1.5rem;
Expand Down
Loading