mirrors/mostlymatter
1
0
Форкнуть 0
Код Задачи Запросы на слияние Действия Пакеты Проекты Релизы Вики Активность

Updates the property service and store method signatures (#30103)

Просмотр исходного кода 
* Updates the property service and store method signatures

Getters can now receive a `groupID` that narrows down the query if
present, so it's not necessary to check for the group ID on the
returning values from the outside layers.

The Search methods now receive the `groupID` and the `targetID`
explicitly as parameters, incentivizing the use of the indexes that
the underlying tables have on the searches.

* Fix tests

---------

Co-authored-by: Miguel de la Cruz <miguel@ctrlz.es>
Co-authored-by: Mattermost Build <build@mattermost.com>
Этот коммит содержится в:
Miguel de la Cruz
2025-03-10 15:26:47 +01:00
коммит произвёл GitHub
родитель dd258f5ee5
Коммит c5e6d9f570
13 изменённых файлов: 256 добавлений и 161 удалений
Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

42
server/channels/app/custom_profile_attributes.go
Просмотреть файл

@@ -4,6 +4,7 @@
package app
import (
"database/sql"
"encoding/json"
"net/http"
"sort"
@@ -41,13 +42,14 @@ func (a *App) GetCPAField(fieldID string) (*model.PropertyField, *model.AppError
return nil, model.NewAppError("GetCPAField", "app.custom_profile_attributes.cpa_group_id.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
}
field, err := a.Srv().propertyService.GetPropertyField(fieldID)
field, err := a.Srv().propertyService.GetPropertyField(groupID, fieldID)
if err != nil {
return nil, model.NewAppError("GetCPAField", "app.custom_profile_attributes.get_property_field.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
}
if field.GroupID != groupID {
return nil, model.NewAppError("GetCPAField", "app.custom_profile_attributes.property_field_not_found.app_error", nil, "", http.StatusNotFound)
switch {
case errors.Is(err, sql.ErrNoRows):
return nil, model.NewAppError("GetCPAField", "app.custom_profile_attributes.property_field_not_found.app_error", nil, "", http.StatusNotFound).Wrap(err)
default:
return nil, model.NewAppError("GetCPAField", "app.custom_profile_attributes.get_property_field.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
}
}
return field, nil
@@ -64,7 +66,7 @@ func (a *App) ListCPAFields() ([]*model.PropertyField, *model.AppError) {
PerPage: CustomProfileAttributesFieldLimit,
}
fields, err := a.Srv().propertyService.SearchPropertyFields(opts)
fields, err := a.Srv().propertyService.SearchPropertyFields(groupID, "", opts)
if err != nil {
return nil, model.NewAppError("GetCPAFields", "app.custom_profile_attributes.search_property_fields.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
}
@@ -145,13 +147,13 @@ func (a *App) DeleteCPAField(id string) *model.AppError {
return model.NewAppError("DeleteCPAField", "app.custom_profile_attributes.cpa_group_id.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
}
existingField, err := a.Srv().propertyService.GetPropertyField(id)
if err != nil {
return model.NewAppError("DeleteCPAField", "app.custom_profile_attributes.get_property_field.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
}
if existingField.GroupID != groupID {
return model.NewAppError("DeleteCPAField", "app.custom_profile_attributes.property_field_not_found.app_error", nil, "", http.StatusNotFound)
if _, err := a.Srv().propertyService.GetPropertyField(groupID, id); err != nil {
switch {
case errors.Is(err, sql.ErrNoRows):
return model.NewAppError("DeleteCPAField", "app.custom_profile_attributes.property_field_not_found.app_error", nil, "", http.StatusNotFound)
default:
return model.NewAppError("DeleteCPAField", "app.custom_profile_attributes.get_property_field.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
}
}
if err := a.Srv().propertyService.DeletePropertyField(id); err != nil {
@@ -177,10 +179,8 @@ 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)
}
values, err := a.Srv().propertyService.SearchPropertyValues(model.PropertyValueSearchOpts{
GroupID: groupID,
TargetID: userID,
PerPage: CustomProfileAttributesFieldLimit,
values, err := a.Srv().propertyService.SearchPropertyValues(groupID, userID, model.PropertyValueSearchOpts{
PerPage: CustomProfileAttributesFieldLimit,
})
if err != nil {
return nil, model.NewAppError("ListCPAValues", "app.custom_profile_attributes.list_property_values.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
@@ -195,15 +195,11 @@ func (a *App) GetCPAValue(valueID string) (*model.PropertyValue, *model.AppError
return nil, model.NewAppError("GetCPAValue", "app.custom_profile_attributes.cpa_group_id.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
}
value, err := a.Srv().propertyService.GetPropertyValue(valueID)
value, err := a.Srv().propertyService.GetPropertyValue(groupID, valueID)
if err != nil {
return nil, model.NewAppError("GetCPAValue", "app.custom_profile_attributes.get_property_field.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
}
if value.GroupID != groupID {
return nil, model.NewAppError("GetCPAValue", "app.custom_profile_attributes.property_field_not_found.app_error", nil, "", http.StatusNotFound)
}
return value, nil
}

18
server/channels/app/custom_profile_attributes_test.go
Просмотреть файл

@@ -25,9 +25,9 @@ func TestGetCPAField(t *testing.T) {
require.NoError(t, cErr)
t.Run("should fail when getting a non-existent field", func(t *testing.T) {
field, err := th.App.GetCPAField(model.NewId())
require.NotNil(t, err)
require.Equal(t, "app.custom_profile_attributes.get_property_field.app_error", err.Id)
field, appErr := th.App.GetCPAField(model.NewId())
require.NotNil(t, appErr)
require.Equal(t, "app.custom_profile_attributes.property_field_not_found.app_error", appErr.Id)
require.Empty(t, field)
})
@@ -153,7 +153,7 @@ func TestCreateCPAField(t *testing.T) {
require.Equal(t, cpaGroupID, createdField.GroupID)
require.Equal(t, model.StringInterface{"visibility": "hidden"}, createdField.Attrs)
fetchedField, gErr := th.App.Srv().propertyService.GetPropertyField(createdField.ID)
fetchedField, gErr := th.App.Srv().propertyService.GetPropertyField("", createdField.ID)
require.NoError(t, gErr)
require.Equal(t, field.Name, fetchedField.Name)
require.NotZero(t, fetchedField.CreateAt)
@@ -304,7 +304,7 @@ func TestDeleteCPAField(t *testing.T) {
t.Run("should fail if the field doesn't exist", func(t *testing.T) {
err := th.App.DeleteCPAField(model.NewId())
require.NotNil(t, err)
require.Equal(t, "app.custom_profile_attributes.get_property_field.app_error", err.Id)
require.Equal(t, "app.custom_profile_attributes.property_field_not_found.app_error", err.Id)
})
t.Run("should not allow to delete a field outside of CPA", func(t *testing.T) {
@@ -324,7 +324,7 @@ func TestDeleteCPAField(t *testing.T) {
t.Run("should correctly delete the field", func(t *testing.T) {
// check that we have the associated values to the field prior deletion
opts := model.PropertyValueSearchOpts{PerPage: 10, FieldID: createdField.ID}
values, err := th.App.Srv().propertyService.SearchPropertyValues(opts)
values, err := th.App.Srv().propertyService.SearchPropertyValues(cpaGroupID, "", opts)
require.NoError(t, err)
require.Len(t, values, 3)
@@ -332,17 +332,17 @@ func TestDeleteCPAField(t *testing.T) {
require.Nil(t, th.App.DeleteCPAField(createdField.ID))
// check that it is marked as deleted
fetchedField, err := th.App.Srv().propertyService.GetPropertyField(createdField.ID)
fetchedField, err := th.App.Srv().propertyService.GetPropertyField("", createdField.ID)
require.NoError(t, err)
require.NotZero(t, fetchedField.DeleteAt)
// ensure that the associated fields have been marked as deleted too
values, err = th.App.Srv().propertyService.SearchPropertyValues(opts)
values, err = th.App.Srv().propertyService.SearchPropertyValues(cpaGroupID, "", opts)
require.NoError(t, err)
require.Len(t, values, 0)
opts.IncludeDeleted = true
values, err = th.App.Srv().propertyService.SearchPropertyValues(opts)
values, err = th.App.Srv().propertyService.SearchPropertyValues(cpaGroupID, "", opts)
require.NoError(t, err)
require.Len(t, values, 3)
for _, value := range values {

15
server/channels/app/properties/property_field.go
Просмотреть файл

@@ -11,19 +11,24 @@ func (ps *PropertyService) CreatePropertyField(field *model.PropertyField) (*mod
return ps.fieldStore.Create(field)
}
func (ps *PropertyService) GetPropertyField(id string) (*model.PropertyField, error) {
return ps.fieldStore.Get(id)
func (ps *PropertyService) GetPropertyField(groupID, id string) (*model.PropertyField, error) {
return ps.fieldStore.Get(groupID, id)
}
func (ps *PropertyService) GetPropertyFields(ids []string) ([]*model.PropertyField, error) {
return ps.fieldStore.GetMany(ids)
func (ps *PropertyService) GetPropertyFields(groupID string, ids []string) ([]*model.PropertyField, error) {
return ps.fieldStore.GetMany(groupID, 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) {
func (ps *PropertyService) SearchPropertyFields(groupID, targetID string, opts model.PropertyFieldSearchOpts) ([]*model.PropertyField, error) {
// groupID and targetID are part of the search method signature to
// incentivize the use of the database indexes in searches
opts.GroupID = groupID
opts.TargetID = targetID
return ps.fieldStore.SearchPropertyFields(opts)
}

14
server/channels/app/properties/property_value.go
Просмотреть файл

@@ -11,15 +11,19 @@ func (ps *PropertyService) CreatePropertyValue(value *model.PropertyValue) (*mod
return ps.valueStore.Create(value)
}
func (ps *PropertyService) GetPropertyValue(id string) (*model.PropertyValue, error) {
return ps.valueStore.Get(id)
func (ps *PropertyService) GetPropertyValue(groupID, id string) (*model.PropertyValue, error) {
return ps.valueStore.Get(groupID, id)
}
func (ps *PropertyService) GetPropertyValues(ids []string) ([]*model.PropertyValue, error) {
return ps.valueStore.GetMany(ids)
func (ps *PropertyService) GetPropertyValues(groupID string, ids []string) ([]*model.PropertyValue, error) {
return ps.valueStore.GetMany(groupID, ids)
}
func (ps *PropertyService) SearchPropertyValues(opts model.PropertyValueSearchOpts) ([]*model.PropertyValue, error) {
func (ps *PropertyService) SearchPropertyValues(groupID, targetID string, opts model.PropertyValueSearchOpts) ([]*model.PropertyValue, error) {
// groupID and targetID are part of the search method signature to
// incentivize the use of the database indexes in searches
opts.GroupID = groupID
opts.TargetID = targetID
return ps.valueStore.SearchPropertyValues(opts)
}