feat(community)_: add version to image url to let clients update (#6118)
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.
This commit is contained in:
parent
92ba63b282
commit
0794edc3db
|
@ -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
|
||||
}
|
||||
|
|
|
@ -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)
|
||||
|
|
|
@ -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
|
||||
}
|
||||
|
|
|
@ -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() {
|
||||
|
|
|
@ -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()
|
||||
|
|
|
@ -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))
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue