Improve API

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
+	}
 
-	new := reflect.ValueOf(src).Elem()
-	for i := 0; i < new.NumField(); i++ {
-		field := new.Field(i)
+	if override.SiteURL != "" && override.SiteURL != f.SiteURL {
+		f.SiteURL = override.SiteURL
+	}
 
-		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.FeedURL != "" && override.FeedURL != f.FeedURL {
+		f.FeedURL = override.FeedURL
+	}
+
+	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
+	}
+
+	if override.Category != nil && override.Category.ID != 0 && override.Category.ID != f.Category.ID {
+		f.Category.ID = override.Category.ID
 	}
 }
 

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`)
+	}
+}

model/theme.go

@@ -22,5 +22,5 @@ func ValidateTheme(theme string) error {
 		}
 	}
 
-	return errors.NewLocalizedError("Invalid theme.")
+	return errors.NewLocalizedError("Invalid theme")
 }

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 u.Password != "" {
+		return u.ValidatePassword()
 	}
 
-	if err := u.ValidatePassword(); err != nil {
-		return err
-	}
-
-	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
 	}
 }

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`)
-	}
-
-	user = &User{ID: 42, Username: "test", Password: "", Theme: "default"}
 	if err := user.ValidateUserModification(); err != nil {
-		t.Error(`User without password should not generate an error`)
+		t.Error(`There is no changes, so we should not have an error`)
 	}
 
-	user = &User{ID: 42, Username: "test", Password: "a", Theme: "default"}
-	if err := user.ValidateUserModification(); err == nil {
-		t.Error(`Passwords shorter than 6 characters should generate an error`)
+	user = &User{Theme: "default"}
+	if err := user.ValidateUserModification(); err != nil {
+		t.Error(`A valid theme should not generate any errors`)
 	}
 
-	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{ID: -1, Username: "test", Password: "secret", Theme: "default"}
-	if err := user.ValidateUserModification(); err == nil {
-		t.Error(`An invalid userID 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`)
-	}
-
-	user = &User{ID: 42, Username: "test", Password: "secret", Theme: "invalid"}
+	user = &User{Theme: "invalid theme"}
 	if err := user.ValidateUserModification(); err == nil {
 		t.Error(`An invalid theme should generate an error`)
 	}
 
-	user = &User{ID: 42, Username: "test", Password: "secret", Theme: "default"}
+	user = &User{Password: "test123"}
 	if err := user.ValidateUserModification(); err != nil {
-		t.Error(`A valid user should not generate any error`)
+		t.Error(`A valid password should not generate any errors`)
+	}
+
+	user = &User{Password: "a"}
+	if err := user.ValidateUserModification(); err == nil {
+		t.Error(`An invalid password 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`)
+	}
+
+	if user1.Theme != "default" {
+		t.Fatal(`The theme should not be merged into user1`)
+	}
+}
+
+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`)
 	}
 }