Adding etag to channel extra_info api call.

7 files changed, 176 insertions, 23 deletions

diff --git a/api/channel.go b/api/channel.go
index 123fd8a35..9216dbb09 100644
--- a/api/channel.go
+++ b/api/channel.go
@@ -570,6 +570,11 @@ func getChannelExtraInfo(c *Context, w http.ResponseWriter, r *http.Request) {
 		channel := cresult.Data.(*model.Channel)
 		member := cmresult.Data.(model.ChannelMember)
 		extraMembers := ecmresult.Data.([]model.ExtraMember)
+		extraEtag := channel.ExtraEtag()
+
+		if HandleEtag(extraEtag, w, r) {
+			return
+		}
 
 		if !c.HasPermissionsToTeam(channel.TeamId, "getChannelExtraInfo") {
 			return
@@ -586,6 +591,7 @@ func getChannelExtraInfo(c *Context, w http.ResponseWriter, r *http.Request) {
 		}
 
 		data := model.ChannelExtra{Id: channel.Id, Members: extraMembers}
+		w.Header().Set(model.HEADER_ETAG_SERVER, extraEtag)
 		w.Header().Set("Expires", "-1")
 		w.Write([]byte(data.ToJson()))
 	}
@@ -711,7 +717,7 @@ func removeChannelMember(c *Context, w http.ResponseWriter, r *http.Request) {
 		}
 
 		message := model.NewMessage(c.Session.TeamId, "", userId, model.ACTION_USER_REMOVED)
-		message.Add("channel_id",id)
+		message.Add("channel_id", id)
 		message.Add("remover", c.Session.UserId)
 		PublishAndForget(message)
 
diff --git a/api/channel_benchmark_test.go b/api/channel_benchmark_test.go
index 881638176..77e679c14 100644
--- a/api/channel_benchmark_test.go
+++ b/api/channel_benchmark_test.go
@@ -189,7 +189,7 @@ func BenchmarkGetChannelExtraInfo(b *testing.B) {
 	b.ResetTimer()
 	for i := 0; i < b.N; i++ {
 		for j := range channels {
-			Client.Must(Client.GetChannelExtraInfo(channels[j].Id))
+			Client.Must(Client.GetChannelExtraInfo(channels[j].Id, ""))
 		}
 	}
 }
diff --git a/api/channel_test.go b/api/channel_test.go
index a8d53c4b5..d4fb11bd8 100644
--- a/api/channel_test.go
+++ b/api/channel_test.go
@@ -6,6 +6,7 @@ package api
 import (
 	"github.com/mattermost/platform/model"
 	"github.com/mattermost/platform/store"
+	"github.com/mattermost/platform/utils"
 	"net/http"
 	"testing"
 	"time"
@@ -543,10 +544,73 @@ func TestGetChannelExtraInfo(t *testing.T) {
 	channel1 := &model.Channel{DisplayName: "A Test API Name", Name: "a" + model.NewId() + "a", Type: model.CHANNEL_OPEN, TeamId: team.Id}
 	channel1 = Client.Must(Client.CreateChannel(channel1)).Data.(*model.Channel)
 
-	rget := Client.Must(Client.GetChannelExtraInfo(channel1.Id)).Data.(*model.ChannelExtra)
-	if rget.Id != channel1.Id {
+	rget := Client.Must(Client.GetChannelExtraInfo(channel1.Id, ""))
+	data := rget.Data.(*model.ChannelExtra)
+	if data.Id != channel1.Id {
 		t.Fatal("couldnt't get extra info")
 	}
+
+	//
+	// Testing etag caching
+	//
+
+	currentEtag := rget.Etag
+
+	if cache_result, err := Client.GetChannelExtraInfo(channel1.Id, currentEtag); err != nil {
+		t.Fatal(err)
+	} else if cache_result.Data.(*model.ChannelExtra) != nil {
+		t.Log(cache_result.Data)
+		t.Fatal("response should be empty")
+	} else {
+		currentEtag = cache_result.Etag
+	}
+
+	Client2 := model.NewClient("http://localhost:" + utils.Cfg.ServiceSettings.Port + "/api/v1")
+
+	user2 := &model.User{TeamId: team.Id, Email: model.NewId() + "tester2@test.com", Nickname: "Tester 2", Password: "pwd"}
+	user2 = Client2.Must(Client2.CreateUser(user2, "")).Data.(*model.User)
+	store.Must(Srv.Store.User().VerifyEmail(user2.Id))
+
+	Client2.LoginByEmail(team.Name, user2.Email, "pwd")
+	Client2.Must(Client2.JoinChannel(channel1.Id))
+
+	if cache_result, err := Client.GetChannelExtraInfo(channel1.Id, currentEtag); err != nil {
+		t.Fatal(err)
+	} else if cache_result.Data.(*model.ChannelExtra) == nil {
+		t.Log(cache_result.Data)
+		t.Fatal("response should not be empty")
+	} else {
+		currentEtag = cache_result.Etag
+	}
+
+	if cache_result, err := Client.GetChannelExtraInfo(channel1.Id, currentEtag); err != nil {
+		t.Fatal(err)
+	} else if cache_result.Data.(*model.ChannelExtra) != nil {
+		t.Log(cache_result.Data)
+		t.Fatal("response should be empty")
+	} else {
+		currentEtag = cache_result.Etag
+	}
+
+	Client2.Must(Client2.LeaveChannel(channel1.Id))
+
+	if cache_result, err := Client.GetChannelExtraInfo(channel1.Id, currentEtag); err != nil {
+		t.Fatal(err)
+	} else if cache_result.Data.(*model.ChannelExtra) == nil {
+		t.Log(cache_result.Data)
+		t.Fatal("response should not be empty")
+	} else {
+		currentEtag = cache_result.Etag
+	}
+
+	if cache_result, err := Client.GetChannelExtraInfo(channel1.Id, currentEtag); err != nil {
+		t.Fatal(err)
+	} else if cache_result.Data.(*model.ChannelExtra) != nil {
+		t.Log(cache_result.Data)
+		t.Fatal("response should be empty")
+	} else {
+		currentEtag = cache_result.Etag
+	}
 }
 
 func TestAddChannelMember(t *testing.T) {
diff --git a/model/channel.go b/model/channel.go
index ab36d46a2..94b8fdb43 100644
--- a/model/channel.go
+++ b/model/channel.go
@@ -27,6 +27,7 @@ type Channel struct {
 	Description   string `json:"description"`
 	LastPostAt    int64  `json:"last_post_at"`
 	TotalMsgCount int64  `json:"total_msg_count"`
+	ExtraUpdateAt int64  `json:"extra_update_at"`
 }
 
 func (o *Channel) ToJson() string {
@@ -50,7 +51,11 @@ func ChannelFromJson(data io.Reader) *Channel {
 }
 
 func (o *Channel) Etag() string {
-	return Etag(o.Id, o.LastPostAt)
+	return Etag(o.Id, o.UpdateAt)
+}
+
+func (o *Channel) ExtraEtag() string {
+	return Etag(o.Id, o.ExtraUpdateAt)
 }
 
 func (o *Channel) IsValid() *AppError {
@@ -97,8 +102,13 @@ func (o *Channel) PreSave() {
 
 	o.CreateAt = GetMillis()
 	o.UpdateAt = o.CreateAt
+	o.ExtraUpdateAt = o.CreateAt
 }
 
 func (o *Channel) PreUpdate() {
 	o.UpdateAt = GetMillis()
 }
+
+func (o *Channel) ExtraUpdated() {
+	o.ExtraUpdateAt = GetMillis()
+}
diff --git a/model/client.go b/model/client.go
index 17bb898ca..a5016fa2c 100644
--- a/model/client.go
+++ b/model/client.go
@@ -457,8 +457,8 @@ func (c *Client) UpdateLastViewedAt(channelId string) (*Result, *AppError) {
 	}
 }
 
-func (c *Client) GetChannelExtraInfo(id string) (*Result, *AppError) {
-	if r, err := c.DoGet("/channels/"+id+"/extra_info", "", ""); err != nil {
+func (c *Client) GetChannelExtraInfo(id string, etag string) (*Result, *AppError) {
+	if r, err := c.DoGet("/channels/"+id+"/extra_info", "", etag); err != nil {
 		return nil, err
 	} else {
 		return &Result{r.Header.Get(HEADER_REQUEST_ID),
diff --git a/store/sql_channel_store.go b/store/sql_channel_store.go
index ce1a8c6fa..cac5c681b 100644
--- a/store/sql_channel_store.go
+++ b/store/sql_channel_store.go
@@ -36,6 +36,7 @@ func NewSqlChannelStore(sqlStore *SqlStore) ChannelStore {
 }
 
 func (s SqlChannelStore) UpgradeSchemaIfNeeded() {
+	s.CreateColumnIfNotExists("Channels", "ExtraUpdateAt", "TotalMsgCount", "bigint(20)", "0")
 }
 
 func (s SqlChannelStore) CreateIndexesIfNotExists() {
@@ -142,6 +143,25 @@ func (s SqlChannelStore) Update(channel *model.Channel) StoreChannel {
 	return storeChannel
 }
 
+func (s SqlChannelStore) extraUpdated(channel *model.Channel) StoreChannel {
+	storeChannel := make(StoreChannel)
+
+	go func() {
+		result := StoreResult{}
+
+		channel.ExtraUpdated()
+
+		if count, err := s.GetMaster().Update(channel); err != nil || count != 1 {
+			result.Err = model.NewAppError("SqlChannelStore.extraUpdated", "Problem updating members last updated time", "id="+channel.Id+", "+err.Error())
+		}
+
+		storeChannel <- result
+		close(storeChannel)
+	}()
+
+	return storeChannel
+}
+
 func (s SqlChannelStore) Get(id string) StoreChannel {
 	storeChannel := make(StoreChannel)
 
@@ -288,20 +308,31 @@ func (s SqlChannelStore) SaveMember(member *model.ChannelMember) StoreChannel {
 	go func() {
 		result := StoreResult{}
 
-		member.PreSave()
-		if result.Err = member.IsValid(); result.Err != nil {
-			storeChannel <- result
-			return
-		}
+		// Grab the channel we are saving this member to
+		if cr := <-s.Get(member.ChannelId); cr.Err != nil {
+			result.Err = cr.Err
+		} else {
+			channel := cr.Data.(*model.Channel)
+
+			member.PreSave()
+			if result.Err = member.IsValid(); result.Err != nil {
+				storeChannel <- result
+				return
+			}
 
-		if err := s.GetMaster().Insert(member); err != nil {
-			if IsUniqueConstraintError(err.Error(), "ChannelId", "channelmembers_pkey") {
-				result.Err = model.NewAppError("SqlChannelStore.SaveMember", "A channel member with that id already exists", "channel_id="+member.ChannelId+", user_id="+member.UserId+", "+err.Error())
+			if err := s.GetMaster().Insert(member); err != nil {
+				if IsUniqueConstraintError(err.Error(), "ChannelId", "channelmembers_pkey") {
+					result.Err = model.NewAppError("SqlChannelStore.SaveMember", "A channel member with that id already exists", "channel_id="+member.ChannelId+", user_id="+member.UserId+", "+err.Error())
+				} else {
+					result.Err = model.NewAppError("SqlChannelStore.SaveMember", "We couldn't save the channel member", "channel_id="+member.ChannelId+", user_id="+member.UserId+", "+err.Error())
+				}
 			} else {
-				result.Err = model.NewAppError("SqlChannelStore.SaveMember", "We couldn't save the channel member", "channel_id="+member.ChannelId+", user_id="+member.UserId+", "+err.Error())
+				result.Data = member
+				// If sucessfull record members have changed in channel
+				if mu := <-s.extraUpdated(channel); mu.Err != nil {
+					result.Err = mu.Err
+				}
 			}
-		} else {
-			result.Data = member
 		}
 
 		storeChannel <- result
@@ -383,9 +414,21 @@ func (s SqlChannelStore) RemoveMember(channelId string, userId string) StoreChan
 	go func() {
 		result := StoreResult{}
 
-		_, err := s.GetMaster().Exec("DELETE FROM ChannelMembers WHERE ChannelId = :ChannelId AND UserId = :UserId", map[string]interface{}{"ChannelId": channelId, "UserId": userId})
-		if err != nil {
-			result.Err = model.NewAppError("SqlChannelStore.RemoveMember", "We couldn't remove the channel member", "channel_id="+channelId+", user_id="+userId+", "+err.Error())
+		// Grab the channel we are saving this member to
+		if cr := <-s.Get(channelId); cr.Err != nil {
+			result.Err = cr.Err
+		} else {
+			channel := cr.Data.(*model.Channel)
+
+			_, err := s.GetMaster().Exec("DELETE FROM ChannelMembers WHERE ChannelId = :ChannelId AND UserId = :UserId", map[string]interface{}{"ChannelId": channelId, "UserId": userId})
+			if err != nil {
+				result.Err = model.NewAppError("SqlChannelStore.RemoveMember", "We couldn't remove the channel member", "channel_id="+channelId+", user_id="+userId+", "+err.Error())
+			} else {
+				// If sucessfull record members have changed in channel
+				if mu := <-s.extraUpdated(channel); mu.Err != nil {
+					result.Err = mu.Err
+				}
+			}
 		}
 
 		storeChannel <- result
diff --git a/store/sql_channel_store_test.go b/store/sql_channel_store_test.go
index ae29bc2b3..b14883843 100644
--- a/store/sql_channel_store_test.go
+++ b/store/sql_channel_store_test.go
@@ -197,6 +197,16 @@ func TestChannelStoreGetByName(t *testing.T) {
 func TestChannelMemberStore(t *testing.T) {
 	Setup()
 
+	c1 := model.Channel{}
+	c1.TeamId = model.NewId()
+	c1.DisplayName = "NameName"
+	c1.Name = "a" + model.NewId() + "b"
+	c1.Type = model.CHANNEL_OPEN
+	c1 = *Must(store.Channel().Save(&c1)).(*model.Channel)
+
+	c1t1 := (<-store.Channel().Get(c1.Id)).Data.(*model.Channel)
+	t1 := c1t1.ExtraUpdateAt
+
 	u1 := model.User{}
 	u1.TeamId = model.NewId()
 	u1.Email = model.NewId()
@@ -210,17 +220,24 @@ func TestChannelMemberStore(t *testing.T) {
 	Must(store.User().Save(&u2))
 
 	o1 := model.ChannelMember{}
-	o1.ChannelId = model.NewId()
+	o1.ChannelId = c1.Id
 	o1.UserId = u1.Id
 	o1.NotifyLevel = model.CHANNEL_NOTIFY_ALL
 	Must(store.Channel().SaveMember(&o1))
 
 	o2 := model.ChannelMember{}
-	o2.ChannelId = o1.ChannelId
+	o2.ChannelId = c1.Id
 	o2.UserId = u2.Id
 	o2.NotifyLevel = model.CHANNEL_NOTIFY_ALL
 	Must(store.Channel().SaveMember(&o2))
 
+	c1t2 := (<-store.Channel().Get(c1.Id)).Data.(*model.Channel)
+	t2 := c1t2.ExtraUpdateAt
+
+	if t2 <= t1 {
+		t.Fatal("Member update time incorrect")
+	}
+
 	members := (<-store.Channel().GetMembers(o1.ChannelId)).Data.([]model.ChannelMember)
 	if len(members) != 2 {
 		t.Fatal("should have saved 2 members")
@@ -233,6 +250,13 @@ func TestChannelMemberStore(t *testing.T) {
 		t.Fatal("should have removed 1 member")
 	}
 
+	c1t3 := (<-store.Channel().Get(c1.Id)).Data.(*model.Channel)
+	t3 := c1t3.ExtraUpdateAt
+
+	if t3 <= t2 || t3 <= t1 {
+		t.Fatal("Member update time incorrect on delete")
+	}
+
 	member := (<-store.Channel().GetMember(o1.ChannelId, o1.UserId)).Data.(model.ChannelMember)
 	if member.ChannelId != o1.ChannelId {
 		t.Fatal("should have go member")
@@ -246,6 +270,12 @@ func TestChannelMemberStore(t *testing.T) {
 	if err := (<-store.Channel().SaveMember(&o1)).Err; err == nil {
 		t.Fatal("Should have been a duplicate")
 	}
+
+	c1t4 := (<-store.Channel().Get(c1.Id)).Data.(*model.Channel)
+	t4 := c1t4.ExtraUpdateAt
+	if t4 != t3 {
+		t.Fatal("Should not update time upon failure")
+	}
 }
 
 func TestChannelStorePermissionsTo(t *testing.T) {