From d5b8f2fb8839189bdf6893da0f86f3bb26001d3d Mon Sep 17 00:00:00 2001 From: Frédéric Guillot Date: Sun, 24 Dec 2017 18:04:34 -0800 Subject: Improve API --- model/feed.go | 58 +++++++++++++++++++++++++++++------------------------ model/feed_test.go | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ model/theme.go | 2 +- model/user.go | 26 ++++++++++-------------- model/user_test.go | 57 ++++++++++++++++++++++++++++++++++------------------ 5 files changed, 140 insertions(+), 62 deletions(-) create mode 100644 model/feed_test.go (limited to 'model') diff --git a/model/feed.go b/model/feed.go index 8d15d7a..c2ee6e8 100644 --- a/model/feed.go +++ b/model/feed.go @@ -6,7 +6,6 @@ package model import ( "fmt" - "reflect" "time" ) @@ -17,17 +16,17 @@ type Feed struct { FeedURL string `json:"feed_url"` SiteURL string `json:"site_url"` Title string `json:"title"` - CheckedAt time.Time `json:"checked_at,omitempty"` - EtagHeader string `json:"etag_header,omitempty"` - LastModifiedHeader string `json:"last_modified_header,omitempty"` - ParsingErrorMsg string `json:"parsing_error_message,omitempty"` - ParsingErrorCount int `json:"parsing_error_count,omitempty"` + CheckedAt time.Time `json:"checked_at"` + EtagHeader string `json:"etag_header"` + LastModifiedHeader string `json:"last_modified_header"` + ParsingErrorMsg string `json:"parsing_error_message"` + ParsingErrorCount int `json:"parsing_error_count"` ScraperRules string `json:"scraper_rules"` RewriteRules string `json:"rewrite_rules"` Crawler bool `json:"crawler"` Category *Category `json:"category,omitempty"` Entries Entries `json:"entries,omitempty"` - Icon *FeedIcon `json:"icon,omitempty"` + Icon *FeedIcon `json:"icon"` } func (f *Feed) String() string { @@ -41,27 +40,34 @@ func (f *Feed) String() string { ) } -// Merge combine src to the current struct -func (f *Feed) Merge(src *Feed) { - src.ID = f.ID - src.UserID = f.UserID +// Merge combine override to the current struct +func (f *Feed) Merge(override *Feed) { + if override.Title != "" && override.Title != f.Title { + f.Title = override.Title + } + + if override.SiteURL != "" && override.SiteURL != f.SiteURL { + f.SiteURL = override.SiteURL + } + + if override.FeedURL != "" && override.FeedURL != f.FeedURL { + f.FeedURL = override.FeedURL + } - new := reflect.ValueOf(src).Elem() - for i := 0; i < new.NumField(); i++ { - field := new.Field(i) + if override.ScraperRules != "" && override.ScraperRules != f.ScraperRules { + f.ScraperRules = override.ScraperRules + } + + if override.RewriteRules != "" && override.RewriteRules != f.RewriteRules { + f.RewriteRules = override.RewriteRules + } + + if override.Crawler != f.Crawler { + f.Crawler = override.Crawler + } - switch field.Interface().(type) { - case int64: - value := field.Int() - if value != 0 { - reflect.ValueOf(f).Elem().Field(i).SetInt(value) - } - case string: - value := field.String() - if value != "" { - reflect.ValueOf(f).Elem().Field(i).SetString(value) - } - } + if override.Category != nil && override.Category.ID != 0 && override.Category.ID != f.Category.ID { + f.Category.ID = override.Category.ID } } diff --git a/model/feed_test.go b/model/feed_test.go new file mode 100644 index 0000000..138949d --- /dev/null +++ b/model/feed_test.go @@ -0,0 +1,59 @@ +// Copyright 2017 Frédéric Guillot. All rights reserved. +// Use of this source code is governed by the Apache 2.0 +// license that can be found in the LICENSE file. + +package model + +import "testing" + +func TestMergeFeedTitle(t *testing.T) { + feed1 := &Feed{Title: "Feed 1"} + feed2 := &Feed{Title: "Feed 2"} + feed1.Merge(feed2) + + if feed1.Title != "Feed 2" { + t.Fatal(`The title of feed1 should be merged`) + } + + feed1 = &Feed{Title: "Feed 1"} + feed2 = &Feed{} + feed1.Merge(feed2) + + if feed1.Title != "Feed 1" { + t.Fatal(`The title of feed1 should not be merged`) + } + + feed1 = &Feed{Title: "Feed 1"} + feed2 = &Feed{Title: "Feed 1"} + feed1.Merge(feed2) + + if feed1.Title != "Feed 1" { + t.Fatal(`The title of feed1 should not be changed`) + } +} + +func TestMergeFeedCategory(t *testing.T) { + feed1 := &Feed{Category: &Category{ID: 222}} + feed2 := &Feed{Category: &Category{ID: 333}} + feed1.Merge(feed2) + + if feed1.Category.ID != 333 { + t.Fatal(`The category of feed1 should be merged`) + } + + feed1 = &Feed{Category: &Category{ID: 222}} + feed2 = &Feed{} + feed1.Merge(feed2) + + if feed1.Category.ID != 222 { + t.Fatal(`The category of feed1 should not be merged`) + } + + feed1 = &Feed{Category: &Category{ID: 222}} + feed2 = &Feed{Category: &Category{ID: 0}} + feed1.Merge(feed2) + + if feed1.Category.ID != 222 { + t.Fatal(`The category of feed1 should not be merged`) + } +} diff --git a/model/theme.go b/model/theme.go index 6b87ec8..f4990e5 100644 --- a/model/theme.go +++ b/model/theme.go @@ -22,5 +22,5 @@ func ValidateTheme(theme string) error { } } - return errors.NewLocalizedError("Invalid theme.") + return errors.NewLocalizedError("Invalid theme") } diff --git a/model/user.go b/model/user.go index 4889dc4..8026a36 100644 --- a/model/user.go +++ b/model/user.go @@ -37,21 +37,17 @@ func (u User) ValidateUserCreation() error { return u.ValidatePassword() } -// ValidateUserModification validates user for modification. +// ValidateUserModification validates user modification payload. func (u User) ValidateUserModification() error { - if u.ID <= 0 { - return errors.New("The ID is mandatory") + if u.Theme != "" { + return ValidateTheme(u.Theme) } - if u.Username == "" { - return errors.New("The username is mandatory") - } - - if err := u.ValidatePassword(); err != nil { - return err + if u.Password != "" { + return u.ValidatePassword() } - return ValidateTheme(u.Theme) + return nil } // ValidateUserLogin validates user credential requirements. @@ -78,11 +74,11 @@ func (u User) ValidatePassword() error { // Merge update the current user with another user. func (u *User) Merge(override *User) { - if u.Username != override.Username { + if override.Username != "" && u.Username != override.Username { u.Username = override.Username } - if u.Password != override.Password { + if override.Password != "" && u.Password != override.Password { u.Password = override.Password } @@ -90,15 +86,15 @@ func (u *User) Merge(override *User) { u.IsAdmin = override.IsAdmin } - if u.Theme != override.Theme { + if override.Theme != "" && u.Theme != override.Theme { u.Theme = override.Theme } - if u.Language != override.Language { + if override.Language != "" && u.Language != override.Language { u.Language = override.Language } - if u.Timezone != override.Timezone { + if override.Timezone != "" && u.Timezone != override.Timezone { u.Timezone = override.Timezone } } diff --git a/model/user_test.go b/model/user_test.go index 769696f..ac075a8 100644 --- a/model/user_test.go +++ b/model/user_test.go @@ -35,42 +35,59 @@ func TestValidateUserCreation(t *testing.T) { func TestValidateUserModification(t *testing.T) { user := &User{} - if err := user.ValidateUserModification(); err == nil { - t.Error(`An empty user should generate an error`) + if err := user.ValidateUserModification(); err != nil { + t.Error(`There is no changes, so we should not have an error`) } - user = &User{ID: 42, Username: "test", Password: "", Theme: "default"} + user = &User{Theme: "default"} if err := user.ValidateUserModification(); err != nil { - t.Error(`User without password should not generate an error`) + t.Error(`A valid theme should not generate any errors`) } - user = &User{ID: 42, Username: "test", Password: "a", Theme: "default"} + user = &User{Theme: "invalid theme"} if err := user.ValidateUserModification(); err == nil { - t.Error(`Passwords shorter than 6 characters should generate an error`) + t.Error(`An invalid theme should generate an error`) } - user = &User{ID: 42, Username: "", Password: "secret", Theme: "default"} - if err := user.ValidateUserModification(); err == nil { - t.Error(`An empty username should generate an error`) + user = &User{Password: "test123"} + if err := user.ValidateUserModification(); err != nil { + t.Error(`A valid password should not generate any errors`) } - user = &User{ID: -1, Username: "test", Password: "secret", Theme: "default"} + user = &User{Password: "a"} if err := user.ValidateUserModification(); err == nil { - t.Error(`An invalid userID should generate an error`) + t.Error(`An invalid password should generate an error`) } +} - user = &User{ID: 0, Username: "test", Password: "secret", Theme: "default"} - if err := user.ValidateUserModification(); err == nil { - t.Error(`An invalid userID should generate an error`) +func TestMergeUsername(t *testing.T) { + user1 := &User{ID: 42, Username: "user1", Password: "secret", Theme: "default"} + user2 := &User{ID: 42, Username: "user2"} + user1.Merge(user2) + + if user1.Username != "user2" { + t.Fatal(`The username should be merged into user1`) } - user = &User{ID: 42, Username: "test", Password: "secret", Theme: "invalid"} - if err := user.ValidateUserModification(); err == nil { - t.Error(`An invalid theme should generate an error`) + if user1.Theme != "default" { + t.Fatal(`The theme should not be merged into user1`) } +} - user = &User{ID: 42, Username: "test", Password: "secret", Theme: "default"} - if err := user.ValidateUserModification(); err != nil { - t.Error(`A valid user should not generate any error`) +func TestMergeIsAdmin(t *testing.T) { + user1 := &User{ID: 42, Username: "user1", Password: "secret", Theme: "default"} + user2 := &User{ID: 42, IsAdmin: true} + user1.Merge(user2) + + if !user1.IsAdmin { + t.Fatal(`The is_admin flag should be merged into user1`) + } + + user1 = &User{ID: 42, Username: "user1", Password: "secret", Theme: "default"} + user2 = &User{ID: 42} + user1.Merge(user2) + + if user1.IsAdmin { + t.Fatal(`The is_admin flag should not be merged into user1`) } } -- cgit v1.2.3