Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: text length not computed properly #1303

Merged
merged 3 commits into from
Feb 13, 2024
Merged

fix: text length not computed properly #1303

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b085e04
fix: text length not computed properly
RiccardoM Feb 13, 2024
173288b
chore: added changeset entry
RiccardoM Feb 13, 2024
036ca6b
chore: add comment to explain why counting graphemes
RiccardoM Feb 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ require (
github.com/osmosis-labs/go-mutesting v0.0.0-20221219192234-827e6d6b9d4e
github.com/prometheus/client_golang v1.18.0
github.com/rakyll/statik v0.1.7
github.com/rivo/uniseg v0.4.7
github.com/spf13/cast v1.6.0
github.com/spf13/cobra v1.8.0
github.com/spf13/pflag v1.0.5
Expand Down
2 changes: 2 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1288,6 +1288,8 @@ github.com/rcrowley/go-metrics v0.0.0-20181016184325-3113b8401b8a/go.mod h1:bCqn
github.com/rcrowley/go-metrics v0.0.0-20201227073835-cf1acfcdf475 h1:N/ElC8H3+5XpJzTSTfLsJV/mx9Q9g7kxmchpfZyxgzM=
github.com/rcrowley/go-metrics v0.0.0-20201227073835-cf1acfcdf475/go.mod h1:bCqnVzQkZxMG4s8nGwiZ5l3QUCyqpo9Y+/ZMZ9VjZe4=
github.com/retailnext/hllpp v1.0.1-0.20180308014038-101a6d2f8b52/go.mod h1:RDpi1RftBQPUCDRw6SmxeaREsAaRKnOclghuzp/WRzc=
github.com/rivo/uniseg v0.4.7 h1:WUdvkW8uEhrYfLC4ZzdpI2ztxP1I582+49Oc5Mq64VQ=
github.com/rivo/uniseg v0.4.7/go.mod h1:FN3SvrM+Zdj16jyLfmOkMNblXMcoc8DfTHruCPUcx88=
github.com/rjeczalik/notify v0.9.1/go.mod h1:rKwnCoCGeuQnwBtTSPL9Dad03Vh2n40ePRrjvIXnJho=
github.com/rogpeppe/fastuuid v0.0.0-20150106093220-6724a57986af/go.mod h1:XWv6SoW27p1b0cqNHllgS5HIMJraePCO15w5zCzIWYg=
github.com/rogpeppe/fastuuid v1.2.0/go.mod h1:jVj6XXZzXRy/MSR5jhDC/2q6DgLz+nrA6LYCDYWNEvQ=
Expand Down
2 changes: 1 addition & 1 deletion x/posts/keeper/posts.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ func (k Keeper) ValidatePost(ctx sdk.Context, post types.Post) error {
}

// Check the post text length to make sure it's not exceeding the max length
if uint32(len(post.Text)) > params.MaxTextLength {
if uint32(post.GetTextLength()) > params.MaxTextLength {
RiccardoM marked this conversation as resolved.
Show resolved Hide resolved
return errors.Wrapf(types.ErrInvalidPost, "text exceed max length allowed")
}

Expand Down
6 changes: 6 additions & 0 deletions x/posts/types/models.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/gogo/protobuf/proto"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/rivo/uniseg"
)

// ParsePostID parses the given value as a post id, returning an error if it's invalid
Expand Down Expand Up @@ -155,6 +156,11 @@ func (p Post) GetMentionedUsers() []string {
return mentions
}

// GetTextLength returns the length of the post text
func (p Post) GetTextLength() int {
return uniseg.GraphemeClusterCount(p.Text)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure the uniseg.GraphemeClusterCount function handles all edge cases correctly, especially with complex grapheme clusters. Consider adding a comment to explain why grapheme counting is preferred over byte or rune counting for future maintainability.


// NewPostReference returns a new PostReference instance
func NewPostReference(referenceType PostReferenceType, postID uint64, position uint64) PostReference {
return PostReference{
Expand Down
97 changes: 97 additions & 0 deletions x/posts/types/models_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,103 @@ func TestPost_Validate(t *testing.T) {
}
}

func TestPost_GetTextLength(t *testing.T) {
testCases := []struct {
name string
post types.Post
expLength int
}{
{
name: "latin text returns correct length",
post: types.NewPost(
1,
0,
2,
"External id",
"Hello",
"cosmos1eqpa6mv2jgevukaqtjmx5535vhc3mm3cf458zg",
1,
nil,
nil,
nil,
types.REPLY_SETTING_EVERYONE,
time.Date(2020, 1, 1, 12, 00, 00, 000, time.UTC),
nil,
"cosmos13t6y2nnugtshwuy0zkrq287a95lyy8vzleaxmd",
),
expLength: 5,
},
{
name: "non latin text returns correct length",
post: types.NewPost(
1,
0,
2,
"External id",
"δΈ–η•Œ",
"cosmos1eqpa6mv2jgevukaqtjmx5535vhc3mm3cf458zg",
1,
nil,
nil,
nil,
types.REPLY_SETTING_EVERYONE,
time.Date(2020, 1, 1, 12, 00, 00, 000, time.UTC),
nil,
"cosmos13t6y2nnugtshwuy0zkrq287a95lyy8vzleaxmd",
),
expLength: 2,
},
{
name: "emoji text returns correct length",
post: types.NewPost(
1,
0,
2,
"External id",
"πŸ˜ƒπŸ˜ƒπŸ˜ƒ",
"cosmos1eqpa6mv2jgevukaqtjmx5535vhc3mm3cf458zg",
1,
nil,
nil,
nil,
types.REPLY_SETTING_EVERYONE,
time.Date(2020, 1, 1, 12, 00, 00, 000, time.UTC),
nil,
"cosmos13t6y2nnugtshwuy0zkrq287a95lyy8vzleaxmd",
),
expLength: 3,
},
{
name: "mixed text returns correct length",
post: types.NewPost(
1,
0,
2,
"External id",
"Hello δΈ–η•Œ! πŸ˜ƒ",
"cosmos1eqpa6mv2jgevukaqtjmx5535vhc3mm3cf458zg",
1,
nil,
nil,
nil,
types.REPLY_SETTING_EVERYONE,
time.Date(2020, 1, 1, 12, 00, 00, 000, time.UTC),
nil,
"cosmos13t6y2nnugtshwuy0zkrq287a95lyy8vzleaxmd",
),
expLength: 11,
},
}

for _, tc := range testCases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
require.Equal(t, tc.expLength, tc.post.GetTextLength())
})
}
}

