From 13bba1d1ce4592543b654036c7fb441e2249c199 Mon Sep 17 00:00:00 2001 From: jkoberg Date: Tue, 25 Jun 2024 16:20:20 +0200 Subject: [PATCH 1/2] fix(activitylog): multiple minor bugfixes Signed-off-by: jkoberg --- changelog/unreleased/activitylog-fixes.md | 5 ++++ services/activitylog/pkg/command/server.go | 1 + services/activitylog/pkg/service/http.go | 7 +++++- services/activitylog/pkg/service/response.go | 24 +++++++++++++++++++- services/activitylog/pkg/service/service.go | 10 ++++++++ 5 files changed, 45 insertions(+), 2 deletions(-) create mode 100644 changelog/unreleased/activitylog-fixes.md diff --git a/changelog/unreleased/activitylog-fixes.md b/changelog/unreleased/activitylog-fixes.md new file mode 100644 index 00000000000..c2e509d37bc --- /dev/null +++ b/changelog/unreleased/activitylog-fixes.md @@ -0,0 +1,5 @@ +Enhancement: Various fixes for the activitylog service + +First round of fixes to make the activitylog service more robust and reliable. + +https://github.com/owncloud/ocis/pull/9467 diff --git a/services/activitylog/pkg/command/server.go b/services/activitylog/pkg/command/server.go index 4ed99d10bc1..e6854065c8b 100644 --- a/services/activitylog/pkg/command/server.go +++ b/services/activitylog/pkg/command/server.go @@ -33,6 +33,7 @@ var _registeredEvents = []events.Unmarshaller{ events.FileTouched{}, events.ContainerCreated{}, events.ItemTrashed{}, + events.ItemPurged{}, events.ItemMoved{}, events.ShareCreated{}, events.ShareRemoved{}, diff --git a/services/activitylog/pkg/service/http.go b/services/activitylog/pkg/service/http.go index 1fbd2fb53d6..9c2388f9fe9 100644 --- a/services/activitylog/pkg/service/http.go +++ b/services/activitylog/pkg/service/http.go @@ -17,6 +17,7 @@ import ( "github.com/cs3org/reva/v2/pkg/utils" "google.golang.org/grpc/metadata" + libregraph "github.com/owncloud/libre-graph-api-go" "github.com/owncloud/ocis/v2/ocis-pkg/ast" "github.com/owncloud/ocis/v2/ocis-pkg/kql" "github.com/owncloud/ocis/v2/ocis-pkg/l10n" @@ -83,7 +84,7 @@ func (s *ActivitylogService) HandleGetItemActivities(w http.ResponseWriter, r *h return } - var resp GetActivitiesResponse + resp := GetActivitiesResponse{Activities: make([]libregraph.Activity, 0, len(evRes.GetEvents()))} for _, e := range evRes.GetEvents() { delete(toDelete, e.GetId()) @@ -269,6 +270,10 @@ func (s *ActivitylogService) getFilters(query string) (*provider.ResourceId, int if err != nil { return nil, limit, nil, nil, err } + if rid.GetOpaqueId() == "" { + // space root requested - fix format + rid.OpaqueId = rid.GetSpaceId() + } pref := func(a RawActivity) bool { for _, f := range prefilters { if !f(a) { diff --git a/services/activitylog/pkg/service/response.go b/services/activitylog/pkg/service/response.go index 9a9d710bc2c..4d60b822577 100644 --- a/services/activitylog/pkg/service/response.go +++ b/services/activitylog/pkg/service/response.go @@ -57,6 +57,9 @@ func WithResource(ref *provider.Reference, addSpace bool) ActivityOption { return func(ctx context.Context, gwc gateway.GatewayAPIClient, vars map[string]interface{}) error { info, err := utils.GetResource(ctx, ref, gwc) if err != nil { + vars["resource"] = Resource{ + Name: filepath.Base(ref.GetPath()), + } return err } @@ -90,6 +93,10 @@ func WithOldResource(ref *provider.Reference) ActivityOption { // WithTrashedResource sets the resource variable if the resource is trashed func WithTrashedResource(ref *provider.Reference, rid *provider.ResourceId) ActivityOption { return func(ctx context.Context, gwc gateway.GatewayAPIClient, vars map[string]interface{}) error { + vars["resource"] = Resource{ + Name: filepath.Base(ref.GetPath()), + } + resp, err := gwc.ListRecycle(ctx, &provider.ListRecycleRequest{ Ref: ref, }) @@ -122,6 +129,10 @@ func WithUser(uid *user.UserId, username string) ActivityOption { if username == "" { u, err := utils.GetUser(uid, gwc) if err != nil { + vars["user"] = Actor{ + ID: uid.GetOpaqueId(), + DisplayName: "DeletedUser", + } return err } username = u.GetUsername() @@ -143,6 +154,9 @@ func WithSharee(uid *user.UserId, gid *group.GroupId) ActivityOption { case uid != nil: u, err := utils.GetUser(uid, gwc) if err != nil { + vars["sharee"] = Actor{ + DisplayName: "DeletedUser", + } return err } @@ -151,6 +165,10 @@ func WithSharee(uid *user.UserId, gid *group.GroupId) ActivityOption { DisplayName: u.GetUsername(), } case gid != nil: + vars["sharee"] = Actor{ + ID: gid.GetOpaqueId(), + DisplayName: "DeletedGroup", + } r, err := gwc.GetGroup(ctx, &group.GetGroupRequest{GroupId: gid}) if err != nil { return fmt.Errorf("error getting group: %w", err) @@ -176,6 +194,10 @@ func WithSpace(spaceid *provider.StorageSpaceId) ActivityOption { return func(ctx context.Context, gwc gateway.GatewayAPIClient, vars map[string]interface{}) error { s, err := utils.GetSpace(ctx, spaceid.GetOpaqueId(), gwc) if err != nil { + vars["space"] = Resource{ + ID: spaceid.GetOpaqueId(), + Name: "DeletedSpace", + } return err } vars["space"] = Resource{ @@ -209,7 +231,7 @@ func (s *ActivitylogService) GetVars(ctx context.Context, opts ...ActivityOption vars := make(map[string]interface{}) for _, opt := range opts { if err := opt(ctx, gwc, vars); err != nil { - return nil, err + s.log.Info().Err(err).Msg("error getting activity vars") } } diff --git a/services/activitylog/pkg/service/service.go b/services/activitylog/pkg/service/service.go index 2653bd434ba..930156b67eb 100644 --- a/services/activitylog/pkg/service/service.go +++ b/services/activitylog/pkg/service/service.go @@ -104,6 +104,8 @@ func (a *ActivitylogService) Run() { err = a.AddActivity(ev.Ref, e.ID, utils.TSToTime(ev.Timestamp)) case events.ItemTrashed: err = a.AddActivityTrashed(ev.ID, ev.Ref, e.ID, utils.TSToTime(ev.Timestamp)) + case events.ItemPurged: + err = a.RemoveResource(ev.ID) case events.ItemMoved: err = a.AddActivity(ev.Ref, e.ID, utils.TSToTime(ev.Timestamp)) case events.ShareCreated: @@ -221,6 +223,14 @@ func (a *ActivitylogService) RemoveActivities(rid *provider.ResourceId, toDelete }) } +// RemoveResource removes the resource from the store +func (a *ActivitylogService) RemoveResource(rid *provider.ResourceId) error { + a.lock.Lock() + defer a.lock.Unlock() + + return a.store.Delete(storagespace.FormatResourceID(*rid)) +} + func (a *ActivitylogService) activities(rid *provider.ResourceId) ([]RawActivity, error) { resourceID := storagespace.FormatResourceID(*rid) From 17df46f9255d61814ac56a6e5f1d60561b803f82 Mon Sep 17 00:00:00 2001 From: jkoberg Date: Wed, 26 Jun 2024 16:29:04 +0200 Subject: [PATCH 2/2] feat(activitylog): allow sorting results Signed-off-by: jkoberg --- services/activitylog/pkg/service/http.go | 30 +++++++++++++++------ services/activitylog/pkg/service/service.go | 4 +++ 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/services/activitylog/pkg/service/http.go b/services/activitylog/pkg/service/http.go index 9c2388f9fe9..af203113eea 100644 --- a/services/activitylog/pkg/service/http.go +++ b/services/activitylog/pkg/service/http.go @@ -6,6 +6,7 @@ import ( "errors" "net/http" "path/filepath" + "slices" "strconv" "strings" "time" @@ -52,7 +53,7 @@ func (s *ActivitylogService) HandleGetItemActivities(w http.ResponseWriter, r *h return } - rid, limit, rawActivityAccepted, activityAccepted, err := s.getFilters(r.URL.Query().Get("kql")) + rid, limit, rawActivityAccepted, activityAccepted, sort, err := s.getFilters(r.URL.Query().Get("kql")) if err != nil { s.log.Info().Str("query", r.URL.Query().Get("kql")).Err(err).Msg("error getting filters") _, _ = w.Write([]byte(err.Error())) @@ -180,6 +181,8 @@ func (s *ActivitylogService) HandleGetItemActivities(w http.ResponseWriter, r *h }() } + sort(resp.Activities) + b, err := json.Marshal(resp) if err != nil { s.log.Error().Err(err).Msg("error marshalling activities") @@ -211,15 +214,17 @@ func (s *ActivitylogService) unwrapEvent(e *ehmsg.Event) interface{} { return einterface } -func (s *ActivitylogService) getFilters(query string) (*provider.ResourceId, int, func(RawActivity) bool, func(*ehmsg.Event) bool, error) { +func (s *ActivitylogService) getFilters(query string) (*provider.ResourceId, int, func(RawActivity) bool, func(*ehmsg.Event) bool, func([]libregraph.Activity), error) { qast, err := kql.Builder{}.Build(query) if err != nil { - return nil, 0, nil, nil, err + return nil, 0, nil, nil, nil, err } prefilters := make([]func(RawActivity) bool, 0) postfilters := make([]func(*ehmsg.Event) bool, 0) + sortby := func(_ []libregraph.Activity) {} + var ( itemID string limit int @@ -234,7 +239,7 @@ func (s *ActivitylogService) getFilters(query string) (*provider.ResourceId, int case "depth": depth, err := strconv.Atoi(v.Value) if err != nil { - return nil, limit, nil, nil, err + return nil, limit, nil, nil, sortby, err } prefilters = append(prefilters, func(a RawActivity) bool { @@ -243,10 +248,19 @@ func (s *ActivitylogService) getFilters(query string) (*provider.ResourceId, int case "limit": l, err := strconv.Atoi(v.Value) if err != nil { - return nil, limit, nil, nil, err + return nil, limit, nil, nil, sortby, err } limit = l + case "sort": + switch v.Value { + case "asc": + // nothing to do - already ascending + case "desc": + sortby = func(activities []libregraph.Activity) { + slices.Reverse(activities) + } + } } case *ast.DateTimeNode: switch v.Operator.Value { @@ -261,14 +275,14 @@ func (s *ActivitylogService) getFilters(query string) (*provider.ResourceId, int } case *ast.OperatorNode: if v.Value != "AND" { - return nil, limit, nil, nil, errors.New("only AND operator is supported") + return nil, limit, nil, nil, sortby, errors.New("only AND operator is supported") } } } rid, err := storagespace.ParseID(itemID) if err != nil { - return nil, limit, nil, nil, err + return nil, limit, nil, nil, sortby, err } if rid.GetOpaqueId() == "" { // space root requested - fix format @@ -290,7 +304,7 @@ func (s *ActivitylogService) getFilters(query string) (*provider.ResourceId, int } return true } - return &rid, limit, pref, postf, nil + return &rid, limit, pref, postf, sortby, nil } // returns true if this is just a rename diff --git a/services/activitylog/pkg/service/service.go b/services/activitylog/pkg/service/service.go index 930156b67eb..d1bacb28374 100644 --- a/services/activitylog/pkg/service/service.go +++ b/services/activitylog/pkg/service/service.go @@ -225,6 +225,10 @@ func (a *ActivitylogService) RemoveActivities(rid *provider.ResourceId, toDelete // RemoveResource removes the resource from the store func (a *ActivitylogService) RemoveResource(rid *provider.ResourceId) error { + if rid == nil { + return fmt.Errorf("resource id is required") + } + a.lock.Lock() defer a.lock.Unlock()