diff --git a/internal/api/abort.go b/internal/api/abort.go index 3e74071f0..562233570 100644 --- a/internal/api/abort.go +++ b/internal/api/abort.go @@ -110,6 +110,10 @@ func AbortBusy(c *gin.Context) { Abort(c, http.StatusTooManyRequests, i18n.ErrBusy) } +func AbortInvalidName(c *gin.Context) { + Abort(c, http.StatusBadRequest, i18n.ErrInvalidName) +} + func AbortInvalidCredentials(c *gin.Context) { if c != nil { c.AbortWithStatusJSON(http.StatusUnauthorized, gin.H{"error": authn.ErrInvalidCredentials.Error(), "code": i18n.ErrInvalidCredentials, "message": i18n.Msg(i18n.ErrInvalidCredentials)}) diff --git a/internal/api/labels.go b/internal/api/labels.go index 3272670ab..6039bd8e9 100644 --- a/internal/api/labels.go +++ b/internal/api/labels.go @@ -46,22 +46,25 @@ func UpdateLabel(router *gin.RouterGroup) { } // Create new label form. - f, formErr := form.NewLabel(m) + frm, frmErr := form.NewLabel(m) - if formErr != nil { + if frmErr != nil { Abort(c, http.StatusBadRequest, i18n.ErrBadRequest) return } // Set form values from request. - if formErr = c.BindJSON(f); formErr != nil { + if frmErr = c.BindJSON(frm); frmErr != nil { AbortBadRequest(c) return + } else if frmErr = frm.Validate(); frmErr != nil { + AbortInvalidName(c) + return } // Save label and return new model values if successful. - if err = m.SaveForm(f); err != nil { - log.Error(err) + if err = m.SaveForm(frm); err != nil { + log.Errorf("label: %s", clean.Error(err)) AbortSaveFailed(c) return } diff --git a/internal/api/photo_label.go b/internal/api/photo_label.go index c97b65e4c..68754e431 100644 --- a/internal/api/photo_label.go +++ b/internal/api/photo_label.go @@ -49,12 +49,15 @@ func AddPhotoLabel(router *gin.RouterGroup) { if err = c.BindJSON(frm); err != nil { AbortBadRequest(c) return + } else if err = frm.Validate(); err != nil { + AbortInvalidName(c) + return } labelEntity := entity.FirstOrCreateLabel(entity.NewLabel(frm.LabelName, frm.LabelPriority)) if labelEntity == nil { - c.AbortWithStatusJSON(http.StatusInternalServerError, gin.H{"error": "failed to create label"}) + AbortInvalidName(c) return } diff --git a/internal/entity/entity_errors.go b/internal/entity/entity_errors.go new file mode 100644 index 000000000..8b32cf936 --- /dev/null +++ b/internal/entity/entity_errors.go @@ -0,0 +1,7 @@ +package entity + +import "fmt" + +var ( + ErrInvalidName = fmt.Errorf("invalid name") +) diff --git a/internal/entity/label.go b/internal/entity/label.go index 78be628c3..14cece83d 100644 --- a/internal/entity/label.go +++ b/internal/entity/label.go @@ -114,8 +114,8 @@ func (m *Label) Save() error { func (m *Label) SaveForm(f *form.Label) error { if f == nil { return fmt.Errorf("form is nil") - } else if f.LabelName == "" { - return fmt.Errorf("missing name") + } else if f.LabelName == "" || txt.Slug(f.LabelName) == "" { + return ErrInvalidName } labelMutex.Lock() @@ -125,9 +125,11 @@ func (m *Label) SaveForm(f *form.Label) error { return err } - m.SetName(f.LabelName) - - return Db().Save(m).Error + if m.SetName(f.LabelName) { + return Db().Save(m).Error + } else { + return ErrInvalidName + } } // Create inserts the label to the database. @@ -202,10 +204,14 @@ func (m *Label) Update(attr string, value interface{}) error { // FirstOrCreateLabel returns the existing label, inserts a new label or nil in case of errors. func FirstOrCreateLabel(m *Label) *Label { + if m.LabelSlug == "" && m.CustomSlug == "" { + return nil + } + result := &Label{} if err := UnscopedDb(). - Where("label_slug = ? OR (custom_slug <> '' AND custom_slug = ? OR label_slug <> '' AND label_slug = ?)", m.LabelSlug, m.CustomSlug, m.LabelSlug). + Where("(custom_slug <> '' AND custom_slug = ? OR label_slug <> '' AND label_slug = ?)", m.CustomSlug, m.LabelSlug). First(result).Error; err == nil { return result } else if createErr := m.Create(); createErr == nil { @@ -219,7 +225,7 @@ func FirstOrCreateLabel(m *Label) *Label { return m } else if err = UnscopedDb(). - Where("label_slug = ? OR (custom_slug <> '' AND custom_slug = ? OR label_slug <> '' AND label_slug = ?)", m.LabelSlug, m.CustomSlug, m.LabelSlug). + Where("(custom_slug <> '' AND custom_slug = ? OR label_slug <> '' AND label_slug = ?)", m.CustomSlug, m.LabelSlug). First(result).Error; err == nil { return result } else { @@ -230,15 +236,44 @@ func FirstOrCreateLabel(m *Label) *Label { } // SetName changes the label name. -func (m *Label) SetName(name string) { - name = clean.NameCapitalized(name) +func (m *Label) SetName(name string) bool { + labelName := txt.Clip(clean.NameCapitalized(name), txt.ClipName) - if name == "" { - return + if labelName == "" { + return false } - m.LabelName = txt.Clip(name, txt.ClipName) - m.CustomSlug = txt.Slug(name) + labelSlug := txt.Slug(labelName) + + if labelSlug == "" { + return false + } + + m.LabelName = labelName + m.CustomSlug = labelSlug + + if m.LabelSlug == "" { + m.LabelSlug = labelSlug + } + + return true +} + +// InvalidName checks if the label name is invalid. +func (m *Label) InvalidName() bool { + labelName := txt.Clip(clean.NameCapitalized(m.LabelName), txt.ClipName) + + if labelName == "" { + return true + } + + labelSlug := txt.Slug(labelName) + + if labelSlug == "" { + return true + } + + return false } // GetSlug returns the label slug. @@ -271,8 +306,11 @@ func (m *Label) UpdateClassify(label classify.Label) error { } if m.CustomSlug == m.LabelSlug && label.Title() != m.LabelName { - m.SetName(label.Title()) - save = true + if m.SetName(label.Title()) { + save = true + } else { + return ErrInvalidName + } } // Save label. diff --git a/internal/entity/label_test.go b/internal/entity/label_test.go index 1497f0f4f..1504cead0 100644 --- a/internal/entity/label_test.go +++ b/internal/entity/label_test.go @@ -44,7 +44,6 @@ func TestLabel_SetName(t *testing.T) { assert.Equal(t, "landscape", entity.LabelSlug) assert.Equal(t, "landschaft", entity.CustomSlug) }) - t.Run("new name empty", func(t *testing.T) { entity := LabelFixtures["flower"] @@ -52,7 +51,7 @@ func TestLabel_SetName(t *testing.T) { assert.Equal(t, "flower", entity.LabelSlug) assert.Equal(t, "flower", entity.CustomSlug) - entity.SetName("") + assert.False(t, entity.SetName("")) assert.Equal(t, "Flower", entity.LabelName) assert.Equal(t, "flower", entity.LabelSlug) diff --git a/internal/entity/photo.go b/internal/entity/photo.go index cfe10d11a..97f657f65 100644 --- a/internal/entity/photo.go +++ b/internal/entity/photo.go @@ -692,7 +692,7 @@ func (m *Photo) AddLabels(labels classify.Labels) { labelEntity := FirstOrCreateLabel(NewLabel(classifyLabel.Title(), classifyLabel.Priority)) if labelEntity == nil { - log.Errorf("index: label %s should not be nil - you may have found a bug (%s)", clean.Log(classifyLabel.Title()), m) + log.Errorf("index: label %s coud not be created (%s)", clean.Log(classifyLabel.Title()), m) continue } diff --git a/internal/entity/photo_label.go b/internal/entity/photo_label.go index 3a338334f..cf0111a3f 100644 --- a/internal/entity/photo_label.go +++ b/internal/entity/photo_label.go @@ -50,8 +50,10 @@ func (m *PhotoLabel) Save() error { m.Photo = nil } - if m.Label != nil { - m.Label.SetName(m.Label.LabelName) + if m.Label == nil { + // Do nothing. + } else if !m.Label.SetName(m.Label.LabelName) { + return ErrInvalidName } return Db().Save(m).Error diff --git a/internal/form/label.go b/internal/form/label.go index 9c425babd..d92728e6b 100644 --- a/internal/form/label.go +++ b/internal/form/label.go @@ -1,6 +1,11 @@ package form -import "github.com/ulule/deepcopier" +import ( + "github.com/photoprism/photoprism/pkg/clean" + "github.com/photoprism/photoprism/pkg/i18n" + "github.com/photoprism/photoprism/pkg/txt" + "github.com/ulule/deepcopier" +) // Label represents a label edit form. type Label struct { @@ -20,3 +25,20 @@ func NewLabel(m interface{}) (*Label, error) { err := deepcopier.Copy(m).To(frm) return frm, err } + +// Validate returns an error if any form values are invalid. +func (frm *Label) Validate() error { + labelName := txt.Clip(clean.NameCapitalized(frm.LabelName), txt.ClipName) + + if labelName == "" { + return i18n.Error(i18n.ErrInvalidName) + } + + labelSlug := txt.Slug(labelName) + + if labelSlug == "" { + return i18n.Error(i18n.ErrInvalidName) + } + + return nil +}