func TestPostReference_Validate(t *testing.T) {
testCases := []struct {
name string
Expand Down
2 changes: 1 addition & 1 deletion x/reactions/keeper/reactions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ func (k Keeper) validateFreeTextValue(ctx sdk.Context, reaction types.Reaction,
}

// Make sure the value respected the max length
if uint32(len(value.Text)) > params.MaxLength {
if uint32(value.GetLength()) > params.MaxLength {
RiccardoM marked this conversation as resolved.
Show resolved Hide resolved
return errors.Wrapf(sdkerrors.ErrInvalidRequest, "value exceed max length allowed")
}

Expand Down
8 changes: 8 additions & 0 deletions x/reactions/types/models.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (
"strconv"
"strings"

"github.com/rivo/uniseg"

codectypes "github.com/cosmos/cosmos-sdk/codec/types"
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/gogo/protobuf/proto"
Expand Down Expand Up @@ -121,6 +123,12 @@ func NewFreeTextValue(text string) *FreeTextValue {
// isReactionValue implements ReactionValue
func (v *FreeTextValue) isReactionValue() {}

// GetLength returns the length of the reaction value
func (v *FreeTextValue) GetLength() int {
return uniseg.GraphemeClusterCount(v.Text)

}

// Validate implements ReactionValue
func (v *FreeTextValue) Validate() error {
if strings.TrimSpace(v.Text) == "" {
Expand Down
38 changes: 38 additions & 0 deletions x/reactions/types/models_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,44 @@ func TestRegisteredReactionValue_Validate(t *testing.T) {

// --------------------------------------------------------------------------------------------------------------------

func TestFreeTextValue_Length(t *testing.T) {

testCases := []struct {
name string
reaction *types.FreeTextValue
expLength int
}{
{
name: "latin text returns correct length",
reaction: types.NewFreeTextValue("Hello"),
expLength: 5,
},
{
name: "non latin text returns correct length",
reaction: types.NewFreeTextValue("δΈ–η•Œ"),
expLength: 2,
},
{
name: "emoji text returns correct length",
reaction: types.NewFreeTextValue("πŸ˜ƒπŸ˜ƒπŸ˜ƒ"),
expLength: 3,
},
{
name: "mixed text returns correct length",
reaction: types.NewFreeTextValue("Hello δΈ–η•Œ! πŸ˜ƒ"),
expLength: 11,
},
}

for _, tc := range testCases {
tc := tc
t.Run(tc.name, func(t *testing.T) {
t.Parallel()
require.Equal(t, tc.expLength, tc.reaction.GetLength())
})
}
}

func TestFreeTextValue_Validate(t *testing.T) {
testCases := []struct {
name string
Expand Down
Loading