From 49fd531420c58778d6e83e9bd30670c96f2d78ef Mon Sep 17 00:00:00 2001 From: Michael Mayer Date: Wed, 2 Jun 2021 17:25:04 +0200 Subject: [PATCH] People: Implement marker update API #22 --- frontend/src/css/results.css | 4 + frontend/src/dialog/photo/people.vue | 53 +++++++++---- frontend/src/model/photo.js | 26 +++++- internal/api/file_marker.go | 113 +++++++++++++++++++++++++++ internal/api/file_marker_test.go | 62 +++++++++++++++ internal/entity/album.go | 2 +- internal/entity/file.go | 11 +++ internal/entity/file_test.go | 10 +++ internal/entity/fixtures.go | 1 + internal/entity/marker.go | 25 ++++++ internal/entity/marker_fixtures.go | 61 +++++++++++++++ internal/form/marker.go | 20 +++++ internal/form/marker_test.go | 43 ++++++++++ internal/query/markers.go | 15 ++++ internal/server/routes.go | 1 + pkg/txt/convert.go | 31 -------- pkg/txt/convert_test.go | 33 -------- pkg/txt/int.go | 50 ++++++++++++ pkg/txt/int_test.go | 39 +++++++++ 19 files changed, 516 insertions(+), 84 deletions(-) create mode 100644 internal/api/file_marker.go create mode 100644 internal/api/file_marker_test.go create mode 100644 internal/entity/marker_fixtures.go create mode 100644 internal/form/marker.go create mode 100644 internal/form/marker_test.go create mode 100644 internal/query/markers.go create mode 100644 pkg/txt/int.go create mode 100644 pkg/txt/int_test.go diff --git a/frontend/src/css/results.css b/frontend/src/css/results.css index adcd28631..3e195ae63 100644 --- a/frontend/src/css/results.css +++ b/frontend/src/css/results.css @@ -358,4 +358,8 @@ body.chrome #photoprism .search-results .result { top: 50%; transform: translateY(-50%); text-align: center; +} + +#photoprism .face-results .invalid { + opacity: 0.5; } \ No newline at end of file diff --git a/frontend/src/dialog/photo/people.vue b/frontend/src/dialog/photo/people.vue index 3fd819aa7..7cb58d948 100644 --- a/frontend/src/dialog/photo/people.vue +++ b/frontend/src/dialog/photo/people.vue @@ -13,7 +13,7 @@ - +
- - + + + + + + + - - @@ -73,11 +80,12 @@ export default { }, data() { return { - markers: this.model.getMarkers(), + markers: this.model.getMarkers(true), imageUrl: this.model.thumbnailUrl("fit_720"), disabled: !this.$config.feature("edit"), config: this.$config.values, readonly: this.$config.get("readonly"), + textRule: v => v.length <= this.$config.get('clip') || this.$gettext("Text too long"), }; }, mounted () { @@ -115,10 +123,21 @@ export default { refresh() { }, reject(marker) { - this.$notify.warn("Work in progress"); + marker.Invalid = true; + this.model.updateMarker(marker); }, confirm(marker) { - this.$notify.warn("Work in progress"); + marker.Score = 100; + marker.Invalid = false; + this.model.updateMarker(marker); + }, + clearName(marker) { + marker.Label = ""; + marker.RefUID = ""; + this.model.updateMarker(marker); + }, + updateName(marker) { + this.model.updateMarker(marker); }, }, }; diff --git a/frontend/src/model/photo.js b/frontend/src/model/photo.js index 88e563f1d..b0ffb6a72 100644 --- a/frontend/src/model/photo.js +++ b/frontend/src/model/photo.js @@ -742,7 +742,7 @@ export class Photo extends RestModel { ); } - getMarkers() { + getMarkers(valid) { let result = []; let file = this.Files.find((f) => !!f.Primary); @@ -752,7 +752,9 @@ export class Photo extends RestModel { } file.Markers.forEach((m) => { - result.push(m); + if (!valid || !m.Invalid) { + result.push(m); + } }); return result; @@ -828,6 +830,26 @@ export class Photo extends RestModel { }); } + updateMarker(marker) { + if (!marker || !marker.ID) { + return Promise.reject("invalid marker id"); + } + + marker.MarkerSrc = SrcManual; + + const file = this.mainFile(); + + if (!file || !file.UID) { + return Promise.reject("invalid file uid"); + } + + const url = `${this.getEntityResource()}/files/${file.UID}/markers/${marker.ID}`; + + return Api.put(url, marker).then((resp) => { + return Promise.resolve(this.setValues(resp.data)); + }); + } + static batchSize() { return 60; } diff --git a/internal/api/file_marker.go b/internal/api/file_marker.go new file mode 100644 index 000000000..dd8d3c3ad --- /dev/null +++ b/internal/api/file_marker.go @@ -0,0 +1,113 @@ +package api + +import ( + "net/http" + + "github.com/gin-gonic/gin" + "github.com/photoprism/photoprism/internal/acl" + "github.com/photoprism/photoprism/internal/event" + "github.com/photoprism/photoprism/internal/form" + "github.com/photoprism/photoprism/internal/i18n" + "github.com/photoprism/photoprism/internal/query" + "github.com/photoprism/photoprism/internal/service" + "github.com/photoprism/photoprism/pkg/txt" +) + +// PUT /api/v1/photos/:uid/files/:file_uid/markers/:id +// +// Parameters: +// uid: string Photo UID as returned by the API +// file_uid: string File UID as returned by the API +// id: int Marker ID as returned by the API +func UpdateFileMarker(router *gin.RouterGroup) { + router.PUT("/photos/:uid/files/:file_uid/markers/:id", func(c *gin.Context) { + s := Auth(SessionID(c), acl.ResourceFiles, acl.ActionUpdate) + + if s.Invalid() { + AbortUnauthorized(c) + return + } + + conf := service.Config() + + if !conf.Settings().Features.Edit { + AbortFeatureDisabled(c) + return + } + + photoUID := c.Param("uid") + fileUID := c.Param("file_uid") + markerID := txt.UInt(c.Param("id")) + + if photoUID == "" || fileUID == "" || markerID < 1 { + AbortBadRequest(c) + return + } + + file, err := query.FileByUID(fileUID) + + if err != nil { + log.Errorf("photo: %s (update marker)", err) + AbortEntityNotFound(c) + return + } + + if !file.FilePrimary { + log.Errorf("photo: can't update markers for non-primary files") + AbortBadRequest(c) + return + } else if file.PhotoUID != photoUID { + log.Errorf("photo: file uid doesn't match") + AbortBadRequest(c) + return + } + + marker, err := query.MarkerByID(markerID) + + if err != nil { + log.Errorf("photo: %s (update marker)", err) + AbortEntityNotFound(c) + return + } + + markerForm, err := form.NewMarker(marker) + + if err != nil { + log.Errorf("photo: %s (new marker form)", err) + AbortSaveFailed(c) + return + } + + if err := c.BindJSON(&markerForm); err != nil { + log.Errorf("photo: %s (update marker form)", err) + AbortBadRequest(c) + return + } + + if err := marker.SaveForm(markerForm); err != nil { + log.Errorf("photo: %s (save marker form)", err) + AbortSaveFailed(c) + return + } + + event.SuccessMsg(i18n.MsgChangesSaved) + + if p, err := query.PhotoPreloadByUID(photoUID); err != nil { + AbortEntityNotFound(c) + return + } else { + if faceCount := file.FaceCount(); p.PhotoFaces == faceCount { + // Do nothing. + } else if err := p.Update("PhotoFaces", faceCount); err != nil { + log.Errorf("photo: %s (update face count)", err) + } else { + // Notify clients by publishing events. + PublishPhotoEvent(EntityUpdated, photoUID, c) + + p.PhotoFaces = faceCount + } + + c.JSON(http.StatusOK, p) + } + }) +} diff --git a/internal/api/file_marker_test.go b/internal/api/file_marker_test.go new file mode 100644 index 000000000..7c4e72aaa --- /dev/null +++ b/internal/api/file_marker_test.go @@ -0,0 +1,62 @@ +package api + +import ( + "encoding/json" + "fmt" + "net/http" + "testing" + + "github.com/tidwall/gjson" + + "github.com/stretchr/testify/assert" +) + +func TestUpdateFileMarker(t *testing.T) { + t.Run("success", func(t *testing.T) { + app, router, _ := NewApiTest() + + GetPhoto(router) + UpdateFileMarker(router) + + r := PerformRequest(app, "GET", "/api/v1/photos/pt9jtdre2lvl0y11") + + assert.Equal(t, http.StatusOK, r.Code) + + photoUID := gjson.Get(r.Body.String(), "UID").String() + fileUID := gjson.Get(r.Body.String(), "Files.0.UID").String() + markerID := gjson.Get(r.Body.String(), "Files.0.Markers.0.ID").String() + + assert.NotEmpty(t, photoUID) + assert.NotEmpty(t, fileUID) + assert.NotEmpty(t, markerID) + + u := fmt.Sprintf("/api/v1/photos/%s/files/%s/markers/%s", photoUID, fileUID, markerID) + + var m = struct { + RefUID string + RefSrc string + MarkerSrc string + MarkerType string + MarkerScore int + MarkerInvalid bool + MarkerLabel string + }{ + RefUID: "3h59wvth837b5vyiub35", + RefSrc: "meta", + MarkerSrc: "image", + MarkerType: "Face", + MarkerScore: 100, + MarkerInvalid: true, + MarkerLabel: "Foo", + } + + if b, err := json.Marshal(m); err != nil { + t.Fatal(err) + } else { + t.Logf("PUT %s", u) + r = PerformRequestWithBody(app, "PUT", u, string(b)) + } + + assert.Equal(t, http.StatusOK, r.Code) + }) +} diff --git a/internal/entity/album.go b/internal/entity/album.go index 4ffd09038..9f815c740 100644 --- a/internal/entity/album.go +++ b/internal/entity/album.go @@ -313,7 +313,7 @@ func (m *Album) SetTitle(title string) { } } -// Saves the entity using form data and stores it in the database. +// SaveForm updates the entity using form data and stores it in the database. func (m *Album) SaveForm(f form.Album) error { if err := deepcopier.Copy(m).From(f); err != nil { return err diff --git a/internal/entity/file.go b/internal/entity/file.go index 2e65e5890..e488d4c10 100644 --- a/internal/entity/file.go +++ b/internal/entity/file.go @@ -412,6 +412,17 @@ func (m *File) AddFace(f face.Face, refUID string) { } } +// FaceCount returns the current number of valid faces detected. +func (m *File) FaceCount() (c int) { + if err := Db().Model(Marker{}).Where("marker_invalid = 0 AND file_id = ?", m.ID). + Count(&c).Error; err != nil { + log.Errorf("file: %s (count faces)", err) + return 0 + } else { + return c + } +} + // PreloadMarkers loads existing file markers. func (m *File) PreloadMarkers() { if res, err := FindMarkers(m.ID); err != nil { diff --git a/internal/entity/file_test.go b/internal/entity/file_test.go index 42adf7068..d6580ec1e 100644 --- a/internal/entity/file_test.go +++ b/internal/entity/file_test.go @@ -447,3 +447,13 @@ func TestFile_AddFaces(t *testing.T) { assert.NotEmpty(t, file.Markers) }) } + +func TestFile_FaceCount(t *testing.T) { + t.Run("FileFixturesExampleBridge", func(t *testing.T) { + file := FileFixturesExampleBridge + + result := file.FaceCount() + + assert.GreaterOrEqual(t, result, 3) + }) +} diff --git a/internal/entity/fixtures.go b/internal/entity/fixtures.go index 6f85d38d1..852a7300a 100644 --- a/internal/entity/fixtures.go +++ b/internal/entity/fixtures.go @@ -24,4 +24,5 @@ func CreateTestFixtures() { CreateFileShareFixtures() CreateFileSyncFixtures() CreateLensFixtures() + CreateMarkerFixtures() } diff --git a/internal/entity/marker.go b/internal/entity/marker.go index d38ff1b93..0915f1a2d 100644 --- a/internal/entity/marker.go +++ b/internal/entity/marker.go @@ -4,6 +4,10 @@ import ( "fmt" "time" + "github.com/photoprism/photoprism/internal/form" + "github.com/photoprism/photoprism/pkg/txt" + "github.com/ulule/deepcopier" + "github.com/photoprism/photoprism/internal/face" ) @@ -18,6 +22,7 @@ type Marker struct { ID uint `gorm:"primary_key" json:"ID" yaml:"-"` FileID uint `gorm:"index;" json:"-" yaml:"-"` RefUID string `gorm:"type:VARBINARY(42);index;" json:"RefUID" yaml:"RefUID,omitempty"` + RefSrc string `gorm:"type:VARBINARY(8);default:'';" json:"RefSrc" yaml:"RefSrc,omitempty"` MarkerSrc string `gorm:"type:VARBINARY(8);default:'';" json:"Src" yaml:"Src,omitempty"` MarkerType string `gorm:"type:VARBINARY(8);default:'';" json:"Type" yaml:"Type"` MarkerScore int `gorm:"type:SMALLINT" json:"Score" yaml:"Score"` @@ -32,6 +37,9 @@ type Marker struct { UpdatedAt time.Time } +// UnknownMarker can be used as a default for unknown markers. +var UnknownMarker = NewMarker(0, "", SrcAuto, MarkerUnknown, 0, 0, 0, 0) + // TableName returns the entity database table name. func (Marker) TableName() string { return "markers_dev" @@ -75,6 +83,23 @@ func (m *Marker) Update(attr string, value interface{}) error { return UnscopedDb().Model(m).UpdateColumn(attr, value).Error } +// SaveForm updates the entity using form data and stores it in the database. +func (m *Marker) SaveForm(f form.Marker) error { + if err := deepcopier.Copy(m).From(f); err != nil { + return err + } + + if f.MarkerLabel != "" { + m.MarkerLabel = txt.Title(txt.Clip(f.MarkerLabel, txt.ClipKeyword)) + } + + if err := m.Save(); err != nil { + return err + } + + return nil +} + // Save updates the existing or inserts a new row. func (m *Marker) Save() error { if m.X == 0 || m.Y == 0 || m.X > 1 || m.Y > 1 || m.X < -1 || m.Y < -1 { diff --git a/internal/entity/marker_fixtures.go b/internal/entity/marker_fixtures.go new file mode 100644 index 000000000..51af4feb5 --- /dev/null +++ b/internal/entity/marker_fixtures.go @@ -0,0 +1,61 @@ +package entity + +type MarkerMap map[string]Marker + +func (m MarkerMap) Get(name string) Marker { + if result, ok := m[name]; ok { + return result + } + + return *UnknownMarker +} + +func (m MarkerMap) Pointer(name string) *Marker { + if result, ok := m[name]; ok { + return &result + } + + return UnknownMarker +} + +var MarkerFixtures = MarkerMap{ + "1000003-1": Marker{ + FileID: 1000003, + RefUID: "lt9k3pw1wowuy3c3", + MarkerSrc: SrcImage, + MarkerType: MarkerLabel, + X: 0.308333, + Y: 0.206944, + W: 0.355556, + H: .355556, + }, + "1000003-2": Marker{ + FileID: 1000003, + RefUID: "", + MarkerLabel: "Unknown", + MarkerSrc: SrcImage, + MarkerType: MarkerLabel, + X: 0.208333, + Y: 0.106944, + W: 0.05, + H: 0.05, + }, + "1000003-3": Marker{ + FileID: 1000003, + RefUID: "", + MarkerSrc: SrcImage, + MarkerType: MarkerLabel, + MarkerLabel: "Center", + X: 0.5, + Y: 0.5, + W: 0, + H: 0, + }, +} + +// CreateMarkerFixtures inserts known entities into the database for testing. +func CreateMarkerFixtures() { + for _, entity := range MarkerFixtures { + Db().Create(&entity) + } +} diff --git a/internal/form/marker.go b/internal/form/marker.go new file mode 100644 index 000000000..cd846b2cf --- /dev/null +++ b/internal/form/marker.go @@ -0,0 +1,20 @@ +package form + +import "github.com/ulule/deepcopier" + +// Marker represents an image marker edit form. +type Marker struct { + RefUID string `json:"RefUID"` + RefSrc string `json:"RefSrc"` + MarkerSrc string `json:"Src"` + MarkerType string `json:"Type"` + MarkerScore int `json:"Score"` + MarkerInvalid bool `json:"Invalid"` + MarkerLabel string `json:"Label"` +} + +func NewMarker(m interface{}) (f Marker, err error) { + err = deepcopier.Copy(m).To(&f) + + return f, err +} diff --git a/internal/form/marker_test.go b/internal/form/marker_test.go new file mode 100644 index 000000000..6c01e25b5 --- /dev/null +++ b/internal/form/marker_test.go @@ -0,0 +1,43 @@ +package form + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestNewMarker(t *testing.T) { + t.Run("success", func(t *testing.T) { + var m = struct { + RefUID string + RefSrc string + MarkerSrc string + MarkerType string + MarkerScore int + MarkerInvalid bool + MarkerLabel string + }{ + RefUID: "3h59wvth837b5vyiub35", + RefSrc: "meta", + MarkerSrc: "image", + MarkerType: "Face", + MarkerScore: 100, + MarkerInvalid: true, + MarkerLabel: "Foo", + } + + f, err := NewMarker(m) + + if err != nil { + t.Fatal(err) + } + + assert.Equal(t, "3h59wvth837b5vyiub35", f.RefUID) + assert.Equal(t, "meta", f.RefSrc) + assert.Equal(t, "image", f.MarkerSrc) + assert.Equal(t, "Face", f.MarkerType) + assert.Equal(t, 100, f.MarkerScore) + assert.Equal(t, true, f.MarkerInvalid) + assert.Equal(t, "Foo", f.MarkerLabel) + }) +} diff --git a/internal/query/markers.go b/internal/query/markers.go new file mode 100644 index 000000000..5b4cad397 --- /dev/null +++ b/internal/query/markers.go @@ -0,0 +1,15 @@ +package query + +import ( + "github.com/photoprism/photoprism/internal/entity" +) + +// MarkerByID returns a Marker based on the ID. +func MarkerByID(id uint) (marker entity.Marker, err error) { + if err := UnscopedDb().Where("id = ?", id). + First(&marker).Error; err != nil { + return marker, err + } + + return marker, nil +} diff --git a/internal/server/routes.go b/internal/server/routes.go index e513cbe81..71ecb3365 100644 --- a/internal/server/routes.go +++ b/internal/server/routes.go @@ -72,6 +72,7 @@ func registerRoutes(router *gin.Engine, conf *config.Config) { api.GetMomentsTime(v1) api.GetFile(v1) api.DeleteFile(v1) + api.UpdateFileMarker(v1) api.PhotoPrimary(v1) api.PhotoUnstack(v1) diff --git a/pkg/txt/convert.go b/pkg/txt/convert.go index 1338b1c08..61c12a8c1 100644 --- a/pkg/txt/convert.go +++ b/pkg/txt/convert.go @@ -2,43 +2,12 @@ package txt import ( "regexp" - "strconv" "strings" ) var UnknownCountryCode = "zz" var CountryWordsRegexp = regexp.MustCompile("[\\p{L}]{2,}") -// Int returns a string as int or 0 if it can not be converted. -func Int(s string) int { - if s == "" { - return 0 - } - - result, err := strconv.ParseInt(s, 10, 32) - - if err != nil { - return 0 - } - - return int(result) -} - -// IsUInt returns true if a string only contains an unsigned integer. -func IsUInt(s string) bool { - if s == "" { - return false - } - - for _, r := range s { - if r < 48 || r > 57 { - return false - } - } - - return true -} - // CountryCode tries to find a matching country code for a given string e.g. from a file oder directory name. func CountryCode(s string) (code string) { code = UnknownCountryCode diff --git a/pkg/txt/convert_test.go b/pkg/txt/convert_test.go index 7b0c20737..b3458e86b 100644 --- a/pkg/txt/convert_test.go +++ b/pkg/txt/convert_test.go @@ -6,33 +6,6 @@ import ( "github.com/stretchr/testify/assert" ) -func TestInt(t *testing.T) { - t.Run("empty", func(t *testing.T) { - result := Int("") - assert.Equal(t, 0, result) - }) - - t.Run("non-numeric", func(t *testing.T) { - result := Int("Screenshot") - assert.Equal(t, 0, result) - }) - - t.Run("zero", func(t *testing.T) { - result := Int("0") - assert.Equal(t, 0, result) - }) - - t.Run("int", func(t *testing.T) { - result := Int("123") - assert.Equal(t, 123, result) - }) - - t.Run("negative int", func(t *testing.T) { - result := Int("-123") - assert.Equal(t, -123, result) - }) -} - func TestCountryCode(t *testing.T) { t.Run("London", func(t *testing.T) { result := CountryCode("London") @@ -134,9 +107,3 @@ func TestCountryCode(t *testing.T) { assert.Equal(t, "it", result) }) } - -func TestIsUInt(t *testing.T) { - assert.False(t, IsUInt("")) - assert.False(t, IsUInt("12 3")) - assert.True(t, IsUInt("123")) -} diff --git a/pkg/txt/int.go b/pkg/txt/int.go new file mode 100644 index 000000000..a44340e15 --- /dev/null +++ b/pkg/txt/int.go @@ -0,0 +1,50 @@ +package txt + +import ( + "strconv" +) + +// Int converts a string to a signed integer or 0 if invalid. +func Int(s string) int { + if s == "" { + return 0 + } + + result, err := strconv.ParseInt(s, 10, 32) + + if err != nil { + return 0 + } + + return int(result) +} + +// UInt converts a string to an unsigned integer or 0 if invalid. +func UInt(s string) uint { + if s == "" { + return 0 + } + + result, err := strconv.ParseInt(s, 10, 32) + + if err != nil || result < 0 { + return 0 + } + + return uint(result) +} + +// IsUInt tests if a string represents an unsigned integer. +func IsUInt(s string) bool { + if s == "" { + return false + } + + for _, r := range s { + if r < 48 || r > 57 { + return false + } + } + + return true +} diff --git a/pkg/txt/int_test.go b/pkg/txt/int_test.go new file mode 100644 index 000000000..4427b464d --- /dev/null +++ b/pkg/txt/int_test.go @@ -0,0 +1,39 @@ +package txt + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestInt(t *testing.T) { + t.Run("empty", func(t *testing.T) { + result := Int("") + assert.Equal(t, 0, result) + }) + + t.Run("non-numeric", func(t *testing.T) { + result := Int("Screenshot") + assert.Equal(t, 0, result) + }) + + t.Run("zero", func(t *testing.T) { + result := Int("0") + assert.Equal(t, 0, result) + }) + + t.Run("int", func(t *testing.T) { + result := Int("123") + assert.Equal(t, 123, result) + }) + + t.Run("negative int", func(t *testing.T) { + result := Int("-123") + assert.Equal(t, -123, result) + }) +} +func TestIsUInt(t *testing.T) { + assert.False(t, IsUInt("")) + assert.False(t, IsUInt("12 3")) + assert.True(t, IsUInt("123")) +}