diff --git a/server/channels/app/properties/property_field.go b/server/channels/app/properties/property_field.go index ddf9387642..388b3478df 100644 --- a/server/channels/app/properties/property_field.go +++ b/server/channels/app/properties/property_field.go @@ -21,6 +21,10 @@ func (ps *PropertyService) GetPropertyFields(groupID string, ids []string) ([]*m return ps.fieldStore.GetMany(groupID, ids) } +func (ps *PropertyService) GetPropertyFieldByName(groupID, targetID, name string) (*model.PropertyField, error) { + return ps.fieldStore.GetFieldByName(groupID, targetID, name) +} + func (ps *PropertyService) CountActivePropertyFieldsForGroup(groupID string) (int64, error) { return ps.fieldStore.CountForGroup(groupID, false) } diff --git a/server/channels/store/retrylayer/retrylayer.go b/server/channels/store/retrylayer/retrylayer.go index 1feea42a13..0f2208f081 100644 --- a/server/channels/store/retrylayer/retrylayer.go +++ b/server/channels/store/retrylayer/retrylayer.go @@ -9190,6 +9190,27 @@ func (s *RetryLayerPropertyFieldStore) Get(groupID string, id string) (*model.Pr } +func (s *RetryLayerPropertyFieldStore) GetFieldByName(groupID string, targetID string, name string) (*model.PropertyField, error) { + + tries := 0 + for { + result, err := s.PropertyFieldStore.GetFieldByName(groupID, targetID, name) + 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) GetMany(groupID string, ids []string) ([]*model.PropertyField, error) { tries := 0 diff --git a/server/channels/store/sqlstore/property_field_store.go b/server/channels/store/sqlstore/property_field_store.go index 27a294f42b..ab2e541b9e 100644 --- a/server/channels/store/sqlstore/property_field_store.go +++ b/server/channels/store/sqlstore/property_field_store.go @@ -67,6 +67,21 @@ func (s *SqlPropertyFieldStore) Get(groupID, id string) (*model.PropertyField, e return &field, nil } +func (s *SqlPropertyFieldStore) GetFieldByName(groupID, targetID, name string) (*model.PropertyField, error) { + builder := s.tableSelectQuery. + Where(sq.Eq{"GroupID": groupID}). + Where(sq.Eq{"TargetID": targetID}). + Where(sq.Eq{"Name": name}). + Where(sq.Eq{"DeleteAt": 0}) + + var field model.PropertyField + if err := s.GetReplica().GetBuilder(&field, builder); err != nil { + return nil, errors.Wrap(err, "property_field_get_by_name_select") + } + + return &field, nil +} + func (s *SqlPropertyFieldStore) GetMany(groupID string, ids []string) ([]*model.PropertyField, error) { builder := s.tableSelectQuery.Where(sq.Eq{"id": ids}) diff --git a/server/channels/store/store.go b/server/channels/store/store.go index 008855c7dc..a51778e03b 100644 --- a/server/channels/store/store.go +++ b/server/channels/store/store.go @@ -1092,6 +1092,7 @@ type PropertyFieldStore interface { Create(field *model.PropertyField) (*model.PropertyField, error) Get(groupID, id string) (*model.PropertyField, error) GetMany(groupID string, ids []string) ([]*model.PropertyField, error) + GetFieldByName(groupID, targetID, name string) (*model.PropertyField, error) CountForGroup(groupID string, includeDeleted bool) (int64, error) SearchPropertyFields(opts model.PropertyFieldSearchOpts) ([]*model.PropertyField, error) Update(groupID string, fields []*model.PropertyField) ([]*model.PropertyField, error) diff --git a/server/channels/store/storetest/mocks/PropertyFieldStore.go b/server/channels/store/storetest/mocks/PropertyFieldStore.go index 58f7b571ce..e627df3533 100644 --- a/server/channels/store/storetest/mocks/PropertyFieldStore.go +++ b/server/channels/store/storetest/mocks/PropertyFieldStore.go @@ -120,6 +120,36 @@ func (_m *PropertyFieldStore) Get(groupID string, id string) (*model.PropertyFie return r0, r1 } +// GetFieldByName provides a mock function with given fields: groupID, targetID, name +func (_m *PropertyFieldStore) GetFieldByName(groupID string, targetID string, name string) (*model.PropertyField, error) { + ret := _m.Called(groupID, targetID, name) + + if len(ret) == 0 { + panic("no return value specified for GetFieldByName") + } + + var r0 *model.PropertyField + var r1 error + if rf, ok := ret.Get(0).(func(string, string, string) (*model.PropertyField, error)); ok { + return rf(groupID, targetID, name) + } + if rf, ok := ret.Get(0).(func(string, string, string) *model.PropertyField); ok { + r0 = rf(groupID, targetID, name) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(*model.PropertyField) + } + } + + if rf, ok := ret.Get(1).(func(string, string, string) error); ok { + r1 = rf(groupID, targetID, name) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // GetMany provides a mock function with given fields: groupID, ids func (_m *PropertyFieldStore) GetMany(groupID string, ids []string) ([]*model.PropertyField, error) { ret := _m.Called(groupID, ids) diff --git a/server/channels/store/storetest/property_field_store.go b/server/channels/store/storetest/property_field_store.go index 21a8148658..3f3346d6f9 100644 --- a/server/channels/store/storetest/property_field_store.go +++ b/server/channels/store/storetest/property_field_store.go @@ -19,6 +19,7 @@ func TestPropertyFieldStore(t *testing.T, rctx request.CTX, ss store.Store, s Sq t.Run("CreatePropertyField", func(t *testing.T) { testCreatePropertyField(t, rctx, ss) }) t.Run("GetPropertyField", func(t *testing.T) { testGetPropertyField(t, rctx, ss) }) t.Run("GetManyPropertyFields", func(t *testing.T) { testGetManyPropertyFields(t, rctx, ss) }) + t.Run("GetFieldByName", func(t *testing.T) { testGetFieldByName(t, rctx, ss) }) 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) }) @@ -173,6 +174,162 @@ func testGetManyPropertyFields(t *testing.T, _ request.CTX, ss store.Store) { }) } +func testGetFieldByName(t *testing.T, _ request.CTX, ss store.Store) { + t.Run("should fail on nonexisting field", func(t *testing.T) { + field, err := ss.PropertyField().GetFieldByName("", "", "nonexistent-field-name") + require.Zero(t, field) + require.ErrorIs(t, err, sql.ErrNoRows) + }) + + groupID := model.NewId() + targetID := model.NewId() + newField := &model.PropertyField{ + GroupID: groupID, + TargetID: targetID, + Name: "unique-field-name", + Type: model.PropertyFieldTypeText, + Attrs: map[string]any{ + "locked": true, + "special": "value", + }, + } + _, cErr := ss.PropertyField().Create(newField) + require.NoError(t, cErr) + require.NotZero(t, newField.ID) + + t.Run("should be able to retrieve an existing property field by name", func(t *testing.T) { + field, err := ss.PropertyField().GetFieldByName(groupID, targetID, "unique-field-name") + require.NoError(t, err) + require.Equal(t, newField.ID, field.ID) + require.Equal(t, "unique-field-name", field.Name) + require.True(t, field.Attrs["locked"].(bool)) + require.Equal(t, "value", field.Attrs["special"]) + }) + + t.Run("should not be able to retrieve an existing field when specifying a different group ID", func(t *testing.T) { + field, err := ss.PropertyField().GetFieldByName(model.NewId(), targetID, "unique-field-name") + require.Zero(t, field) + require.ErrorIs(t, err, sql.ErrNoRows) + }) + + t.Run("should not be able to retrieve an existing field when specifying a different target ID", func(t *testing.T) { + field, err := ss.PropertyField().GetFieldByName(groupID, model.NewId(), "unique-field-name") + require.Zero(t, field) + require.ErrorIs(t, err, sql.ErrNoRows) + }) + + // Test with multiple fields with the same name but different groups + anotherGroupID := model.NewId() + duplicateNameField := &model.PropertyField{ + GroupID: anotherGroupID, + TargetID: targetID, + Name: "unique-field-name", // Same name as the first field + Type: model.PropertyFieldTypeSelect, + Attrs: map[string]any{ + "options": []string{"a", "b", "c"}, + }, + } + _, cErr = ss.PropertyField().Create(duplicateNameField) + require.NoError(t, cErr) + require.NotZero(t, duplicateNameField.ID) + + t.Run("should retrieve the correct field when multiple fields have the same name but different groups", func(t *testing.T) { + // Get the field from the first group + field, err := ss.PropertyField().GetFieldByName(groupID, targetID, "unique-field-name") + require.NoError(t, err) + require.Equal(t, newField.ID, field.ID) + require.Equal(t, model.PropertyFieldTypeText, field.Type) + + // Get the field from the second group + field, err = ss.PropertyField().GetFieldByName(anotherGroupID, targetID, "unique-field-name") + require.NoError(t, err) + require.Equal(t, duplicateNameField.ID, field.ID) + require.Equal(t, model.PropertyFieldTypeSelect, field.Type) + }) + + // Test with multiple fields with the same name and same group but different target IDs + anotherTargetID := model.NewId() + sameGroupDifferentTargetField := &model.PropertyField{ + GroupID: groupID, + TargetID: anotherTargetID, + Name: "unique-field-name", // Same name as the first field + Type: model.PropertyFieldTypeText, + Attrs: map[string]any{ + "min": 1, + "max": 100, + }, + } + _, cErr = ss.PropertyField().Create(sameGroupDifferentTargetField) + require.NoError(t, cErr) + require.NotZero(t, sameGroupDifferentTargetField.ID) + + t.Run("should retrieve the correct field when multiple fields have the same name and group but different target IDs", func(t *testing.T) { + // Get the field with the first target ID + field, err := ss.PropertyField().GetFieldByName(groupID, targetID, "unique-field-name") + require.NoError(t, err) + require.Equal(t, newField.ID, field.ID) + require.Equal(t, model.PropertyFieldTypeText, field.Type) + + // Get the field with the second target ID + field, err = ss.PropertyField().GetFieldByName(groupID, anotherTargetID, "unique-field-name") + require.NoError(t, err) + require.Equal(t, sameGroupDifferentTargetField.ID, field.ID) + require.Equal(t, model.PropertyFieldTypeText, field.Type) + }) + + // Test with a deleted field + t.Run("should not retrieve deleted fields", func(t *testing.T) { + // Create another field with a unique name + deletedField := &model.PropertyField{ + GroupID: groupID, + TargetID: targetID, + Name: "to-be-deleted-field", + Type: model.PropertyFieldTypeText, + } + _, cErr := ss.PropertyField().Create(deletedField) + require.NoError(t, cErr) + require.NotZero(t, deletedField.ID) + + // Verify it can be retrieved before deletion + field, err := ss.PropertyField().GetFieldByName(groupID, targetID, "to-be-deleted-field") + require.NoError(t, err) + require.Equal(t, deletedField.ID, field.ID) + + // Delete the field + err = ss.PropertyField().Delete("", deletedField.ID) + require.NoError(t, err) + + // Verify it can't be retrieved after deletion + field, err = ss.PropertyField().GetFieldByName(groupID, targetID, "to-be-deleted-field") + require.Zero(t, field) + require.ErrorIs(t, err, sql.ErrNoRows) + }) + + t.Run("should not retrieve fields with matching name but different DeleteAt status", func(t *testing.T) { + // Create a field with the same name/group/target as the deleted one + replacementField := &model.PropertyField{ + GroupID: groupID, + TargetID: targetID, + Name: "to-be-deleted-field", // Same name as the deleted field + Type: model.PropertyFieldTypeText, + Attrs: map[string]any{ + "min": 0, + "max": 10, + }, + } + _, cErr := ss.PropertyField().Create(replacementField) + require.NoError(t, cErr) + require.NotZero(t, replacementField.ID) + + // Verify only the non-deleted field is retrieved + field, err := ss.PropertyField().GetFieldByName(groupID, targetID, "to-be-deleted-field") + require.NoError(t, err) + require.Equal(t, replacementField.ID, field.ID) + require.Equal(t, model.PropertyFieldTypeText, field.Type) + require.Zero(t, field.DeleteAt) + }) +} + func testUpdatePropertyField(t *testing.T, _ request.CTX, ss store.Store) { t.Run("should fail on nonexisting field", func(t *testing.T) { field := &model.PropertyField{ diff --git a/server/channels/store/timerlayer/timerlayer.go b/server/channels/store/timerlayer/timerlayer.go index baca906301..b26fdda458 100644 --- a/server/channels/store/timerlayer/timerlayer.go +++ b/server/channels/store/timerlayer/timerlayer.go @@ -7291,6 +7291,22 @@ func (s *TimerLayerPropertyFieldStore) Get(groupID string, id string) (*model.Pr return result, err } +func (s *TimerLayerPropertyFieldStore) GetFieldByName(groupID string, targetID string, name string) (*model.PropertyField, error) { + start := time.Now() + + result, err := s.PropertyFieldStore.GetFieldByName(groupID, targetID, name) + + 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.GetFieldByName", success, elapsed) + } + return result, err +} + func (s *TimerLayerPropertyFieldStore) GetMany(groupID string, ids []string) ([]*model.PropertyField, error) { start := time.Now()