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.

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

feat: implement update experiment REST no command #1482

Merged
merged 6 commits into from
Jan 30, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
90 changes: 90 additions & 0 deletions api-description/web-api.swagger.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3494,6 +3494,54 @@ paths:
$ref: '#/definitions/experimentCreateExperimentRequest'
tags:
- experiment
patch:
summary: Update
description: Update an experiment.
operationId: web.v1.experiment.update
responses:
"200":
description: A successful response.
schema:
$ref: '#/definitions/experimentUpdateExperimentResponse'
"400":
description: Returned for bad requests that may have failed validation.
schema:
$ref: '#/definitions/googlerpcStatus'
examples:
application/json:
code: 3
details: []
message: invalid arguments error
"401":
description: Request could not be authenticated (authentication required).
schema:
$ref: '#/definitions/googlerpcStatus'
examples:
application/json:
code: 16
details: []
message: not authenticated
"503":
description: Returned for internal errors.
schema:
$ref: '#/definitions/googlerpcStatus'
examples:
application/json:
code: 13
details: []
message: internal
default:
description: An unexpected error response.
schema:
$ref: '#/definitions/googlerpcStatus'
parameters:
- name: body
in: body
required: true
schema:
$ref: '#/definitions/experimentUpdateExperimentRequest'
tags:
- experiment
/v1/experiments:
get:
summary: List
Expand Down Expand Up @@ -5193,6 +5241,11 @@ definitions:
properties:
role:
$ref: '#/definitions/AccountV2RoleOrganization'
UpdateExperimentRequestUpdatedStatus:
type: object
properties:
status:
$ref: '#/definitions/experimentExperimentStatus'
UpdateFeatureTargetingRequestFrom:
type: string
enum:
Expand Down Expand Up @@ -7091,8 +7144,45 @@ definitions:
type: object
experimentStopExperimentResponse:
type: object
experimentUpdateExperimentRequest:
type: object
properties:
id:
type: string
changeExperimentPeriodCommand:
$ref: '#/definitions/experimentChangeExperimentPeriodCommand'
description: deprecated
changeNameCommand:
$ref: '#/definitions/experimentChangeExperimentNameCommand'
description: deprecated
changeDescriptionCommand:
$ref: '#/definitions/experimentChangeExperimentDescriptionCommand'
description: deprecated
environmentId:
type: string
name:
type: string
description:
type: string
startAt:
type: string
format: int64
stopAt:
type: string
format: int64
archived:
type: boolean
description: if true, the experiment will be archived
status:
$ref: '#/definitions/UpdateExperimentRequestUpdatedStatus'
required:
- id
- environmentId
experimentUpdateExperimentResponse:
type: object
properties:
experiment:
$ref: '#/definitions/experimentExperiment'
experimentUpdateGoalRequest:
type: object
properties:
Expand Down
8 changes: 5 additions & 3 deletions manifests/bucketeer/charts/web/values.yaml

Large diffs are not rendered by default.

