Improve feed and user API updates with optional values

4 files changed, 317 insertions, 22 deletions

diff --git a/api/feed.go b/api/feed.go
index 351457f..b7c46ea 100644
--- a/api/feed.go
+++ b/api/feed.go
@@ -97,7 +97,7 @@ func (c *Controller) UpdateFeed(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	newFeed, err := decodeFeedModificationPayload(r.Body)
+	feedChanges, err := decodeFeedModificationPayload(r.Body)
 	if err != nil {
 		json.BadRequest(w, err)
 		return
@@ -106,11 +106,6 @@ func (c *Controller) UpdateFeed(w http.ResponseWriter, r *http.Request) {
 	ctx := context.New(r)
 	userID := ctx.UserID()
 
-	if newFeed.Category != nil && newFeed.Category.ID != 0 && !c.store.CategoryExists(userID, newFeed.Category.ID) {
-		json.BadRequest(w, errors.New("This category_id doesn't exists or doesn't belongs to this user"))
-		return
-	}
-
 	originalFeed, err := c.store.FeedByID(userID, feedID)
 	if err != nil {
 		json.NotFound(w, errors.New("Unable to find this feed"))
@@ -122,7 +117,13 @@ func (c *Controller) UpdateFeed(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	originalFeed.Merge(newFeed)
+	feedChanges.Update(originalFeed)
+
+	if !c.store.CategoryExists(userID, originalFeed.Category.ID) {
+		json.BadRequest(w, errors.New("This category_id doesn't exists or doesn't belongs to this user"))
+		return
+	}
+
 	if err := c.store.UpdateFeed(originalFeed); err != nil {
 		json.ServerError(w, errors.New("Unable to update this feed"))
 		return
diff --git a/api/payload.go b/api/payload.go
index 45cb826..ed5255a 100644
--- a/api/payload.go
+++ b/api/payload.go
@@ -37,13 +37,115 @@ type subscriptionDiscovery struct {
 	Password string `json:"password"`
 }
 
-func decodeUserPayload(r io.ReadCloser) (*model.User, error) {
-	var user model.User
+type feedModification struct {
+	FeedURL      *string `json:"feed_url"`
+	SiteURL      *string `json:"site_url"`
+	Title        *string `json:"title"`
+	ScraperRules *string `json:"scraper_rules"`
+	RewriteRules *string `json:"rewrite_rules"`
+	Crawler      *bool   `json:"crawler"`
+	Username     *string `json:"username"`
+	Password     *string `json:"password"`
+	CategoryID   *int64  `json:"category_id"`
+}
+
+func (f *feedModification) Update(feed *model.Feed) {
+	if f.FeedURL != nil && *f.FeedURL != "" {
+		feed.FeedURL = *f.FeedURL
+	}
+
+	if f.SiteURL != nil && *f.SiteURL != "" {
+		feed.SiteURL = *f.SiteURL
+	}
+
+	if f.Title != nil && *f.Title != "" {
+		feed.Title = *f.Title
+	}
+
+	if f.ScraperRules != nil {
+		feed.ScraperRules = *f.ScraperRules
+	}
+
+	if f.RewriteRules != nil {
+		feed.RewriteRules = *f.RewriteRules
+	}
+
+	if f.Crawler != nil {
+		feed.Crawler = *f.Crawler
+	}
+
+	if f.Username != nil {
+		feed.Username = *f.Username
+	}
 
+	if f.Password != nil {
+		feed.Password = *f.Password
+	}
+
+	if f.CategoryID != nil && *f.CategoryID > 0 {
+		feed.Category.ID = *f.CategoryID
+	}
+}
+
+type userModification struct {
+	Username       *string `json:"username"`
+	Password       *string `json:"password"`
+	IsAdmin        *bool   `json:"is_admin"`
+	Theme          *string `json:"theme"`
+	Language       *string `json:"language"`
+	Timezone       *string `json:"timezone"`
+	EntryDirection *string `json:"entry_sorting_direction"`
+}
+
+func (u *userModification) Update(user *model.User) {
+	if u.Username != nil {
+		user.Username = *u.Username
+	}
+
+	if u.Password != nil {
+		user.Password = *u.Password
+	}
+
+	if u.IsAdmin != nil {
+		user.IsAdmin = *u.IsAdmin
+	}
+
+	if u.Theme != nil {
+		user.Theme = *u.Theme
+	}
+
+	if u.Language != nil {
+		user.Language = *u.Language
+	}
+
+	if u.Timezone != nil {
+		user.Timezone = *u.Timezone
+	}
+
+	if u.EntryDirection != nil {
+		user.EntryDirection = *u.EntryDirection
+	}
+}
+
+func decodeUserModificationPayload(r io.ReadCloser) (*userModification, error) {
+	defer r.Close()
+
+	var user userModification
 	decoder := json.NewDecoder(r)
+	if err := decoder.Decode(&user); err != nil {
+		return nil, fmt.Errorf("Unable to decode user modification JSON object: %v", err)
+	}
+
+	return &user, nil
+}
+
+func decodeUserCreationPayload(r io.ReadCloser) (*model.User, error) {
 	defer r.Close()
+
+	var user model.User
+	decoder := json.NewDecoder(r)
 	if err := decoder.Decode(&user); err != nil {
-		return nil, fmt.Errorf("Unable to decode user JSON object: %v", err)
+		return nil, fmt.Errorf("Unable to decode user modification JSON object: %v", err)
 	}
 
 	return &user, nil
@@ -89,13 +191,13 @@ func decodeFeedCreationPayload(r io.ReadCloser) (*feedCreation, error) {
 	return &fc, nil
 }
 
-func decodeFeedModificationPayload(r io.ReadCloser) (*model.Feed, error) {
-	var feed model.Feed
+func decodeFeedModificationPayload(r io.ReadCloser) (*feedModification, error) {
+	defer r.Close()
 
+	var feed feedModification
 	decoder := json.NewDecoder(r)
-	defer r.Close()
 	if err := decoder.Decode(&feed); err != nil {
-		return nil, fmt.Errorf("Unable to decode feed JSON object: %v", err)
+		return nil, fmt.Errorf("Unable to decode feed modification JSON object: %v", err)
 	}
 
 	return &feed, nil
diff --git a/api/payload_test.go b/api/payload_test.go
new file mode 100644
index 0000000..debf897
--- /dev/null
+++ b/api/payload_test.go
@@ -0,0 +1,192 @@
+// Copyright 2018 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 api
+
+import (
+	"testing"
+
+	"github.com/miniflux/miniflux/model"
+)
+
+func TestUpdateFeedURL(t *testing.T) {
+	feedURL := "http://example.com/"
+	changes := &feedModification{FeedURL: &feedURL}
+	feed := &model.Feed{FeedURL: "http://example.org/"}
+	changes.Update(feed)
+
+	if feed.FeedURL != feedURL {
+		t.Fatalf(`Unexpected value, got %q instead of %q`, feed.FeedURL, feedURL)
+	}
+}
+
+func TestUpdateFeedURLWithEmptyString(t *testing.T) {
+	feedURL := ""
+	changes := &feedModification{FeedURL: &feedURL}
+	feed := &model.Feed{FeedURL: "http://example.org/"}
+	changes.Update(feed)
+
+	if feed.FeedURL == feedURL {
+		t.Fatal(`The FeedURL should not be modified`)
+	}
+}
+
+func TestUpdateFeedURLWhenNotSet(t *testing.T) {
+	changes := &feedModification{}
+	feed := &model.Feed{FeedURL: "http://example.org/"}
+	changes.Update(feed)
+
+	if feed.FeedURL != "http://example.org/" {
+		t.Fatal(`The FeedURL should not be modified`)
+	}
+}
+
+func TestUpdateFeedSiteURL(t *testing.T) {
+	siteURL := "http://example.com/"
+	changes := &feedModification{SiteURL: &siteURL}
+	feed := &model.Feed{SiteURL: "http://example.org/"}
+	changes.Update(feed)
+
+	if feed.SiteURL != siteURL {
+		t.Fatalf(`Unexpected value, got %q instead of %q`, feed.SiteURL, siteURL)
+	}
+}
+
+func TestUpdateFeedSiteURLWithEmptyString(t *testing.T) {
+	siteURL := ""
+	changes := &feedModification{FeedURL: &siteURL}
+	feed := &model.Feed{SiteURL: "http://example.org/"}
+	changes.Update(feed)
+
+	if feed.SiteURL == siteURL {
+		t.Fatal(`The FeedURL should not be modified`)
+	}
+}
+
+func TestUpdateFeedSiteURLWhenNotSet(t *testing.T) {
+	changes := &feedModification{}
+	feed := &model.Feed{SiteURL: "http://example.org/"}
+	changes.Update(feed)
+
+	if feed.SiteURL != "http://example.org/" {
+		t.Fatal(`The SiteURL should not be modified`)
+	}
+}
+
+func TestUpdateFeedTitle(t *testing.T) {
+	title := "Example 2"
+	changes := &feedModification{Title: &title}
+	feed := &model.Feed{Title: "Example"}
+	changes.Update(feed)
+
+	if feed.Title != title {
+		t.Fatalf(`Unexpected value, got %q instead of %q`, feed.Title, title)
+	}
+}
+
+func TestUpdateFeedTitleWithEmptyString(t *testing.T) {
+	title := ""
+	changes := &feedModification{Title: &title}
+	feed := &model.Feed{Title: "Example"}
+	changes.Update(feed)
+
+	if feed.Title == title {
+		t.Fatal(`The Title should not be modified`)
+	}
+}
+
+func TestUpdateFeedTitleWhenNotSet(t *testing.T) {
+	changes := &feedModification{}
+	feed := &model.Feed{Title: "Example"}
+	changes.Update(feed)
+
+	if feed.Title != "Example" {
+		t.Fatal(`The Title should not be modified`)
+	}
+}
+
+func TestUpdateFeedUsername(t *testing.T) {
+	username := "Alice"
+	changes := &feedModification{Username: &username}
+	feed := &model.Feed{Username: "Bob"}
+	changes.Update(feed)
+
+	if feed.Username != username {
+		t.Fatalf(`Unexpected value, got %q instead of %q`, feed.Username, username)
+	}
+}
+
+func TestUpdateFeedUsernameWithEmptyString(t *testing.T) {
+	username := ""
+	changes := &feedModification{Username: &username}
+	feed := &model.Feed{Username: "Bob"}
+	changes.Update(feed)
+
+	if feed.Username != "" {
+		t.Fatal(`The Username should be empty now`)
+	}
+}
+
+func TestUpdateFeedUsernameWhenNotSet(t *testing.T) {
+	changes := &feedModification{}
+	feed := &model.Feed{Username: "Alice"}
+	changes.Update(feed)
+
+	if feed.Username != "Alice" {
+		t.Fatal(`The Username should not be modified`)
+	}
+}
+
+func TestUpdateFeedCategory(t *testing.T) {
+	categoryID := int64(1)
+	changes := &feedModification{CategoryID: &categoryID}
+	feed := &model.Feed{Category: &model.Category{ID: 42}}
+	changes.Update(feed)
+
+	if feed.Category.ID != categoryID {
+		t.Fatalf(`Unexpected value, got %q instead of %q`, feed.Username, categoryID)
+	}
+}
+
+func TestUpdateFeedCategoryWithZero(t *testing.T) {
+	categoryID := int64(0)
+	changes := &feedModification{CategoryID: &categoryID}
+	feed := &model.Feed{Category: &model.Category{ID: 42}}
+	changes.Update(feed)
+
+	if feed.Category.ID != 42 {
+		t.Fatal(`The CategoryID should not be modified`)
+	}
+}
+
+func TestUpdateFeedCategoryWhenNotSet(t *testing.T) {
+	changes := &feedModification{}
+	feed := &model.Feed{Category: &model.Category{ID: 42}}
+	changes.Update(feed)
+
+	if feed.Category.ID != 42 {
+		t.Fatal(`The CategoryID should not be modified`)
+	}
+}
+
+func TestUpdateUserTheme(t *testing.T) {
+	theme := "Example 2"
+	changes := &userModification{Theme: &theme}
+	user := &model.User{Theme: "Example"}
+	changes.Update(user)
+
+	if user.Theme != theme {
+		t.Fatalf(`Unexpected value, got %q instead of %q`, user.Theme, theme)
+	}
+}
+
+func TestUserThemeWhenNotSet(t *testing.T) {
+	changes := &userModification{}
+	user := &model.User{Theme: "Example"}
+	changes.Update(user)
+
+	if user.Theme != "Example" {
+		t.Fatal(`The user Theme should not be modified`)
+	}
+}
diff --git a/api/user.go b/api/user.go
index a231e8c..4e5fa82 100644
--- a/api/user.go
+++ b/api/user.go
@@ -33,7 +33,7 @@ func (c *Controller) CreateUser(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	user, err := decodeUserPayload(r.Body)
+	user, err := decodeUserCreationPayload(r.Body)
 	if err != nil {
 		json.BadRequest(w, err)
 		return
@@ -73,17 +73,12 @@ func (c *Controller) UpdateUser(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	user, err := decodeUserPayload(r.Body)
+	userChanges, err := decodeUserModificationPayload(r.Body)
 	if err != nil {
 		json.BadRequest(w, err)
 		return
 	}
 
-	if err := user.ValidateUserModification(); err != nil {
-		json.BadRequest(w, err)
-		return
-	}
-
 	originalUser, err := c.store.UserByID(userID)
 	if err != nil {
 		json.BadRequest(w, errors.New("Unable to fetch this user from the database"))
@@ -95,7 +90,12 @@ func (c *Controller) UpdateUser(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	originalUser.Merge(user)
+	userChanges.Update(originalUser)
+	if err := originalUser.ValidateUserModification(); err != nil {
+		json.BadRequest(w, err)
+		return
+	}
+
 	if err = c.store.UpdateUser(originalUser); err != nil {
 		json.ServerError(w, errors.New("Unable to update this user"))
 		return