[MM-62553]+[MM-62554] Property Architecture: cursor based pagination (#30119)

* refactor: Replace pagination with cursor-based pagination for custom profile attributes

* remove pagination loop on property value retrieval for CPA

* add migrations to optimize pagination on property fields and values

* adapt test to remove pagination check

* update migrations list

* postgres: drop index concurrently

* concurrent index manipulation must be done outside of a Tx

* fix: Correct SQL index drop syntax from "OM" to "ON" in migration files

* test: Add CountForGroup test cases for property field store

* refactor: Add CountForGroup method to PropertyFieldStore interface and implementations

* Fix style and i18n

* feat: Add optional deleted property field filtering to CountForGroup method

* refactor: Update CountForGroup to support optional deleted property fields

* test: Add comprehensive tests for CountForGroup with includeDeleted parameter

* adapt test + gen layers

* rename property service method and set the includeDelete to false

* refactor: Remove redundant constant and use CustomProfileAttributesFieldLimit directly

* fix tests

---------

Co-authored-by: Mattermost Build <build@mattermost.com>
Этот коммит содержится в:
Julien Tant
2025-02-13 08:23:50 -07:00
коммит произвёл GitHub
родитель 4615ca5f28
Коммит 632a60b332
25 изменённых файлов: 820 добавлений и 61 удалений

Просмотреть файл

@@ -12,7 +12,9 @@ import (
"github.com/pkg/errors"
)
const CustomProfileAttributesFieldLimit = 20
const (
CustomProfileAttributesFieldLimit = 20
)
var cpaGroupID string
@@ -58,7 +60,6 @@ func (a *App) ListCPAFields() ([]*model.PropertyField, *model.AppError) {
opts := model.PropertyFieldSearchOpts{
GroupID: groupID,
Page: 0,
PerPage: CustomProfileAttributesFieldLimit,
}
@@ -76,12 +77,12 @@ func (a *App) CreateCPAField(field *model.PropertyField) (*model.PropertyField,
return nil, model.NewAppError("CreateCPAField", "app.custom_profile_attributes.cpa_group_id.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
}
existingFields, appErr := a.ListCPAFields()
if appErr != nil {
return nil, appErr
fieldCount, err := a.Srv().propertyService.CountActivePropertyFieldsForGroup(groupID)
if err != nil {
return nil, model.NewAppError("CreateCPAField", "app.custom_profile_attributes.count_property_fields.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
}
if len(existingFields) >= CustomProfileAttributesFieldLimit {
if fieldCount >= CustomProfileAttributesFieldLimit {
return nil, model.NewAppError("CreateCPAField", "app.custom_profile_attributes.limit_reached.app_error", nil, "", http.StatusUnprocessableEntity).Wrap(err)
}
@@ -171,19 +172,16 @@ func (a *App) ListCPAValues(userID string) ([]*model.PropertyValue, *model.AppEr
return nil, model.NewAppError("GetCPAFields", "app.custom_profile_attributes.cpa_group_id.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
}
opts := model.PropertyValueSearchOpts{
GroupID: groupID,
TargetID: userID,
Page: 0,
PerPage: 999999,
IncludeDeleted: false,
}
fields, err := a.Srv().propertyService.SearchPropertyValues(opts)
values, err := a.Srv().propertyService.SearchPropertyValues(model.PropertyValueSearchOpts{
GroupID: groupID,
TargetID: userID,
PerPage: CustomProfileAttributesFieldLimit,
})
if err != nil {
return nil, model.NewAppError("ListCPAValues", "app.custom_profile_attributes.list_property_values.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
}
return fields, nil
return values, nil
}
func (a *App) GetCPAValue(valueID string) (*model.PropertyValue, *model.AppError) {

Просмотреть файл

@@ -517,3 +517,60 @@ func TestPatchCPAValue(t *testing.T) {
require.Equal(t, userID, updatedValue.TargetID)
})
}
func TestListCPAValues(t *testing.T) {
os.Setenv("MM_FEATUREFLAGS_CUSTOMPROFILEATTRIBUTES", "true")
defer os.Unsetenv("MM_FEATUREFLAGS_CUSTOMPROFILEATTRIBUTES")
th := Setup(t).InitBasic()
defer th.TearDown()
cpaGroupID, cErr := th.App.cpaGroupID()
require.NoError(t, cErr)
userID := model.NewId()
t.Run("should return empty list when user has no values", func(t *testing.T) {
values, appErr := th.App.ListCPAValues(userID)
require.Nil(t, appErr)
require.Empty(t, values)
})
t.Run("should list all values for a user", func(t *testing.T) {
var expectedValues []json.RawMessage
for i := 1; i <= CustomProfileAttributesFieldLimit; i++ {
field := &model.PropertyField{
GroupID: cpaGroupID,
Name: fmt.Sprintf("Field %d", i),
Type: model.PropertyFieldTypeText,
}
_, err := th.App.Srv().propertyService.CreatePropertyField(field)
require.NoError(t, err)
value := &model.PropertyValue{
TargetID: userID,
TargetType: "user",
GroupID: cpaGroupID,
FieldID: field.ID,
Value: json.RawMessage(fmt.Sprintf(`"Value %d"`, i)),
}
_, err = th.App.Srv().propertyService.CreatePropertyValue(value)
require.NoError(t, err)
expectedValues = append(expectedValues, value.Value)
}
// List values for original user
values, appErr := th.App.ListCPAValues(userID)
require.Nil(t, appErr)
require.Len(t, values, CustomProfileAttributesFieldLimit)
actualValues := make([]json.RawMessage, len(values))
for i, value := range values {
require.Equal(t, userID, value.TargetID)
require.Equal(t, "user", value.TargetType)
require.Equal(t, cpaGroupID, value.GroupID)
actualValues[i] = value.Value
}
require.ElementsMatch(t, expectedValues, actualValues)
})
}

Просмотреть файл

@@ -19,6 +19,10 @@ func (ps *PropertyService) GetPropertyFields(ids []string) ([]*model.PropertyFie
return ps.fieldStore.GetMany(ids)
}
func (ps *PropertyService) CountActivePropertyFieldsForGroup(groupID string) (int64, error) {
return ps.fieldStore.CountForGroup(groupID, false)
}
func (ps *PropertyService) SearchPropertyFields(opts model.PropertyFieldSearchOpts) ([]*model.PropertyField, error) {
return ps.fieldStore.SearchPropertyFields(opts)
}

Просмотреть файл

@@ -257,6 +257,10 @@ channels/db/migrations/mysql/000129_add_property_system_architecture.down.sql
channels/db/migrations/mysql/000129_add_property_system_architecture.up.sql
channels/db/migrations/mysql/000130_system_console_stats.down.sql
channels/db/migrations/mysql/000130_system_console_stats.up.sql
channels/db/migrations/mysql/000131_create_index_pagination_on_property_values.down.sql
channels/db/migrations/mysql/000131_create_index_pagination_on_property_values.up.sql
channels/db/migrations/mysql/000132_create_index_pagination_on_property_fields.down.sql
channels/db/migrations/mysql/000132_create_index_pagination_on_property_fields.up.sql
channels/db/migrations/postgres/000001_create_teams.down.sql
channels/db/migrations/postgres/000001_create_teams.up.sql
channels/db/migrations/postgres/000002_create_team_members.down.sql
@@ -515,3 +519,7 @@ channels/db/migrations/postgres/000129_add_property_system_architecture.down.sql
channels/db/migrations/postgres/000129_add_property_system_architecture.up.sql
channels/db/migrations/postgres/000130_system_console_stats.down.sql
channels/db/migrations/postgres/000130_system_console_stats.up.sql
channels/db/migrations/postgres/000131_create_index_pagination_on_property_values.down.sql
channels/db/migrations/postgres/000131_create_index_pagination_on_property_values.up.sql
channels/db/migrations/postgres/000132_create_index_pagination_on_property_fields.down.sql
channels/db/migrations/postgres/000132_create_index_pagination_on_property_fields.up.sql

Просмотреть файл

@@ -0,0 +1,14 @@
SET @preparedStatement = (SELECT IF(
(
SELECT COUNT(*) FROM INFORMATION_SCHEMA.STATISTICS
WHERE table_name = 'PropertyValues'
AND table_schema = DATABASE()
AND index_name = 'idx_propertyvalues_create_at_id'
) > 0,
'DROP INDEX idx_propertyvalues_create_at_id ON PropertyValues;',
'SELECT 1'
));
PREPARE removeIndexIfExists FROM @preparedStatement;
EXECUTE removeIndexIfExists;
DEALLOCATE PREPARE removeIndexIfExists;

Просмотреть файл

@@ -0,0 +1,14 @@
SET @preparedStatement = (SELECT IF(
(
SELECT COUNT(*) FROM INFORMATION_SCHEMA.STATISTICS
WHERE table_name = 'PropertyValues'
AND table_schema = DATABASE()
AND index_name = 'idx_propertyvalues_create_at_id'
) > 0,
'SELECT 1',
'CREATE INDEX idx_propertyvalues_create_at_id ON PropertyValues(CreateAt, ID);'
));
PREPARE createIndexIfNotExists FROM @preparedStatement;
EXECUTE createIndexIfNotExists;
DEALLOCATE PREPARE createIndexIfNotExists;

Просмотреть файл

@@ -0,0 +1,14 @@
SET @preparedStatement = (SELECT IF(
(
SELECT COUNT(*) FROM INFORMATION_SCHEMA.STATISTICS
WHERE table_name = 'PropertyFields'
AND table_schema = DATABASE()
AND index_name = 'idx_propertyfields_create_at_id'
) > 0,
'DROP INDEX idx_propertyfields_create_at_id ON PropertyFields;',
'SELECT 1'
));
PREPARE removeIndexIfExists FROM @preparedStatement;
EXECUTE removeIndexIfExists;
DEALLOCATE PREPARE removeIndexIfExists;

Просмотреть файл

@@ -0,0 +1,14 @@
SET @preparedStatement = (SELECT IF(
(
SELECT COUNT(*) FROM INFORMATION_SCHEMA.STATISTICS
WHERE table_name = 'PropertyFields'
AND table_schema = DATABASE()
AND index_name = 'idx_propertyfields_create_at_id'
) > 0,
'SELECT 1',
'CREATE INDEX idx_propertyfields_create_at_id ON PropertyFields(CreateAt, ID);'
));
PREPARE createIndexIfNotExists FROM @preparedStatement;
EXECUTE createIndexIfNotExists;
DEALLOCATE PREPARE createIndexIfNotExists;

Просмотреть файл

@@ -0,0 +1,2 @@
-- morph:nontransactional
DROP INDEX CONCURRENTLY IF EXISTS idx_propertyvalues_create_at_id;

Просмотреть файл

@@ -0,0 +1,2 @@
-- morph:nontransactional
CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_propertyvalues_create_at_id ON PropertyValues(CreateAt, ID)

Просмотреть файл

@@ -0,0 +1,2 @@
-- morph:nontransactional
DROP INDEX CONCURRENTLY IF EXISTS idx_propertyfields_create_at_id;

Просмотреть файл

@@ -0,0 +1,2 @@
-- morph:nontransactional
CREATE INDEX CONCURRENTLY IF NOT EXISTS idx_propertyfields_create_at_id ON PropertyFields(CreateAt, ID)

Просмотреть файл

@@ -8949,6 +8949,27 @@ func (s *RetryLayerProductNoticesStore) View(userID string, notices []string) er
}
func (s *RetryLayerPropertyFieldStore) CountForGroup(groupID string, includeDeleted bool) (int64, error) {
tries := 0
for {
result, err := s.PropertyFieldStore.CountForGroup(groupID, includeDeleted)
if err == nil {
return result, nil
}
if !isRepeatableError(err) {
return result, err
}
tries++
if tries >= 3 {
err = errors.Wrap(err, "giving up after 3 consecutive repeatable transaction failures")
return result, err
}
timepkg.Sleep(100 * timepkg.Millisecond)
}
}
func (s *RetryLayerPropertyFieldStore) Create(field *model.PropertyField) (*model.PropertyField, error) {
tries := 0

Просмотреть файл

@@ -78,9 +78,26 @@ func (s *SqlPropertyFieldStore) GetMany(ids []string) ([]*model.PropertyField, e
return fields, nil
}
func (s *SqlPropertyFieldStore) CountForGroup(groupID string, includeDeleted bool) (int64, error) {
var count int64
builder := s.getQueryBuilder().
Select("COUNT(id)").
From("PropertyFields").
Where(sq.Eq{"GroupID": groupID})
if !includeDeleted {
builder = builder.Where(sq.Eq{"DeleteAt": 0})
}
if err := s.GetReplica().GetBuilder(&count, builder); err != nil {
return int64(0), errors.Wrap(err, "failed to count Sessions")
}
return count, nil
}
func (s *SqlPropertyFieldStore) SearchPropertyFields(opts model.PropertyFieldSearchOpts) ([]*model.PropertyField, error) {
if opts.Page < 0 {
return nil, errors.New("page must be positive integer")
if err := opts.Cursor.IsValid(); err != nil {
return nil, fmt.Errorf("cursor is invalid: %w", err)
}
if opts.PerPage < 1 {
@@ -88,10 +105,19 @@ func (s *SqlPropertyFieldStore) SearchPropertyFields(opts model.PropertyFieldSea
}
builder := s.tableSelectQuery.
OrderBy("CreateAt ASC").
Offset(uint64(opts.Page * opts.PerPage)).
OrderBy("CreateAt ASC, Id ASC").
Limit(uint64(opts.PerPage))
if !opts.Cursor.IsEmpty() {
builder = builder.Where(sq.Or{
sq.Gt{"CreateAt": opts.Cursor.CreateAt},
sq.And{
sq.Eq{"CreateAt": opts.Cursor.CreateAt},
sq.Gt{"Id": opts.Cursor.PropertyFieldID},
},
})
}
if !opts.IncludeDeleted {
builder = builder.Where(sq.Eq{"DeleteAt": 0})
}

Просмотреть файл

@@ -83,8 +83,8 @@ func (s *SqlPropertyValueStore) GetMany(ids []string) ([]*model.PropertyValue, e
}
func (s *SqlPropertyValueStore) SearchPropertyValues(opts model.PropertyValueSearchOpts) ([]*model.PropertyValue, error) {
if opts.Page < 0 {
return nil, errors.New("page must be positive integer")
if err := opts.Cursor.IsValid(); err != nil {
return nil, fmt.Errorf("cursor is invalid: %w", err)
}
if opts.PerPage < 1 {
@@ -92,10 +92,19 @@ func (s *SqlPropertyValueStore) SearchPropertyValues(opts model.PropertyValueSea
}
builder := s.tableSelectQuery.
OrderBy("CreateAt ASC").
Offset(uint64(opts.Page * opts.PerPage)).
OrderBy("CreateAt ASC, Id ASC").
Limit(uint64(opts.PerPage))
if !opts.Cursor.IsEmpty() {
builder = builder.Where(sq.Or{
sq.Gt{"CreateAt": opts.Cursor.CreateAt},
sq.And{
sq.Eq{"CreateAt": opts.Cursor.CreateAt},
sq.Gt{"Id": opts.Cursor.PropertyValueID},
},
})
}
if !opts.IncludeDeleted {
builder = builder.Where(sq.Eq{"DeleteAt": 0})
}

Просмотреть файл

@@ -1089,6 +1089,7 @@ type PropertyFieldStore interface {
Create(field *model.PropertyField) (*model.PropertyField, error)
Get(id string) (*model.PropertyField, error)
GetMany(ids []string) ([]*model.PropertyField, error)
CountForGroup(groupID string, includeDeleted bool) (int64, error)
SearchPropertyFields(opts model.PropertyFieldSearchOpts) ([]*model.PropertyField, error)
Update(fields []*model.PropertyField) ([]*model.PropertyField, error)
Delete(id string) error

Просмотреть файл

@@ -14,6 +14,34 @@ type PropertyFieldStore struct {
mock.Mock
}
// CountForGroup provides a mock function with given fields: groupID, includeDeleted
func (_m *PropertyFieldStore) CountForGroup(groupID string, includeDeleted bool) (int64, error) {
ret := _m.Called(groupID, includeDeleted)
if len(ret) == 0 {
panic("no return value specified for CountForGroup")
}
var r0 int64
var r1 error
if rf, ok := ret.Get(0).(func(string, bool) (int64, error)); ok {
return rf(groupID, includeDeleted)
}
if rf, ok := ret.Get(0).(func(string, bool) int64); ok {
r0 = rf(groupID, includeDeleted)
} else {
r0 = ret.Get(0).(int64)
}
if rf, ok := ret.Get(1).(func(string, bool) error); ok {
r1 = rf(groupID, includeDeleted)
} else {
r1 = ret.Error(1)
}
return r0, r1
}
// Create provides a mock function with given fields: field
func (_m *PropertyFieldStore) Create(field *model.PropertyField) (*model.PropertyField, error) {
ret := _m.Called(field)

Просмотреть файл

@@ -5,6 +5,7 @@ package storetest
import (
"database/sql"
"fmt"
"testing"
"time"
@@ -21,6 +22,7 @@ func TestPropertyFieldStore(t *testing.T, rctx request.CTX, ss store.Store, s Sq
t.Run("UpdatePropertyField", func(t *testing.T) { testUpdatePropertyField(t, rctx, ss) })
t.Run("DeletePropertyField", func(t *testing.T) { testDeletePropertyField(t, rctx, ss) })
t.Run("SearchPropertyFields", func(t *testing.T) { testSearchPropertyFields(t, rctx, ss) })
t.Run("CountForGroup", func(t *testing.T) { testCountForGroup(t, rctx, ss) })
}
func testCreatePropertyField(t *testing.T, _ request.CTX, ss store.Store) {
@@ -356,6 +358,97 @@ func testDeletePropertyField(t *testing.T, _ request.CTX, ss store.Store) {
})
}
func testCountForGroup(t *testing.T, _ request.CTX, ss store.Store) {
t.Run("should return 0 for group with no properties", func(t *testing.T) {
count, err := ss.PropertyField().CountForGroup(model.NewId(), false)
require.NoError(t, err)
require.Equal(t, int64(0), count)
})
t.Run("should return correct count for group with properties", func(t *testing.T) {
groupID := model.NewId()
// Create 5 property fields
for i := 0; i < 5; i++ {
field := &model.PropertyField{
GroupID: groupID,
Name: fmt.Sprintf("Field %d", i),
Type: model.PropertyFieldTypeText,
}
_, err := ss.PropertyField().Create(field)
require.NoError(t, err)
}
count, err := ss.PropertyField().CountForGroup(groupID, false)
require.NoError(t, err)
require.Equal(t, int64(5), count)
})
t.Run("should not count deleted properties when includeDeleted is false", func(t *testing.T) {
groupID := model.NewId()
// Create 5 property fields
for i := 0; i < 5; i++ {
field := &model.PropertyField{
GroupID: groupID,
Name: fmt.Sprintf("Field %d", i),
Type: model.PropertyFieldTypeText,
}
_, err := ss.PropertyField().Create(field)
require.NoError(t, err)
}
// Create one more and delete it
deletedField := &model.PropertyField{
GroupID: groupID,
Name: "To be deleted",
Type: model.PropertyFieldTypeText,
}
_, err := ss.PropertyField().Create(deletedField)
require.NoError(t, err)
err = ss.PropertyField().Delete(deletedField.ID)
require.NoError(t, err)
// Count should be 5 since the deleted field shouldn't be counted
count, err := ss.PropertyField().CountForGroup(groupID, false)
require.NoError(t, err)
require.Equal(t, int64(5), count)
})
t.Run("should count deleted properties when includeDeleted is true", func(t *testing.T) {
groupID := model.NewId()
// Create 5 property fields
for i := 0; i < 5; i++ {
field := &model.PropertyField{
GroupID: groupID,
Name: fmt.Sprintf("Field %d", i),
Type: model.PropertyFieldTypeText,
}
_, err := ss.PropertyField().Create(field)
require.NoError(t, err)
}
// Create one more and delete it
deletedField := &model.PropertyField{
GroupID: groupID,
Name: "To be deleted",
Type: model.PropertyFieldTypeText,
}
_, err := ss.PropertyField().Create(deletedField)
require.NoError(t, err)
err = ss.PropertyField().Delete(deletedField.ID)
require.NoError(t, err)
// Count should be 6 since we're including deleted fields
count, err := ss.PropertyField().CountForGroup(groupID, true)
require.NoError(t, err)
require.Equal(t, int64(6), count)
})
}
func testSearchPropertyFields(t *testing.T, _ request.CTX, ss store.Store) {
groupID := model.NewId()
targetID := model.NewId()
@@ -406,18 +499,9 @@ func testSearchPropertyFields(t *testing.T, _ request.CTX, ss store.Store) {
expectedError bool
expectedIDs []string
}{
{
name: "negative page",
opts: model.PropertyFieldSearchOpts{
Page: -1,
PerPage: 10,
},
expectedError: true,
},
{
name: "negative per_page",
opts: model.PropertyFieldSearchOpts{
Page: 0,
PerPage: -1,
},
expectedError: true,
@@ -426,7 +510,6 @@ func testSearchPropertyFields(t *testing.T, _ request.CTX, ss store.Store) {
name: "filter by group_id",
opts: model.PropertyFieldSearchOpts{
GroupID: groupID,
Page: 0,
PerPage: 10,
},
expectedIDs: []string{field1.ID, field2.ID},
@@ -435,7 +518,6 @@ func testSearchPropertyFields(t *testing.T, _ request.CTX, ss store.Store) {
name: "filter by group_id including deleted",
opts: model.PropertyFieldSearchOpts{
GroupID: groupID,
Page: 0,
PerPage: 10,
IncludeDeleted: true,
},
@@ -445,7 +527,6 @@ func testSearchPropertyFields(t *testing.T, _ request.CTX, ss store.Store) {
name: "filter by target_type",
opts: model.PropertyFieldSearchOpts{
TargetType: "test_type",
Page: 0,
PerPage: 10,
},
expectedIDs: []string{field1.ID, field3.ID},
@@ -454,7 +535,6 @@ func testSearchPropertyFields(t *testing.T, _ request.CTX, ss store.Store) {
name: "filter by target_id",
opts: model.PropertyFieldSearchOpts{
TargetID: targetID,
Page: 0,
PerPage: 10,
},
expectedIDs: []string{field1.ID, field2.ID},
@@ -463,7 +543,6 @@ func testSearchPropertyFields(t *testing.T, _ request.CTX, ss store.Store) {
name: "pagination page 0",
opts: model.PropertyFieldSearchOpts{
GroupID: groupID,
Page: 0,
PerPage: 2,
IncludeDeleted: true,
},
@@ -472,8 +551,11 @@ func testSearchPropertyFields(t *testing.T, _ request.CTX, ss store.Store) {
{
name: "pagination page 1",
opts: model.PropertyFieldSearchOpts{
GroupID: groupID,
Page: 1,
GroupID: groupID,
Cursor: model.PropertyFieldSearchCursor{
CreateAt: field2.CreateAt,
PropertyFieldID: field2.ID,
},
PerPage: 2,
IncludeDeleted: true,
},
@@ -490,7 +572,7 @@ func testSearchPropertyFields(t *testing.T, _ request.CTX, ss store.Store) {
}
require.NoError(t, err)
var ids = make([]string, len(results))
ids := make([]string, len(results))
for i, field := range results {
ids[i] = field.ID
}

Просмотреть файл

@@ -567,18 +567,9 @@ func testSearchPropertyValues(t *testing.T, _ request.CTX, ss store.Store) {
expectedError bool
expectedIDs []string
}{
{
name: "negative page",
opts: model.PropertyValueSearchOpts{
Page: -1,
PerPage: 10,
},
expectedError: true,
},
{
name: "negative per_page",
opts: model.PropertyValueSearchOpts{
Page: 0,
PerPage: -1,
},
expectedError: true,
@@ -587,7 +578,6 @@ func testSearchPropertyValues(t *testing.T, _ request.CTX, ss store.Store) {
name: "filter by group_id",
opts: model.PropertyValueSearchOpts{
GroupID: groupID,
Page: 0,
PerPage: 10,
},
expectedIDs: []string{value1.ID, value2.ID},
@@ -597,7 +587,6 @@ func testSearchPropertyValues(t *testing.T, _ request.CTX, ss store.Store) {
opts: model.PropertyValueSearchOpts{
GroupID: groupID,
TargetType: "test_type",
Page: 0,
PerPage: 10,
},
expectedIDs: []string{value1.ID},
@@ -608,7 +597,6 @@ func testSearchPropertyValues(t *testing.T, _ request.CTX, ss store.Store) {
GroupID: groupID,
TargetType: "test_type",
IncludeDeleted: true,
Page: 0,
PerPage: 10,
},
expectedIDs: []string{value1.ID, value4.ID},
@@ -617,7 +605,6 @@ func testSearchPropertyValues(t *testing.T, _ request.CTX, ss store.Store) {
name: "filter by target_id",
opts: model.PropertyValueSearchOpts{
TargetID: targetID,
Page: 0,
PerPage: 10,
},
expectedIDs: []string{value1.ID, value2.ID},
@@ -627,7 +614,6 @@ func testSearchPropertyValues(t *testing.T, _ request.CTX, ss store.Store) {
opts: model.PropertyValueSearchOpts{
GroupID: groupID,
TargetID: targetID,
Page: 0,
PerPage: 10,
},
expectedIDs: []string{value1.ID, value2.ID},
@@ -636,7 +622,6 @@ func testSearchPropertyValues(t *testing.T, _ request.CTX, ss store.Store) {
name: "filter by field_id",
opts: model.PropertyValueSearchOpts{
FieldID: fieldID,
Page: 0,
PerPage: 10,
},
expectedIDs: []string{value1.ID},
@@ -646,7 +631,6 @@ func testSearchPropertyValues(t *testing.T, _ request.CTX, ss store.Store) {
opts: model.PropertyValueSearchOpts{
FieldID: fieldID,
IncludeDeleted: true,
Page: 0,
PerPage: 10,
},
expectedIDs: []string{value1.ID, value4.ID},
@@ -655,7 +639,6 @@ func testSearchPropertyValues(t *testing.T, _ request.CTX, ss store.Store) {
name: "pagination page 0",
opts: model.PropertyValueSearchOpts{
GroupID: groupID,
Page: 0,
PerPage: 1,
},
expectedIDs: []string{value1.ID},
@@ -664,7 +647,10 @@ func testSearchPropertyValues(t *testing.T, _ request.CTX, ss store.Store) {
name: "pagination page 1",
opts: model.PropertyValueSearchOpts{
GroupID: groupID,
Page: 1,
Cursor: model.PropertyValueSearchCursor{
CreateAt: value1.CreateAt,
PropertyValueID: value1.ID,
},
PerPage: 1,
},
expectedIDs: []string{value2.ID},
@@ -680,7 +666,7 @@ func testSearchPropertyValues(t *testing.T, _ request.CTX, ss store.Store) {
}
require.NoError(t, err)
var ids = make([]string, len(results))
ids := make([]string, len(results))
for i, value := range results {
ids[i] = value.ID
}

Просмотреть файл

@@ -7105,6 +7105,22 @@ func (s *TimerLayerProductNoticesStore) View(userID string, notices []string) er
return err
}
func (s *TimerLayerPropertyFieldStore) CountForGroup(groupID string, includeDeleted bool) (int64, error) {
start := time.Now()
result, err := s.PropertyFieldStore.CountForGroup(groupID, includeDeleted)
elapsed := float64(time.Since(start)) / float64(time.Second)
if s.Root.Metrics != nil {
success := "false"
if err == nil {
success = "true"
}
s.Root.Metrics.ObserveStoreMethodDuration("PropertyFieldStore.CountForGroup", success, elapsed)
}
return result, err
}
func (s *TimerLayerPropertyFieldStore) Create(field *model.PropertyField) (*model.PropertyField, error) {
start := time.Now()

Просмотреть файл

@@ -5006,6 +5006,10 @@
"id": "app.custom_group.unique_name",
"translation": "group name is not unique"
},
{
"id": "app.custom_profile_attributes.count_property_fields.app_error",
"translation": "Unable to count the number of fields for the custom profile attribute group"
},
{
"id": "app.custom_profile_attributes.cpa_group_id.app_error",
"translation": "Cannot register Custom Profile Attributes property group"

Просмотреть файл

@@ -4,6 +4,7 @@
package model
import (
"errors"
"net/http"
"strings"
)
@@ -141,11 +142,35 @@ func (pf *PropertyField) Patch(patch *PropertyFieldPatch) {
}
}
type PropertyFieldSearchCursor struct {
PropertyFieldID string
CreateAt int64
}
func (p PropertyFieldSearchCursor) IsEmpty() bool {
return p.PropertyFieldID == "" && p.CreateAt == 0
}
func (p PropertyFieldSearchCursor) IsValid() error {
if p.IsEmpty() {
return nil
}
if p.CreateAt <= 0 {
return errors.New("create at cannot be negative or zero")
}
if !IsValidId(p.PropertyFieldID) {
return errors.New("property field id is invalid")
}
return nil
}
type PropertyFieldSearchOpts struct {
GroupID string
TargetType string
TargetID string
IncludeDeleted bool
Page int
Cursor PropertyFieldSearchCursor
PerPage int
}

218
server/public/model/property_field_test.go Обычный файл
Просмотреть файл

@@ -0,0 +1,218 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
package model
import (
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestPropertyField_PreSave(t *testing.T) {
t.Run("sets ID if empty", func(t *testing.T) {
pf := &PropertyField{}
pf.PreSave()
assert.NotEmpty(t, pf.ID)
assert.Len(t, pf.ID, 26) // Length of NewId()
})
t.Run("keeps existing ID", func(t *testing.T) {
pf := &PropertyField{ID: "existing_id"}
pf.PreSave()
assert.Equal(t, "existing_id", pf.ID)
})
t.Run("sets CreateAt if zero", func(t *testing.T) {
pf := &PropertyField{}
pf.PreSave()
assert.NotZero(t, pf.CreateAt)
})
t.Run("sets UpdateAt equal to CreateAt", func(t *testing.T) {
pf := &PropertyField{}
pf.PreSave()
assert.Equal(t, pf.CreateAt, pf.UpdateAt)
})
}
func TestPropertyField_IsValid(t *testing.T) {
t.Run("valid field", func(t *testing.T) {
pf := &PropertyField{
ID: NewId(),
GroupID: NewId(),
Name: "test field",
Type: PropertyFieldTypeText,
CreateAt: GetMillis(),
UpdateAt: GetMillis(),
}
require.NoError(t, pf.IsValid())
})
t.Run("invalid ID", func(t *testing.T) {
pf := &PropertyField{
ID: "invalid",
GroupID: NewId(),
Name: "test field",
Type: PropertyFieldTypeText,
CreateAt: GetMillis(),
UpdateAt: GetMillis(),
}
require.Error(t, pf.IsValid())
})
t.Run("invalid GroupID", func(t *testing.T) {
pf := &PropertyField{
ID: NewId(),
GroupID: "invalid",
Name: "test field",
Type: PropertyFieldTypeText,
CreateAt: GetMillis(),
UpdateAt: GetMillis(),
}
require.Error(t, pf.IsValid())
})
t.Run("empty name", func(t *testing.T) {
pf := &PropertyField{
ID: NewId(),
GroupID: NewId(),
Name: "",
Type: PropertyFieldTypeText,
CreateAt: GetMillis(),
UpdateAt: GetMillis(),
}
require.Error(t, pf.IsValid())
})
t.Run("invalid type", func(t *testing.T) {
pf := &PropertyField{
ID: NewId(),
GroupID: NewId(),
Name: "test field",
Type: "invalid",
CreateAt: GetMillis(),
UpdateAt: GetMillis(),
}
require.Error(t, pf.IsValid())
})
t.Run("zero CreateAt", func(t *testing.T) {
pf := &PropertyField{
ID: NewId(),
GroupID: NewId(),
Name: "test field",
Type: PropertyFieldTypeText,
CreateAt: 0,
UpdateAt: GetMillis(),
}
require.Error(t, pf.IsValid())
})
t.Run("zero UpdateAt", func(t *testing.T) {
pf := &PropertyField{
ID: NewId(),
GroupID: NewId(),
Name: "test field",
Type: PropertyFieldTypeText,
CreateAt: GetMillis(),
UpdateAt: 0,
}
require.Error(t, pf.IsValid())
})
}
func TestPropertyField_SanitizeInput(t *testing.T) {
t.Run("trims spaces from name", func(t *testing.T) {
pf := &PropertyField{Name: " test field "}
pf.SanitizeInput()
assert.Equal(t, "test field", pf.Name)
})
}
func TestPropertyField_Patch(t *testing.T) {
t.Run("patches all fields", func(t *testing.T) {
pf := &PropertyField{
Name: "original name",
Type: PropertyFieldTypeText,
TargetID: "original_target",
TargetType: "original_type",
}
patch := &PropertyFieldPatch{
Name: NewPointer("new name"),
Type: NewPointer(PropertyFieldTypeSelect),
TargetID: NewPointer("new_target"),
TargetType: NewPointer("new_type"),
Attrs: &map[string]any{"key": "value"},
}
pf.Patch(patch)
assert.Equal(t, "new name", pf.Name)
assert.Equal(t, PropertyFieldTypeSelect, pf.Type)
assert.Equal(t, "new_target", pf.TargetID)
assert.Equal(t, "new_type", pf.TargetType)
assert.EqualValues(t, StringInterface{"key": "value"}, pf.Attrs)
})
t.Run("patches only specified fields", func(t *testing.T) {
pf := &PropertyField{
Name: "original name",
Type: PropertyFieldTypeText,
TargetID: "original_target",
TargetType: "original_type",
}
patch := &PropertyFieldPatch{
Name: NewPointer("new name"),
}
pf.Patch(patch)
assert.Equal(t, "new name", pf.Name)
assert.Equal(t, PropertyFieldTypeText, pf.Type)
assert.Equal(t, "original_target", pf.TargetID)
assert.Equal(t, "original_type", pf.TargetType)
})
}
func TestPropertyFieldSearchCursor_IsValid(t *testing.T) {
t.Run("empty cursor is valid", func(t *testing.T) {
cursor := PropertyFieldSearchCursor{}
assert.NoError(t, cursor.IsValid())
})
t.Run("valid cursor", func(t *testing.T) {
cursor := PropertyFieldSearchCursor{
PropertyFieldID: NewId(),
CreateAt: GetMillis(),
}
assert.NoError(t, cursor.IsValid())
})
t.Run("invalid PropertyFieldID", func(t *testing.T) {
cursor := PropertyFieldSearchCursor{
PropertyFieldID: "invalid",
CreateAt: GetMillis(),
}
assert.Error(t, cursor.IsValid())
})
t.Run("zero CreateAt", func(t *testing.T) {
cursor := PropertyFieldSearchCursor{
PropertyFieldID: NewId(),
CreateAt: 0,
}
assert.Error(t, cursor.IsValid())
})
t.Run("negative CreateAt", func(t *testing.T) {
cursor := PropertyFieldSearchCursor{
PropertyFieldID: NewId(),
CreateAt: -1,
}
assert.Error(t, cursor.IsValid())
})
}

Просмотреть файл

@@ -6,6 +6,8 @@ package model
import (
"encoding/json"
"net/http"
"github.com/pkg/errors"
)
type PropertyValue struct {
@@ -63,12 +65,36 @@ func (pv *PropertyValue) IsValid() error {
return nil
}
type PropertyValueSearchCursor struct {
PropertyValueID string
CreateAt int64
}
func (p PropertyValueSearchCursor) IsEmpty() bool {
return p.PropertyValueID == "" && p.CreateAt == 0
}
func (p PropertyValueSearchCursor) IsValid() error {
if p.IsEmpty() {
return nil
}
if p.CreateAt <= 0 {
return errors.New("create at cannot be negative or zero")
}
if !IsValidId(p.PropertyValueID) {
return errors.New("property field id is invalid")
}
return nil
}
type PropertyValueSearchOpts struct {
GroupID string
TargetType string
TargetID string
FieldID string
IncludeDeleted bool
Page int
Cursor PropertyValueSearchCursor
PerPage int
}

186
server/public/model/property_value_test.go Обычный файл
Просмотреть файл

@@ -0,0 +1,186 @@
// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
// See LICENSE.txt for license information.
package model
import (
"encoding/json"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestPropertyValue_PreSave(t *testing.T) {
t.Run("sets ID if empty", func(t *testing.T) {
pv := &PropertyValue{}
pv.PreSave()
assert.NotEmpty(t, pv.ID)
assert.Len(t, pv.ID, 26) // Length of NewId()
})
t.Run("keeps existing ID", func(t *testing.T) {
pv := &PropertyValue{ID: "existing_id"}
pv.PreSave()
assert.Equal(t, "existing_id", pv.ID)
})
t.Run("sets CreateAt if zero", func(t *testing.T) {
pv := &PropertyValue{}
pv.PreSave()
assert.NotZero(t, pv.CreateAt)
})
t.Run("sets UpdateAt equal to CreateAt", func(t *testing.T) {
pv := &PropertyValue{}
pv.PreSave()
assert.Equal(t, pv.CreateAt, pv.UpdateAt)
})
}
func TestPropertyValue_IsValid(t *testing.T) {
t.Run("valid value", func(t *testing.T) {
value := json.RawMessage(`{"test": "value"}`)
pv := &PropertyValue{
ID: NewId(),
TargetID: NewId(),
TargetType: "test_type",
GroupID: NewId(),
FieldID: NewId(),
Value: value,
CreateAt: GetMillis(),
UpdateAt: GetMillis(),
}
require.NoError(t, pv.IsValid())
})
t.Run("invalid ID", func(t *testing.T) {
pv := &PropertyValue{
ID: "invalid",
TargetID: NewId(),
TargetType: "test_type",
GroupID: NewId(),
FieldID: NewId(),
CreateAt: GetMillis(),
UpdateAt: GetMillis(),
}
require.Error(t, pv.IsValid())
})
t.Run("invalid TargetID", func(t *testing.T) {
pv := &PropertyValue{
ID: NewId(),
TargetID: "invalid",
TargetType: "test_type",
GroupID: NewId(),
FieldID: NewId(),
CreateAt: GetMillis(),
UpdateAt: GetMillis(),
}
require.Error(t, pv.IsValid())
})
t.Run("empty TargetType", func(t *testing.T) {
pv := &PropertyValue{
ID: NewId(),
TargetID: NewId(),
TargetType: "",
GroupID: NewId(),
FieldID: NewId(),
CreateAt: GetMillis(),
UpdateAt: GetMillis(),
}
require.Error(t, pv.IsValid())
})
t.Run("invalid GroupID", func(t *testing.T) {
pv := &PropertyValue{
ID: NewId(),
TargetID: NewId(),
TargetType: "test_type",
GroupID: "invalid",
FieldID: NewId(),
CreateAt: GetMillis(),
UpdateAt: GetMillis(),
}
require.Error(t, pv.IsValid())
})
t.Run("invalid FieldID", func(t *testing.T) {
pv := &PropertyValue{
ID: NewId(),
TargetID: NewId(),
TargetType: "test_type",
GroupID: NewId(),
FieldID: "invalid",
CreateAt: GetMillis(),
UpdateAt: GetMillis(),
}
require.Error(t, pv.IsValid())
})
t.Run("zero CreateAt", func(t *testing.T) {
pv := &PropertyValue{
ID: NewId(),
TargetID: NewId(),
TargetType: "test_type",
GroupID: NewId(),
FieldID: NewId(),
CreateAt: 0,
UpdateAt: GetMillis(),
}
require.Error(t, pv.IsValid())
})
t.Run("zero UpdateAt", func(t *testing.T) {
pv := &PropertyValue{
ID: NewId(),
TargetID: NewId(),
TargetType: "test_type",
GroupID: NewId(),
FieldID: NewId(),
CreateAt: GetMillis(),
UpdateAt: 0,
}
require.Error(t, pv.IsValid())
})
}
func TestPropertyValueSearchCursor_IsValid(t *testing.T) {
t.Run("empty cursor is valid", func(t *testing.T) {
cursor := PropertyValueSearchCursor{}
assert.NoError(t, cursor.IsValid())
})
t.Run("valid cursor", func(t *testing.T) {
cursor := PropertyValueSearchCursor{
PropertyValueID: NewId(),
CreateAt: GetMillis(),
}
assert.NoError(t, cursor.IsValid())
})
t.Run("invalid PropertyValueID", func(t *testing.T) {
cursor := PropertyValueSearchCursor{
PropertyValueID: "invalid",
CreateAt: GetMillis(),
}
assert.Error(t, cursor.IsValid())
})
t.Run("zero CreateAt", func(t *testing.T) {
cursor := PropertyValueSearchCursor{
PropertyValueID: NewId(),
CreateAt: 0,
}
assert.Error(t, cursor.IsValid())
})
t.Run("negative CreateAt", func(t *testing.T) {
cursor := PropertyValueSearchCursor{
PropertyValueID: NewId(),
CreateAt: -1,
}
assert.Error(t, cursor.IsValid())
})
}