feat(community)_: add version to image url to let clients update (#6118)

Fixes status-im/status-desktop#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_changes.go

@@ -1,6 +1,7 @@
 package communities
 
 import (
+	"bytes"
 	"crypto/ecdsa"
 
 	slices "golang.org/x/exp/slices"
@@ -53,6 +54,9 @@ type CommunityChanges struct {
 	// No kick AC notification will be generated and member will join automatically
 	// as soon as he provides missing data
 	MemberSoftKicked bool `json:"memberSoftRemoved"`
+
+	// CommunityImageModified indicates whether the community image was modified by an admin or owner
+	ImageModified bool `json:"communityImageModified"`
 }
 
 func EmptyCommunityChanges() *CommunityChanges {
@@ -159,6 +163,18 @@ func (c *CommunityChanges) IsMemberUnbanned(identity string) bool {
 	return ok
 }
 
+func CommunityImagesChanged(originCommunityImages, modifiedCommunityImages map[string]*protobuf.IdentityImage) bool {
+	for imageType, newImage := range modifiedCommunityImages {
+		oldImage, ok := originCommunityImages[imageType]
+		if ok {
+			if !bytes.Equal(oldImage.Payload, newImage.Payload) {
+				return true
+			}
+		}
+	}
+	return false
+}
+
 func EvaluateCommunityChanges(origin, modified *Community) *CommunityChanges {
 	changes := evaluateCommunityChangesByDescription(origin.Description(), modified.Description())
 
@@ -187,6 +203,8 @@ func EvaluateCommunityChanges(origin, modified *Community) *CommunityChanges {
 		}
 	}
 
+	changes.ImageModified = CommunityImagesChanged(modified.config.CommunityDescription.Identity.Images, origin.config.CommunityDescription.Identity.Images)
+
 	changes.Community = modified
 	return changes
 }

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,19 @@ 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)
+	s.Require().True(changes.ImageModified)
+}
+
 func (s *CommunitySuite) TestValidateCommunityDescription() {
 
 	testCases := []struct {
@@ -889,6 +903,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 +1018,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)

protocol/communities/manager.go

@@ -110,6 +110,7 @@ type Manager struct {
 	keyDistributor           KeyDistributor
 	communityLock            *CommunityLock
 	mediaServer              server.MediaServerInterface
+	communityImageVersions   map[string]uint32
 }
 
 type CommunityLock struct {
@@ -418,17 +419,18 @@ func NewManager(
 	}
 
 	manager := &Manager{
-		logger:         logger,
-		encryptor:      encryptor,
-		identity:       identity,
-		installationID: installationID,
-		ownerVerifier:  ownerVerifier,
-		quit:           make(chan struct{}),
-		transport:      transport,
-		timesource:     timesource,
-		keyDistributor: keyDistributor,
-		communityLock:  NewCommunityLock(logger),
-		mediaServer:    mediaServer,
+		logger:                 logger,
+		encryptor:              encryptor,
+		identity:               identity,
+		installationID:         installationID,
+		ownerVerifier:          ownerVerifier,
+		quit:                   make(chan struct{}),
+		transport:              transport,
+		timesource:             timesource,
+		keyDistributor:         keyDistributor,
+		communityLock:          NewCommunityLock(logger),
+		mediaServer:            mediaServer,
+		communityImageVersions: make(map[string]uint32),
 	}
 
 	manager.persistence = &Persistence{
@@ -486,6 +488,9 @@ func NewManager(
 }
 
 func (m *Manager) SetMediaServerProperties() {
+	m.mediaServer.SetCommunityImageVersionReader(func(communityID string) uint32 {
+		return m.communityImageVersions[communityID]
+	})
 	m.mediaServer.SetCommunityImageReader(func(communityID string) (map[string]*protobuf.IdentityImage, error) {
 		community, err := m.GetByIDString(communityID)
 		if err != nil {
@@ -1621,6 +1626,15 @@ func (m *Manager) UpdatePubsubTopicPrivateKey(topic string, privKey *ecdsa.Priva
 	return m.transport.RemovePubsubTopicKey(topic)
 }
 
+// Managing the version of community images is necessary because image URLs are "constant"
+// For eg: https://Localhost:46739/communityDescriptionImages?communityID=[ID]&name=thumbnail
+// So the clients have no way of knowing that they need to reload the image
+// Having a version number makes it so that the URL changes on image updates
+// eg: https://Localhost:46739/communityDescriptionImages?communityID=[ID]&name=thumbnail&version=1
+func (m *Manager) incrementCommunityImageVersion(communityID string) {
+	m.communityImageVersions[communityID] = m.communityImageVersions[communityID] + 1
+}
+
 // EditCommunity takes a description, updates the community with the description,
 // saves it and returns it
 func (m *Manager) EditCommunity(request *requests.EditCommunity) (*Community, error) {
@@ -1665,12 +1679,18 @@ func (m *Manager) EditCommunity(request *requests.EditCommunity) (*Community, er
 		return nil, ErrNotAuthorized
 	}
 
+	imageModified := CommunityImagesChanged(newDescription.Identity.Images, community.Images())
+
 	// Edit the community values
 	community.Edit(newDescription)
 	if err != nil {
 		return nil, err
 	}
 
+	if imageModified {
+		m.incrementCommunityImageVersion(community.IDString())
+	}
+
 	if community.IsControlNode() {
 		community.increaseClock()
 	} else {
@@ -2295,6 +2315,10 @@ func (m *Manager) handleCommunityDescriptionMessageCommon(community *Community,
 		return nil, err
 	}
 
+	if changes.ImageModified {
+		m.incrementCommunityImageVersion(community.IDString())
+	}
+
 	if err = m.handleCommunityTokensMetadata(community); err != nil {
 		return nil, err
 	}

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,19 @@ 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
+	communityImageVersion, ok := s.manager.communityImageVersions[community.IDString()]
+	s.Require().True(ok)
+	s.Require().Equal(uint32(1), communityImageVersion)
 
 	//ensure updated community successfully stored
 	communities, err := s.manager.All()
@@ -438,10 +446,10 @@ 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)
 }
 
 func (s *ManagerSuite) TestGetControlledCommunitiesChatIDs() {

server/server_media.go

@@ -4,6 +4,7 @@ import (
 	"crypto/tls"
 	"database/sql"
 	"errors"
+	"fmt"
 	"net/url"
 	"strconv"
 
@@ -27,12 +28,13 @@ func WithMediaServerDisableTLS(disableTLS bool) MediaServerOption {
 type MediaServer struct {
 	Server
 
-	db                    *sql.DB
-	downloader            *ipfs.Downloader
-	multiaccountsDB       *multiaccounts.Database
-	walletDB              *sql.DB
-	communityImagesReader func(communityID string) (map[string]*protobuf.IdentityImage, error)
-	communityTokenReader  func(communityID string) ([]*protobuf.CommunityTokenMetadata, error)
+	db                          *sql.DB
+	downloader                  *ipfs.Downloader
+	multiaccountsDB             *multiaccounts.Database
+	walletDB                    *sql.DB
+	communityImagesReader       func(communityID string) (map[string]*protobuf.IdentityImage, error)
+	communityTokenReader        func(communityID string) ([]*protobuf.CommunityTokenMetadata, error)
+	communityImageVersionReader func(communityID string) uint32
 
 	// disableTLS controls whether the media server uses HTTP instead of HTTPS.
 	// Set to true to avoid TLS certificate issues with react-native-fast-image
@@ -110,6 +112,17 @@ func (s *MediaServer) MakeBaseURL() *url.URL {
 	}
 }
 
+func (s *MediaServer) SetCommunityImageVersionReader(getFunc func(communityID string) uint32) {
+	s.communityImageVersionReader = getFunc
+}
+
+func (s *MediaServer) getCommunityImageVersion(communityID string) uint32 {
+	if s.communityImageVersionReader == nil {
+		return 0
+	}
+	return s.communityImageVersionReader(communityID)
+}
+
 func (s *MediaServer) SetCommunityImageReader(getFunc func(communityID string) (map[string]*protobuf.IdentityImage, error)) {
 	s.communityImagesReader = getFunc
 }
@@ -243,6 +256,7 @@ func (s *MediaServer) MakeCommunityImageURL(communityID, name string) string {
 	u.RawQuery = url.Values{
 		"communityID": {communityID},
 		"name":        {name},
+		"version":     {fmt.Sprintf("%d", (s.getCommunityImageVersion(communityID)))},
 	}.Encode()
 
 	return u.String()

server/server_media_interface.go

@@ -5,6 +5,7 @@ import "github.com/status-im/status-go/protocol/protobuf"
 type MediaServerInterface interface {
 	MakeCommunityDescriptionTokenImageURL(communityID, symbol string) string
 	MakeCommunityImageURL(communityID, name string) string
+	SetCommunityImageVersionReader(func(communityID string) uint32)
 	SetCommunityImageReader(func(communityID string) (map[string]*protobuf.IdentityImage, error))
 	SetCommunityTokensReader(func(communityID string) ([]*protobuf.CommunityTokenMetadata, error))
 }