8 changes: 8 additions & 0 deletions pkg/domainevent/domain/message.go
Original file line number Diff line number Diff line change
Expand Up @@ -431,6 +431,14 @@ func LocalizedMessage(eventType proto.Event_Type, localizer locale.Localizer) *p
localizer.MustLocalizeWithTemplate(locale.Experiment),
),
}
case proto.Event_EXPERIMENT_UPDATED:
return &proto.LocalizedMessage{
Locale: localizer.GetLocale(),
Message: localizer.MustLocalizeWithTemplate(
locale.UpdatedTemplate,
localizer.MustLocalizeWithTemplate(locale.Goal),
),
}
case proto.Event_EXPERIMENT_DELETED:
return &proto.LocalizedMessage{
Locale: localizer.GetLocale(),
Expand Down
1 change: 1 addition & 0 deletions pkg/experiment/api/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ var (
statusInvalidGoalID = gstatus.New(codes.InvalidArgument, "experiment: invalid goal id")
statusGoalNameRequired = gstatus.New(codes.InvalidArgument, "experiment: goal name must be specified")
statusPeriodTooLong = gstatus.New(codes.InvalidArgument, "experiment: period too long")
statusPeriodInvalid = gstatus.New(codes.InvalidArgument, "experiment: period is invalid")
statusInvalidOrderBy = gstatus.New(codes.InvalidArgument, "expriment: order_by is invalid")
statusNotFound = gstatus.New(codes.NotFound, "experiment: not found")
statusGoalNotFound = gstatus.New(codes.NotFound, "experiment: goal not found")
Expand Down
154 changes: 153 additions & 1 deletion pkg/experiment/api/experiment.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (
"errors"
"strconv"

pb "github.com/golang/protobuf/proto"
"github.com/jinzhu/copier"
"go.uber.org/zap"
"google.golang.org/genproto/googleapis/rpc/errdetails"
Expand Down Expand Up @@ -655,6 +656,11 @@ func (s *experimentService) UpdateExperiment(
if err != nil {
return nil, err
}
if req.ChangeExperimentPeriodCommand == nil &&
req.ChangeNameCommand == nil &&
req.ChangeDescriptionCommand == nil {
return s.updateExperimentNoCommand(ctx, req, editor, localizer)
}
if err := validateUpdateExperimentRequest(req, localizer); err != nil {
return nil, err
}
Expand Down Expand Up @@ -730,7 +736,106 @@ func (s *experimentService) UpdateExperiment(
return experimentStorage.UpdateExperiment(ctx, experiment, req.EnvironmentId)
})
if err != nil {
if err == v2es.ErrExperimentNotFound || err == v2es.ErrExperimentUnexpectedAffectedRows {
if errors.Is(err, v2es.ErrExperimentNotFound) || errors.Is(err, v2es.ErrExperimentUnexpectedAffectedRows) {
dt, err := statusNotFound.WithDetails(&errdetails.LocalizedMessage{
Locale: localizer.GetLocale(),
Message: localizer.MustLocalize(locale.NotFoundError),
})
if err != nil {
return nil, statusInternal.Err()
}
return nil, dt.Err()
}
s.logger.Error(
"Failed to update experiment",
log.FieldsFromImcomingContext(ctx).AddFields(
zap.Error(err),
zap.String("environmentId", req.EnvironmentId),
)...,
)
dt, err := statusInternal.WithDetails(&errdetails.LocalizedMessage{
Locale: localizer.GetLocale(),
Message: localizer.MustLocalize(locale.InternalServerError),
})
if err != nil {
return nil, statusInternal.Err()
}
return nil, dt.Err()
}
return &proto.UpdateExperimentResponse{}, nil
}

func (s *experimentService) updateExperimentNoCommand(
ctx context.Context,
req *proto.UpdateExperimentRequest,
editor *eventproto.Editor,
localizer locale.Localizer,
) (*proto.UpdateExperimentResponse, error) {
err := validateUpdateExperimentNoCommandRequest(req, localizer)
if err != nil {
s.logger.Error(
"Failed validate update experiment no command req",
log.FieldsFromImcomingContext(ctx).AddFields(
zap.Error(err),
zap.String("environmentId", req.EnvironmentId),
)...,
)
return nil, err
}

err = s.mysqlClient.RunInTransactionV2(ctx, func(ctxWithTx context.Context, _ mysql.Transaction) error {
experimentStorage := v2es.NewExperimentStorage(s.mysqlClient)
experiment, err := experimentStorage.GetExperiment(ctxWithTx, req.Id, req.EnvironmentId)
if err != nil {
return err
}
updated, err := experiment.Update(
req.Name,
req.Description,
req.StartAt,
req.StopAt,
req.Status,
req.Archived,
)
if err != nil {
return err
}

var eventMsg pb.Message
if req.Archived != nil {
eventMsg = &eventproto.ExperimentArchivedEvent{
Id: req.Id,
}
} else {
eventMsg = &eventproto.ExperimentUpdatedEvent{
Id: experiment.Id,
Name: updated.Name,
Description: updated.Description,
StartAt: updated.StartAt,
StopAt: updated.StopAt,
Status: updated.Status,
}
}
event, err := domainevent.NewEvent(
editor,
eventproto.Event_EXPERIMENT,
experiment.Id,
eventproto.Event_EXPERIMENT_UPDATED,
eventMsg,
req.EnvironmentId,
updated,
experiment,
)
if err != nil {
return err
}
if err := s.publisher.Publish(ctxWithTx, event); err != nil {
return err
}
return experimentStorage.UpdateExperiment(ctxWithTx, updated, req.EnvironmentId)
})
if err != nil {
if errors.Is(err, v2es.ErrExperimentNotFound) || errors.Is(err, v2es.ErrExperimentUnexpectedAffectedRows) {
dt, err := statusNotFound.WithDetails(&errdetails.LocalizedMessage{
Locale: localizer.GetLocale(),
Message: localizer.MustLocalize(locale.NotFoundError),
Expand Down Expand Up @@ -782,6 +887,53 @@ func validateUpdateExperimentRequest(req *proto.UpdateExperimentRequest, localiz
return nil
}

func validateUpdateExperimentNoCommandRequest(
req *proto.UpdateExperimentRequest,
localizer locale.Localizer,
) error {
if req.Id == "" {
dt, err := statusExperimentIDRequired.WithDetails(&errdetails.LocalizedMessage{
Locale: localizer.GetLocale(),
Message: localizer.MustLocalizeWithTemplate(locale.RequiredFieldTemplate, "id"),
})
if err != nil {
return statusInternal.Err()
}
return dt.Err()
}
if req.Name != nil && req.Name.Value == "" {
dt, err := statusExperimentNameRequired.WithDetails(&errdetails.LocalizedMessage{
Locale: localizer.GetLocale(),
Message: localizer.MustLocalizeWithTemplate(locale.RequiredFieldTemplate, "name"),
})
if err != nil {
return statusInternal.Err()
}
return dt.Err()
}
if (req.StartAt != nil && req.StopAt == nil) ||
(req.StartAt == nil && req.StopAt != nil) {
dt, err := statusPeriodInvalid.WithDetails(&errdetails.LocalizedMessage{
Locale: localizer.GetLocale(),
Message: localizer.MustLocalizeWithTemplate(locale.InvalidArgumentError, "period"),
})
if err != nil {
return statusInternal.Err()
}
return dt.Err()
}
if req.StartAt != nil && req.StopAt != nil {
if err := validateExperimentPeriod(
req.StartAt.Value,
req.StopAt.Value,
localizer,
); err != nil {
return err
}
}
return nil
}

func (s *experimentService) StartExperiment(
ctx context.Context,
req *proto.StartExperimentRequest,
Expand Down
Loading
Loading