From dfc4dc8078ebc2d7d4a87c8c617cc0591ed78aa5 Mon Sep 17 00:00:00 2001 From: Jonathan Rainville Date: Mon, 4 Nov 2024 16:08:05 -0500 Subject: [PATCH] feat(community)_: add version to image url to let clients update Fixes https://github.com/status-im/status-desktop/issues/16688 Since we use the local image server to show the community image, the URL never changes when we update the image, since it's served using a query string containing the community ID. eg: `https://Localhost:46739/communityDescriptionImages?communityID=0x03c5ece7da362d31199fb02d632f85fdf853af57d89c3204b4d1e90c6ec13bb23c&name=thumbnail` Because of that, the clients cannot know if the image was updated, so they had to force update the image every time, which was inefficient. We discovered this issue when I refactored the community client code in Desktop so that we only update the changed properties of a community instead of reseting the whole thing. The solution I came up with in the PR is to add a `version` to the URL when we detect that the image changed. This let,s the clients detect when the image was updated without having to do any extra logic. --- protocol/communities/community.go | 20 ++++++++++-- protocol/communities/community_test.go | 43 ++++++++++++++++++++++++++ protocol/communities/manager.go | 5 +++ protocol/communities/manager_test.go | 15 ++++++--- protocol/protobuf/chat_identity.proto | 4 +++ server/server_media.go | 3 +- server/server_media_interface.go | 2 +- 7 files changed, 84 insertions(+), 8 deletions(-) diff --git a/protocol/communities/community.go b/protocol/communities/community.go index 4e6a865f7..9bc47ff01 100644 --- a/protocol/communities/community.go +++ b/protocol/communities/community.go @@ -409,11 +409,11 @@ func (o *Community) MarshalJSON() ([]byte, error) { communityItem.Description = o.config.CommunityDescription.Identity.Description if !utils.IsNil(o.mediaServer) { - for t := range o.config.CommunityDescription.Identity.Images { + for t, image := range o.config.CommunityDescription.Identity.Images { if communityItem.Images == nil { communityItem.Images = make(map[string]Image) } - communityItem.Images[t] = Image{Uri: o.mediaServer.MakeCommunityImageURL(o.IDString(), t)} + communityItem.Images[t] = Image{Uri: o.mediaServer.MakeCommunityImageURL(o.IDString(), t, image.Version)} } } } @@ -1139,6 +1139,7 @@ func (o *Community) Edit(description *protobuf.CommunityDescription) { o.config.CommunityDescription.Identity.Color = description.Identity.Color o.config.CommunityDescription.Tags = description.Tags o.config.CommunityDescription.Identity.Emoji = description.Identity.Emoji + o.UpdateImageVersions(description) o.config.CommunityDescription.Identity.Images = description.Identity.Images o.config.CommunityDescription.IntroMessage = description.IntroMessage o.config.CommunityDescription.OutroMessage = description.OutroMessage @@ -1211,6 +1212,19 @@ func (o *Community) MemberIdentity() *ecdsa.PublicKey { return &o.config.MemberIdentity.PublicKey } +func (o *Community) UpdateImageVersions(description *protobuf.CommunityDescription) { + // If the image was updated, up the version of the image so that the localURL updates + for imageType, newImage := range description.Identity.Images { + oldImage, ok := o.config.CommunityDescription.Identity.Images[imageType] + if ok { + // FIXME this does not work!!!!! + if !bytes.Equal(oldImage.Payload, newImage.Payload) { + newImage.Version = oldImage.Version + 1 + } + } + } +} + // UpdateCommunityDescription will update the community to the new community description and return a list of changes func (o *Community) UpdateCommunityDescription(description *protobuf.CommunityDescription, rawMessage []byte, newControlNode *ecdsa.PublicKey) (*CommunityChanges, error) { o.mutex.Lock() @@ -1231,6 +1245,8 @@ func (o *Community) UpdateCommunityDescription(description *protobuf.CommunityDe originCommunity := o.CreateDeepCopy() + o.UpdateImageVersions(description) + o.config.CommunityDescription = description o.config.CommunityDescriptionProtocolMessage = rawMessage diff --git a/protocol/communities/community_test.go b/protocol/communities/community_test.go index 5037e67e1..c27b8e036 100644 --- a/protocol/communities/community_test.go +++ b/protocol/communities/community_test.go @@ -11,6 +11,7 @@ import ( "github.com/stretchr/testify/suite" "github.com/status-im/status-go/eth-node/crypto" + "github.com/status-im/status-go/images" "github.com/status-im/status-go/protocol/common" "github.com/status-im/status-go/protocol/protobuf" ) @@ -657,6 +658,21 @@ func (s *CommunitySuite) TestHandleCommunityDescription() { } } +func (s *CommunitySuite) TestHandleCommunityDescriptionWithImageChange() { + key, err := crypto.GenerateKey() + s.Require().NoError(err) + + signer := &key.PublicKey + + org := s.buildCommunity(signer) + org.Join() + changes, err := org.UpdateCommunityDescription(s.changedImageCommunityDescription(org), []byte{0x01}, nil) + s.Require().NoError(err) + thumbnail, ok := changes.Community.Identity().Images["thumbnail"] + s.Require().True(ok) + s.Require().Equal(uint32(1), thumbnail.Version) +} + func (s *CommunitySuite) TestValidateCommunityDescription() { testCases := []struct { @@ -889,6 +905,17 @@ func (s *CommunitySuite) buildCommunityDescription() *protobuf.CommunityDescript config := s.configOnRequestOrgOnRequestChat() desc := config.CommunityDescription desc.Clock = 1 + desc.Identity = &protobuf.ChatIdentity{} + desc.Identity.Images = make(map[string]*protobuf.IdentityImage) + imgs, err := images.GenerateIdentityImages("../../_assets/tests/status.png", 0, 0, 0, 0) + s.Require().NoError(err) + for _, image := range imgs { + desc.Identity.Images[image.Name] = &protobuf.IdentityImage{ + Payload: []byte(""), + SourceType: protobuf.IdentityImage_RAW_PAYLOAD, + ImageFormat: images.GetProtobufImageFormat(image.Payload), + } + } desc.Members = make(map[string]*protobuf.CommunityMember) desc.Members[s.member1Key] = &protobuf.CommunityMember{} desc.Members[s.member2Key] = &protobuf.CommunityMember{} @@ -993,6 +1020,22 @@ func (s *CommunitySuite) removedChatCommunityDescription(org *Community) *protob return description } +func (s *CommunitySuite) changedImageCommunityDescription(org *Community) *protobuf.CommunityDescription { + description := proto.Clone(org.config.CommunityDescription).(*protobuf.CommunityDescription) + description.Clock++ + imgs, err := images.GenerateIdentityImages("../../_assets/tests/elephant.jpg", 0, 0, 5, 5) + s.Require().NoError(err) + for _, image := range imgs { + description.Identity.Images[image.Name] = &protobuf.IdentityImage{ + Payload: image.Payload, + SourceType: protobuf.IdentityImage_RAW_PAYLOAD, + ImageFormat: images.GetProtobufImageFormat(image.Payload), + } + } + + return description +} + func (s *CommunitySuite) TestMarshalJSON() { community := s.buildCommunity(&s.identity.PublicKey) channelID := community.ChatID(testChatID1) diff --git a/protocol/communities/manager.go b/protocol/communities/manager.go index be5a5b659..b42ea7206 100644 --- a/protocol/communities/manager.go +++ b/protocol/communities/manager.go @@ -1650,6 +1650,11 @@ func (m *Manager) EditCommunity(request *requests.EditCommunity) (*Community, er if community.IsControlNode() { community.increaseClock() + // Update the community cache. This makes sure that the image server has access to the new image if needed + _, _, err = m.preprocessDescription(request.CommunityID, community.config.CommunityDescription) + if err != nil { + return nil, err + } } else { err := community.addNewCommunityEvent(community.ToCommunityEditCommunityEvent(newDescription)) if err != nil { diff --git a/protocol/communities/manager_test.go b/protocol/communities/manager_test.go index 6a9ccd108..82bf43664 100644 --- a/protocol/communities/manager_test.go +++ b/protocol/communities/manager_test.go @@ -410,6 +410,7 @@ func (s *ManagerSuite) TestEditCommunity() { Name: "status", Description: "status community description", Membership: protobuf.CommunityPermissions_AUTO_ACCEPT, + Image: "../../_assets/tests/elephant.jpg", } community, err := s.manager.CreateCommunity(createRequest, true) @@ -421,12 +422,17 @@ func (s *ManagerSuite) TestEditCommunity() { CreateCommunity: requests.CreateCommunity{ Name: "statusEdited", Description: "status community description edited", + Image: "../../_assets/tests/status.png", + ImageBx: 5, + ImageBy: 5, }, } updatedCommunity, err := s.manager.EditCommunity(update) s.Require().NoError(err) s.Require().NotNil(updatedCommunity) + // Make sure the version of the image got updated with the new image + s.Require().Equal(uint32(1), updatedCommunity.Identity().Images["thumbnail"].Version) //ensure updated community successfully stored communities, err := s.manager.All() @@ -438,10 +444,11 @@ func (s *ManagerSuite) TestEditCommunity() { storedCommunity = communities[0] } - s.Require().Equal(storedCommunity.ID(), updatedCommunity.ID()) - s.Require().Equal(storedCommunity.PrivateKey(), updatedCommunity.PrivateKey()) - s.Require().Equal(storedCommunity.config.CommunityDescription.Identity.DisplayName, update.CreateCommunity.Name) - s.Require().Equal(storedCommunity.config.CommunityDescription.Identity.Description, update.CreateCommunity.Description) + s.Require().Equal(updatedCommunity.ID(), storedCommunity.ID()) + s.Require().Equal(updatedCommunity.PrivateKey(), storedCommunity.PrivateKey()) + s.Require().Equal(update.CreateCommunity.Name, storedCommunity.config.CommunityDescription.Identity.DisplayName) + s.Require().Equal(update.CreateCommunity.Description, storedCommunity.config.CommunityDescription.Identity.Description) + s.Require().Equal(uint32(1), storedCommunity.config.CommunityDescription.Identity.Images["thumbnail"].Version) } func (s *ManagerSuite) TestGetControlledCommunitiesChatIDs() { diff --git a/protocol/protobuf/chat_identity.proto b/protocol/protobuf/chat_identity.proto index ae2de8a0b..aee250017 100644 --- a/protocol/protobuf/chat_identity.proto +++ b/protocol/protobuf/chat_identity.proto @@ -58,6 +58,10 @@ message IdentityImage { // encrypted signals the encryption state of the payload, default is false. bool encrypted = 5; + // version iterator of the image since images use a localUrl to be shown, + // this iterator makes sure the image URL changes when a new image is given + uint32 version = 6; + // SourceType are the predefined types of image source allowed enum SourceType { UNKNOWN_SOURCE_TYPE = 0; diff --git a/server/server_media.go b/server/server_media.go index 793318acf..6d60d9b79 100644 --- a/server/server_media.go +++ b/server/server_media.go @@ -211,12 +211,13 @@ func (s *MediaServer) MakeCommunityTokenImagesURL(communityID string, chainID ui return u.String() } -func (s *MediaServer) MakeCommunityImageURL(communityID, name string) string { +func (s *MediaServer) MakeCommunityImageURL(communityID, name string, version uint32) string { u := s.MakeBaseURL() u.Path = communityDescriptionImagesPath u.RawQuery = url.Values{ "communityID": {communityID}, "name": {name}, + "version": {strconv.FormatUint(uint64(version), 10)}, }.Encode() return u.String() diff --git a/server/server_media_interface.go b/server/server_media_interface.go index a62d5a165..9ad07256b 100644 --- a/server/server_media_interface.go +++ b/server/server_media_interface.go @@ -2,5 +2,5 @@ package server type MediaServerInterface interface { MakeCommunityDescriptionTokenImageURL(communityID, symbol string) string - MakeCommunityImageURL(communityID, name string) string + MakeCommunityImageURL(communityID, name string, version uint32) string }