From 597e26ba89a92df982bc7babe45cd4bda2414663 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Fri, 31 Jul 2026 13:59:09 +0200 Subject: [PATCH 01/11] move event + http handler code to the events package MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../activitylog/pkg/server/http/server.go | 2 +- .../pkg/service/{ => event}/http.go | 0 .../pkg/service/{ => event}/l10n/.tx/config | 0 .../l10n/locale/ca/LC_MESSAGES/activitylog.po | 0 .../l10n/locale/de/LC_MESSAGES/activitylog.po | 0 .../l10n/locale/el/LC_MESSAGES/activitylog.po | 0 .../l10n/locale/es/LC_MESSAGES/activitylog.po | 0 .../l10n/locale/fi/LC_MESSAGES/activitylog.po | 0 .../l10n/locale/fr/LC_MESSAGES/activitylog.po | 0 .../l10n/locale/hu/LC_MESSAGES/activitylog.po | 0 .../l10n/locale/it/LC_MESSAGES/activitylog.po | 0 .../l10n/locale/ja/LC_MESSAGES/activitylog.po | 0 .../l10n/locale/ko/LC_MESSAGES/activitylog.po | 0 .../l10n/locale/lo/LC_MESSAGES/activitylog.po | 0 .../l10n/locale/nl/LC_MESSAGES/activitylog.po | 0 .../l10n/locale/pl/LC_MESSAGES/activitylog.po | 0 .../l10n/locale/pt/LC_MESSAGES/activitylog.po | 0 .../l10n/locale/ru/LC_MESSAGES/activitylog.po | 0 .../l10n/locale/sv/LC_MESSAGES/activitylog.po | 0 .../l10n/locale/vi/LC_MESSAGES/activitylog.po | 0 .../l10n/locale/zh/LC_MESSAGES/activitylog.po | 0 .../pkg/service/{ => event}/migrations.go | 0 .../pkg/service/{ => event}/options.go | 0 .../pkg/service/{ => event}/response.go | 0 .../pkg/service/{ => event}/service.go | 46 +++++-------------- .../service/{ => event}/service_suite_test.go | 0 .../pkg/service/{ => event}/service_test.go | 0 27 files changed, 13 insertions(+), 35 deletions(-) rename services/activitylog/pkg/service/{ => event}/http.go (100%) rename services/activitylog/pkg/service/{ => event}/l10n/.tx/config (100%) rename services/activitylog/pkg/service/{ => event}/l10n/locale/ca/LC_MESSAGES/activitylog.po (100%) rename services/activitylog/pkg/service/{ => event}/l10n/locale/de/LC_MESSAGES/activitylog.po (100%) rename services/activitylog/pkg/service/{ => event}/l10n/locale/el/LC_MESSAGES/activitylog.po (100%) rename services/activitylog/pkg/service/{ => event}/l10n/locale/es/LC_MESSAGES/activitylog.po (100%) rename services/activitylog/pkg/service/{ => event}/l10n/locale/fi/LC_MESSAGES/activitylog.po (100%) rename services/activitylog/pkg/service/{ => event}/l10n/locale/fr/LC_MESSAGES/activitylog.po (100%) rename services/activitylog/pkg/service/{ => event}/l10n/locale/hu/LC_MESSAGES/activitylog.po (100%) rename services/activitylog/pkg/service/{ => event}/l10n/locale/it/LC_MESSAGES/activitylog.po (100%) rename services/activitylog/pkg/service/{ => event}/l10n/locale/ja/LC_MESSAGES/activitylog.po (100%) rename services/activitylog/pkg/service/{ => event}/l10n/locale/ko/LC_MESSAGES/activitylog.po (100%) rename services/activitylog/pkg/service/{ => event}/l10n/locale/lo/LC_MESSAGES/activitylog.po (100%) rename services/activitylog/pkg/service/{ => event}/l10n/locale/nl/LC_MESSAGES/activitylog.po (100%) rename services/activitylog/pkg/service/{ => event}/l10n/locale/pl/LC_MESSAGES/activitylog.po (100%) rename services/activitylog/pkg/service/{ => event}/l10n/locale/pt/LC_MESSAGES/activitylog.po (100%) rename services/activitylog/pkg/service/{ => event}/l10n/locale/ru/LC_MESSAGES/activitylog.po (100%) rename services/activitylog/pkg/service/{ => event}/l10n/locale/sv/LC_MESSAGES/activitylog.po (100%) rename services/activitylog/pkg/service/{ => event}/l10n/locale/vi/LC_MESSAGES/activitylog.po (100%) rename services/activitylog/pkg/service/{ => event}/l10n/locale/zh/LC_MESSAGES/activitylog.po (100%) rename services/activitylog/pkg/service/{ => event}/migrations.go (100%) rename services/activitylog/pkg/service/{ => event}/options.go (100%) rename services/activitylog/pkg/service/{ => event}/response.go (100%) rename services/activitylog/pkg/service/{ => event}/service.go (94%) rename services/activitylog/pkg/service/{ => event}/service_suite_test.go (100%) rename services/activitylog/pkg/service/{ => event}/service_test.go (100%) diff --git a/services/activitylog/pkg/server/http/server.go b/services/activitylog/pkg/server/http/server.go index 735a33d011..a6200daf76 100644 --- a/services/activitylog/pkg/server/http/server.go +++ b/services/activitylog/pkg/server/http/server.go @@ -13,7 +13,7 @@ import ( "github.com/opencloud-eu/opencloud/pkg/service/http" "github.com/opencloud-eu/opencloud/pkg/tracing" "github.com/opencloud-eu/opencloud/pkg/version" - svc "github.com/opencloud-eu/opencloud/services/activitylog/pkg/service" + svc "github.com/opencloud-eu/opencloud/services/activitylog/pkg/service/event" "github.com/riandyrn/otelchi" "go-micro.dev/v4" ) diff --git a/services/activitylog/pkg/service/http.go b/services/activitylog/pkg/service/event/http.go similarity index 100% rename from services/activitylog/pkg/service/http.go rename to services/activitylog/pkg/service/event/http.go diff --git a/services/activitylog/pkg/service/l10n/.tx/config b/services/activitylog/pkg/service/event/l10n/.tx/config similarity index 100% rename from services/activitylog/pkg/service/l10n/.tx/config rename to services/activitylog/pkg/service/event/l10n/.tx/config diff --git a/services/activitylog/pkg/service/l10n/locale/ca/LC_MESSAGES/activitylog.po b/services/activitylog/pkg/service/event/l10n/locale/ca/LC_MESSAGES/activitylog.po similarity index 100% rename from services/activitylog/pkg/service/l10n/locale/ca/LC_MESSAGES/activitylog.po rename to services/activitylog/pkg/service/event/l10n/locale/ca/LC_MESSAGES/activitylog.po diff --git a/services/activitylog/pkg/service/l10n/locale/de/LC_MESSAGES/activitylog.po b/services/activitylog/pkg/service/event/l10n/locale/de/LC_MESSAGES/activitylog.po similarity index 100% rename from services/activitylog/pkg/service/l10n/locale/de/LC_MESSAGES/activitylog.po rename to services/activitylog/pkg/service/event/l10n/locale/de/LC_MESSAGES/activitylog.po diff --git a/services/activitylog/pkg/service/l10n/locale/el/LC_MESSAGES/activitylog.po b/services/activitylog/pkg/service/event/l10n/locale/el/LC_MESSAGES/activitylog.po similarity index 100% rename from services/activitylog/pkg/service/l10n/locale/el/LC_MESSAGES/activitylog.po rename to services/activitylog/pkg/service/event/l10n/locale/el/LC_MESSAGES/activitylog.po diff --git a/services/activitylog/pkg/service/l10n/locale/es/LC_MESSAGES/activitylog.po b/services/activitylog/pkg/service/event/l10n/locale/es/LC_MESSAGES/activitylog.po similarity index 100% rename from services/activitylog/pkg/service/l10n/locale/es/LC_MESSAGES/activitylog.po rename to services/activitylog/pkg/service/event/l10n/locale/es/LC_MESSAGES/activitylog.po diff --git a/services/activitylog/pkg/service/l10n/locale/fi/LC_MESSAGES/activitylog.po b/services/activitylog/pkg/service/event/l10n/locale/fi/LC_MESSAGES/activitylog.po similarity index 100% rename from services/activitylog/pkg/service/l10n/locale/fi/LC_MESSAGES/activitylog.po rename to services/activitylog/pkg/service/event/l10n/locale/fi/LC_MESSAGES/activitylog.po diff --git a/services/activitylog/pkg/service/l10n/locale/fr/LC_MESSAGES/activitylog.po b/services/activitylog/pkg/service/event/l10n/locale/fr/LC_MESSAGES/activitylog.po similarity index 100% rename from services/activitylog/pkg/service/l10n/locale/fr/LC_MESSAGES/activitylog.po rename to services/activitylog/pkg/service/event/l10n/locale/fr/LC_MESSAGES/activitylog.po diff --git a/services/activitylog/pkg/service/l10n/locale/hu/LC_MESSAGES/activitylog.po b/services/activitylog/pkg/service/event/l10n/locale/hu/LC_MESSAGES/activitylog.po similarity index 100% rename from services/activitylog/pkg/service/l10n/locale/hu/LC_MESSAGES/activitylog.po rename to services/activitylog/pkg/service/event/l10n/locale/hu/LC_MESSAGES/activitylog.po diff --git a/services/activitylog/pkg/service/l10n/locale/it/LC_MESSAGES/activitylog.po b/services/activitylog/pkg/service/event/l10n/locale/it/LC_MESSAGES/activitylog.po similarity index 100% rename from services/activitylog/pkg/service/l10n/locale/it/LC_MESSAGES/activitylog.po rename to services/activitylog/pkg/service/event/l10n/locale/it/LC_MESSAGES/activitylog.po diff --git a/services/activitylog/pkg/service/l10n/locale/ja/LC_MESSAGES/activitylog.po b/services/activitylog/pkg/service/event/l10n/locale/ja/LC_MESSAGES/activitylog.po similarity index 100% rename from services/activitylog/pkg/service/l10n/locale/ja/LC_MESSAGES/activitylog.po rename to services/activitylog/pkg/service/event/l10n/locale/ja/LC_MESSAGES/activitylog.po diff --git a/services/activitylog/pkg/service/l10n/locale/ko/LC_MESSAGES/activitylog.po b/services/activitylog/pkg/service/event/l10n/locale/ko/LC_MESSAGES/activitylog.po similarity index 100% rename from services/activitylog/pkg/service/l10n/locale/ko/LC_MESSAGES/activitylog.po rename to services/activitylog/pkg/service/event/l10n/locale/ko/LC_MESSAGES/activitylog.po diff --git a/services/activitylog/pkg/service/l10n/locale/lo/LC_MESSAGES/activitylog.po b/services/activitylog/pkg/service/event/l10n/locale/lo/LC_MESSAGES/activitylog.po similarity index 100% rename from services/activitylog/pkg/service/l10n/locale/lo/LC_MESSAGES/activitylog.po rename to services/activitylog/pkg/service/event/l10n/locale/lo/LC_MESSAGES/activitylog.po diff --git a/services/activitylog/pkg/service/l10n/locale/nl/LC_MESSAGES/activitylog.po b/services/activitylog/pkg/service/event/l10n/locale/nl/LC_MESSAGES/activitylog.po similarity index 100% rename from services/activitylog/pkg/service/l10n/locale/nl/LC_MESSAGES/activitylog.po rename to services/activitylog/pkg/service/event/l10n/locale/nl/LC_MESSAGES/activitylog.po diff --git a/services/activitylog/pkg/service/l10n/locale/pl/LC_MESSAGES/activitylog.po b/services/activitylog/pkg/service/event/l10n/locale/pl/LC_MESSAGES/activitylog.po similarity index 100% rename from services/activitylog/pkg/service/l10n/locale/pl/LC_MESSAGES/activitylog.po rename to services/activitylog/pkg/service/event/l10n/locale/pl/LC_MESSAGES/activitylog.po diff --git a/services/activitylog/pkg/service/l10n/locale/pt/LC_MESSAGES/activitylog.po b/services/activitylog/pkg/service/event/l10n/locale/pt/LC_MESSAGES/activitylog.po similarity index 100% rename from services/activitylog/pkg/service/l10n/locale/pt/LC_MESSAGES/activitylog.po rename to services/activitylog/pkg/service/event/l10n/locale/pt/LC_MESSAGES/activitylog.po diff --git a/services/activitylog/pkg/service/l10n/locale/ru/LC_MESSAGES/activitylog.po b/services/activitylog/pkg/service/event/l10n/locale/ru/LC_MESSAGES/activitylog.po similarity index 100% rename from services/activitylog/pkg/service/l10n/locale/ru/LC_MESSAGES/activitylog.po rename to services/activitylog/pkg/service/event/l10n/locale/ru/LC_MESSAGES/activitylog.po diff --git a/services/activitylog/pkg/service/l10n/locale/sv/LC_MESSAGES/activitylog.po b/services/activitylog/pkg/service/event/l10n/locale/sv/LC_MESSAGES/activitylog.po similarity index 100% rename from services/activitylog/pkg/service/l10n/locale/sv/LC_MESSAGES/activitylog.po rename to services/activitylog/pkg/service/event/l10n/locale/sv/LC_MESSAGES/activitylog.po diff --git a/services/activitylog/pkg/service/l10n/locale/vi/LC_MESSAGES/activitylog.po b/services/activitylog/pkg/service/event/l10n/locale/vi/LC_MESSAGES/activitylog.po similarity index 100% rename from services/activitylog/pkg/service/l10n/locale/vi/LC_MESSAGES/activitylog.po rename to services/activitylog/pkg/service/event/l10n/locale/vi/LC_MESSAGES/activitylog.po diff --git a/services/activitylog/pkg/service/l10n/locale/zh/LC_MESSAGES/activitylog.po b/services/activitylog/pkg/service/event/l10n/locale/zh/LC_MESSAGES/activitylog.po similarity index 100% rename from services/activitylog/pkg/service/l10n/locale/zh/LC_MESSAGES/activitylog.po rename to services/activitylog/pkg/service/event/l10n/locale/zh/LC_MESSAGES/activitylog.po diff --git a/services/activitylog/pkg/service/migrations.go b/services/activitylog/pkg/service/event/migrations.go similarity index 100% rename from services/activitylog/pkg/service/migrations.go rename to services/activitylog/pkg/service/event/migrations.go diff --git a/services/activitylog/pkg/service/options.go b/services/activitylog/pkg/service/event/options.go similarity index 100% rename from services/activitylog/pkg/service/options.go rename to services/activitylog/pkg/service/event/options.go diff --git a/services/activitylog/pkg/service/response.go b/services/activitylog/pkg/service/event/response.go similarity index 100% rename from services/activitylog/pkg/service/response.go rename to services/activitylog/pkg/service/event/response.go diff --git a/services/activitylog/pkg/service/service.go b/services/activitylog/pkg/service/event/service.go similarity index 94% rename from services/activitylog/pkg/service/service.go rename to services/activitylog/pkg/service/event/service.go index 9eec13962c..a97ebf09a1 100644 --- a/services/activitylog/pkg/service/service.go +++ b/services/activitylog/pkg/service/event/service.go @@ -7,7 +7,6 @@ import ( "encoding/json" "fmt" "path/filepath" - "reflect" "sort" "strconv" "strings" @@ -28,7 +27,6 @@ import ( "go.opentelemetry.io/otel/trace" "github.com/opencloud-eu/opencloud/pkg/log" - ehsvc "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/services/eventhistory/v0" settingssvc "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/services/settings/v0" "github.com/opencloud-eu/opencloud/services/activitylog/pkg/config" ) @@ -50,7 +48,6 @@ type ActivitylogService struct { events <-chan events.Event gws pool.Selectable[gateway.GatewayAPIClient] mux *chi.Mux - evHistory ehsvc.EventHistoryService valService settingssvc.ValueService lock sync.RWMutex tp trace.TracerProvider @@ -60,8 +57,6 @@ type ActivitylogService struct { natskv nats.KeyValue maxActivities int - - registeredEvents map[string]events.Unmarshaller } type Debouncer struct { @@ -148,17 +143,8 @@ func New(opts ...Option) (*ActivitylogService, error) { opt(o) } - if o.Stream == nil { - return nil, errors.New("stream is required") - } - - ch, err := events.Consume(o.Stream, o.Config.Service.Name, o.RegisteredEvents...) - if err != nil { - return nil, err - } - cache := ttlcache.NewCache() - err = cache.SetTTL(30 * time.Second) + err := cache.SetTTL(30 * time.Second) if err != nil { return nil, err } @@ -207,20 +193,17 @@ func New(opts ...Option) (*ActivitylogService, error) { } s := &ActivitylogService{ - log: o.Logger, - cfg: o.Config, - events: ch, - gws: o.GatewaySelector, - mux: o.Mux, - evHistory: o.HistoryClient, - valService: o.ValueClient, - lock: sync.RWMutex{}, - registeredEvents: make(map[string]events.Unmarshaller), - tp: o.TraceProvider, - tracer: o.TraceProvider.Tracer("github.com/opencloud-eu/opencloud/services/activitylog/pkg/service"), - parentIdCache: cache, - maxActivities: o.Config.MaxActivities, - natskv: kv, + log: o.Logger, + cfg: o.Config, + gws: o.GatewaySelector, + mux: o.Mux, + valService: o.ValueClient, + lock: sync.RWMutex{}, + tp: o.TraceProvider, + tracer: o.TraceProvider.Tracer("github.com/opencloud-eu/opencloud/services/activitylog/pkg/service"), + parentIdCache: cache, + maxActivities: o.Config.MaxActivities, + natskv: kv, } s.debouncer = NewDebouncer(o.Config.WriteBufferDuration, s.storeActivity) @@ -232,11 +215,6 @@ func New(opts ...Option) (*ActivitylogService, error) { s.mux.Get("/graph/v1beta1/extensions/org.libregraph/activities", s.HandleGetItemActivities) - for _, e := range o.RegisteredEvents { - typ := reflect.TypeOf(e) - s.registeredEvents[typ.String()] = e - } - go s.Run() return s, nil diff --git a/services/activitylog/pkg/service/service_suite_test.go b/services/activitylog/pkg/service/event/service_suite_test.go similarity index 100% rename from services/activitylog/pkg/service/service_suite_test.go rename to services/activitylog/pkg/service/event/service_suite_test.go diff --git a/services/activitylog/pkg/service/service_test.go b/services/activitylog/pkg/service/event/service_test.go similarity index 100% rename from services/activitylog/pkg/service/service_test.go rename to services/activitylog/pkg/service/event/service_test.go From ee31636d966b75082e6bd6bc6019df7e5298997c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Tue, 4 Aug 2026 18:59:26 +0200 Subject: [PATCH 02/11] split handlers, fix bugs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- services/activitylog/pkg/apierrors/errors.go | 12 + services/activitylog/pkg/command/server.go | 171 ++++- services/activitylog/pkg/config/config.go | 19 +- .../pkg/config/defaults/defaultconfig.go | 12 +- services/activitylog/pkg/data/rawactivity.go | 10 + .../event => server/http}/l10n/.tx/config | 0 .../l10n/locale/ca/LC_MESSAGES/activitylog.po | 0 .../l10n/locale/de/LC_MESSAGES/activitylog.po | 0 .../l10n/locale/el/LC_MESSAGES/activitylog.po | 0 .../l10n/locale/es/LC_MESSAGES/activitylog.po | 0 .../l10n/locale/fi/LC_MESSAGES/activitylog.po | 0 .../l10n/locale/fr/LC_MESSAGES/activitylog.po | 0 .../l10n/locale/hu/LC_MESSAGES/activitylog.po | 0 .../l10n/locale/it/LC_MESSAGES/activitylog.po | 0 .../l10n/locale/ja/LC_MESSAGES/activitylog.po | 0 .../l10n/locale/ko/LC_MESSAGES/activitylog.po | 0 .../l10n/locale/lo/LC_MESSAGES/activitylog.po | 0 .../l10n/locale/nl/LC_MESSAGES/activitylog.po | 0 .../l10n/locale/pl/LC_MESSAGES/activitylog.po | 0 .../l10n/locale/pt/LC_MESSAGES/activitylog.po | 0 .../l10n/locale/ru/LC_MESSAGES/activitylog.po | 0 .../l10n/locale/sv/LC_MESSAGES/activitylog.po | 0 .../l10n/locale/vi/LC_MESSAGES/activitylog.po | 0 .../l10n/locale/zh/LC_MESSAGES/activitylog.po | 0 .../activitylog/pkg/server/http/option.go | 94 +-- .../activitylog/pkg/server/http/server.go | 143 ++-- .../pkg/service/activitylog/activitylog.go | 353 ++++++++++ .../pkg/service/activitylog/debouncer.go | 77 +++ .../{event => activitylog}/migrations.go | 9 +- .../pkg/service/activitylog/options.go | 35 + .../service_suite_test.go | 2 +- .../{event => activitylog}/service_test.go | 77 +-- .../activitylog/pkg/service/event/service.go | 652 ------------------ .../pkg/service/events/debouncer.go | 77 +++ .../pkg/service/{event => events}/options.go | 52 +- .../activitylog/pkg/service/events/service.go | 312 +++++++++ .../activitylog/pkg/service/http/option.go | 59 ++ .../pkg/service/{event => http}/response.go | 15 +- .../{event/http.go => http/service.go} | 144 ++-- services/webfinger/pkg/server/http/server.go | 2 +- 40 files changed, 1354 insertions(+), 973 deletions(-) create mode 100644 services/activitylog/pkg/apierrors/errors.go create mode 100644 services/activitylog/pkg/data/rawactivity.go rename services/activitylog/pkg/{service/event => server/http}/l10n/.tx/config (100%) rename services/activitylog/pkg/{service/event => server/http}/l10n/locale/ca/LC_MESSAGES/activitylog.po (100%) rename services/activitylog/pkg/{service/event => server/http}/l10n/locale/de/LC_MESSAGES/activitylog.po (100%) rename services/activitylog/pkg/{service/event => server/http}/l10n/locale/el/LC_MESSAGES/activitylog.po (100%) rename services/activitylog/pkg/{service/event => server/http}/l10n/locale/es/LC_MESSAGES/activitylog.po (100%) rename services/activitylog/pkg/{service/event => server/http}/l10n/locale/fi/LC_MESSAGES/activitylog.po (100%) rename services/activitylog/pkg/{service/event => server/http}/l10n/locale/fr/LC_MESSAGES/activitylog.po (100%) rename services/activitylog/pkg/{service/event => server/http}/l10n/locale/hu/LC_MESSAGES/activitylog.po (100%) rename services/activitylog/pkg/{service/event => server/http}/l10n/locale/it/LC_MESSAGES/activitylog.po (100%) rename services/activitylog/pkg/{service/event => server/http}/l10n/locale/ja/LC_MESSAGES/activitylog.po (100%) rename services/activitylog/pkg/{service/event => server/http}/l10n/locale/ko/LC_MESSAGES/activitylog.po (100%) rename services/activitylog/pkg/{service/event => server/http}/l10n/locale/lo/LC_MESSAGES/activitylog.po (100%) rename services/activitylog/pkg/{service/event => server/http}/l10n/locale/nl/LC_MESSAGES/activitylog.po (100%) rename services/activitylog/pkg/{service/event => server/http}/l10n/locale/pl/LC_MESSAGES/activitylog.po (100%) rename services/activitylog/pkg/{service/event => server/http}/l10n/locale/pt/LC_MESSAGES/activitylog.po (100%) rename services/activitylog/pkg/{service/event => server/http}/l10n/locale/ru/LC_MESSAGES/activitylog.po (100%) rename services/activitylog/pkg/{service/event => server/http}/l10n/locale/sv/LC_MESSAGES/activitylog.po (100%) rename services/activitylog/pkg/{service/event => server/http}/l10n/locale/vi/LC_MESSAGES/activitylog.po (100%) rename services/activitylog/pkg/{service/event => server/http}/l10n/locale/zh/LC_MESSAGES/activitylog.po (100%) create mode 100644 services/activitylog/pkg/service/activitylog/activitylog.go create mode 100644 services/activitylog/pkg/service/activitylog/debouncer.go rename services/activitylog/pkg/service/{event => activitylog}/migrations.go (93%) create mode 100644 services/activitylog/pkg/service/activitylog/options.go rename services/activitylog/pkg/service/{event => activitylog}/service_suite_test.go (87%) rename services/activitylog/pkg/service/{event => activitylog}/service_test.go (81%) delete mode 100644 services/activitylog/pkg/service/event/service.go create mode 100644 services/activitylog/pkg/service/events/debouncer.go rename services/activitylog/pkg/service/{event => events}/options.go (55%) create mode 100644 services/activitylog/pkg/service/events/service.go create mode 100644 services/activitylog/pkg/service/http/option.go rename services/activitylog/pkg/service/{event => http}/response.go (96%) rename services/activitylog/pkg/service/{event/http.go => http/service.go} (74%) diff --git a/services/activitylog/pkg/apierrors/errors.go b/services/activitylog/pkg/apierrors/errors.go new file mode 100644 index 0000000000..25eaf42031 --- /dev/null +++ b/services/activitylog/pkg/apierrors/errors.go @@ -0,0 +1,12 @@ +// apierrors package defines common business API errors that can be used across the service. It is intended to be used by both the service and the API layer to ensure consistent error handling and messaging. +package apierrors + +import "errors" + +var ( + ErrNotFound = errors.New("query target not found") + ErrBadRequest = errors.New("bad request") + ErrUnauthorized = errors.New("unauthorized") + ErrForbidden = errors.New("forbidden") + ErrMissingEmail = errors.New("missing email address") +) diff --git a/services/activitylog/pkg/command/server.go b/services/activitylog/pkg/command/server.go index 59b7cef3a8..af4c74eec5 100644 --- a/services/activitylog/pkg/command/server.go +++ b/services/activitylog/pkg/command/server.go @@ -2,18 +2,18 @@ package command import ( "context" + "crypto/tls" "fmt" - "github.com/opencloud-eu/opencloud/pkg/log" - "github.com/opencloud-eu/opencloud/pkg/runner" - "github.com/opencloud-eu/reva/v2/pkg/events" - "github.com/opencloud-eu/reva/v2/pkg/events/stream" - "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" + "github.com/nats-io/nats.go" + "github.com/olekukonko/errors" "github.com/spf13/cobra" "github.com/opencloud-eu/opencloud/pkg/config/configlog" "github.com/opencloud-eu/opencloud/pkg/generators" + "github.com/opencloud-eu/opencloud/pkg/log" "github.com/opencloud-eu/opencloud/pkg/registry" + "github.com/opencloud-eu/opencloud/pkg/runner" ogrpc "github.com/opencloud-eu/opencloud/pkg/service/grpc" "github.com/opencloud-eu/opencloud/pkg/tracing" "github.com/opencloud-eu/opencloud/pkg/version" @@ -24,6 +24,12 @@ import ( "github.com/opencloud-eu/opencloud/services/activitylog/pkg/metrics" "github.com/opencloud-eu/opencloud/services/activitylog/pkg/server/debug" "github.com/opencloud-eu/opencloud/services/activitylog/pkg/server/http" + "github.com/opencloud-eu/opencloud/services/activitylog/pkg/service/activitylog" + svcEvents "github.com/opencloud-eu/opencloud/services/activitylog/pkg/service/events" + svcHttp "github.com/opencloud-eu/opencloud/services/activitylog/pkg/service/http" + "github.com/opencloud-eu/reva/v2/pkg/events" + "github.com/opencloud-eu/reva/v2/pkg/events/raw" + "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" ) var _registeredEvents = []events.Unmarshaller{ @@ -62,19 +68,11 @@ func Server(cfg *config.Config) *cobra.Command { gr := runner.NewGroup() ctx, cancel := context.WithCancel(cmd.Context()) + defer cancel() mtrcs := metrics.New() mtrcs.BuildInfo.WithLabelValues(version.GetString()).Set(1) - defer cancel() - - connName := generators.GenerateConnectionName(cfg.Service.Name, generators.NTypeBus) - evStream, err := stream.NatsFromConfig(connName, false, stream.NatsConfig(cfg.Events)) - if err != nil { - logger.Error().Err(err).Msg("Failed to initialize event stream") - return err - } - tm, err := pool.StringToTLSMode(cfg.GRPCClientTLS.Mode) if err != nil { logger.Error().Err(err).Msg("Failed to parse tls mode") @@ -99,28 +97,103 @@ func Server(cfg *config.Config) *cobra.Command { return err } - hClient := ehsvc.NewEventHistoryService("eu.opencloud.api.eventhistory", grpcClient) - vClient := settingssvc.NewValueService("eu.opencloud.api.settings", grpcClient) + kv, err := ConnectNatsKV(cfg.Store) + if err != nil { + return err + } + activityLog, err := activitylog.New(kv, + activitylog.Logger(logger), + activitylog.MaxActivities(cfg.MaxActivities), + activitylog.WriteBufferDuration(cfg.WriteBufferDuration), + ) + if err != nil { + logger.Error().Err(err).Msg("Failed to initialize activity log") + return err + } + + if !cfg.HTTP.Disabled { - { - svc, err := http.Server( + hClient := ehsvc.NewEventHistoryService("eu.opencloud.api.eventhistory", grpcClient) + + svc, err := svcHttp.New( + activityLog, + svcHttp.Logger(logger), + svcHttp.GatewaySelector(gatewaySelector), + svcHttp.RegisteredEvents(_registeredEvents), + //svcHttp.TraceProvider(tracerProvider), + svcHttp.HistoryClient(hClient), + ) + if err != nil { + logger.Error().Err(err).Msg("handler init") + return err + } + // TODO svc = service.NewInstrument(svc, metrics) + // TODO svc = service.NewLogging(svc, logger) // this logs service specific data + // TODO svc = service.NewTracing(svc, traceProvider) + vClient := settingssvc.NewValueService("eu.opencloud.api.settings", grpcClient) + + server, err := http.Server( + http.ValueClient(vClient), http.Logger(logger), + http.Context(ctx), http.Config(cfg), - http.Context(ctx), // NOTE: not passing this "option" leads to a panic in go-micro - http.TraceProvider(tracerProvider), - http.Stream(evStream), - http.GatewaySelector(gatewaySelector), - http.HistoryClient(hClient), - http.ValueClient(vClient), - http.RegisteredEvents(_registeredEvents), + http.Service(svc), ) + if err != nil { + logger.Info(). + Err(err). + Str("transport", "http"). + Msg("Failed to initialize server") + + return err + } + + gr.Add(runner.NewGoMicroHttpServerRunner(cfg.Service.Name+".http", server)) + } else { + logger.Info().Msg("HTTP server disabled, not starting HTTP service") + } + + if !cfg.Events.Disabled { + + connName := generators.GenerateConnectionName(cfg.Service.Name, generators.NTypeBus) + evStream, err := raw.FromConfig(ctx, connName, raw.Config{ + Endpoint: cfg.Events.Endpoint, + Cluster: cfg.Events.Cluster, + EnableTLS: cfg.Events.EnableTLS, + TLSInsecure: cfg.Events.TLSInsecure, + TLSRootCACertificate: cfg.Events.TLSRootCACertificate, + AuthUsername: cfg.Events.AuthUsername, + AuthPassword: cfg.Events.AuthPassword, + MaxAckPending: cfg.Events.MaxAckPending, + AckWait: cfg.Events.AckWait, + }) + if err != nil { + logger.Error().Err(err).Msg("Failed to initialize event stream") + return err + } + eventSvc, err := svcEvents.New( + activityLog, + evStream, + svcEvents.Context(ctx), + svcEvents.Logger(logger), + svcEvents.ServiceAccount(cfg.ServiceAccount), + svcEvents.GatewaySelector(gatewaySelector), + svcEvents.RegisteredEvents(_registeredEvents), + svcEvents.NumConsumers(cfg.NumConsumers), + ) if err != nil { - logger.Error().Err(err).Str("transport", "http").Msg("Failed to initialize server") + logger.Error().Err(err).Str("transport", "event").Msg("Failed to initialize server") return err } - gr.Add(runner.NewGoMicroHttpServerRunner(cfg.Service.Name+".http", svc)) + gr.Add(runner.New(cfg.Service.Name+".svc", func() error { + return eventSvc.Run() + }, func() { + eventSvc.Close() + })) + } else { + logger.Info().Msg("event listening disabled, not starting event service") } { @@ -149,3 +222,47 @@ func Server(cfg *config.Config) *cobra.Command { }, } } + +func ConnectNatsKV(cfg config.Store) (nats.KeyValue, error) { + // Connect to NATS servers + natsOptions := nats.Options{ + Servers: cfg.Nodes, + } + if cfg.EnableTLS { + if cfg.TLSRootCACertificate != "" { + // when root ca is configured use it. an insecure flag is ignored. + nats.RootCAs(cfg.TLSRootCACertificate)(&natsOptions) + } else { + // enable tls and use insecure flag + nats.Secure(&tls.Config{MinVersion: tls.VersionTLS12, InsecureSkipVerify: cfg.TLSInsecure})(&natsOptions) + } + } + if cfg.AuthUsername != "" && cfg.AuthPassword != "" { + nats.UserInfo(cfg.AuthUsername, cfg.AuthPassword)(&natsOptions) + } + conn, err := natsOptions.Connect() + if err != nil { + return nil, err + } + + js, err := conn.JetStream() + if err != nil { + return nil, err + } + + kv, err := js.KeyValue(cfg.Database) + if err != nil { + if !errors.Is(err, nats.ErrBucketNotFound) { + return nil, errors.Wrapf(err, "Failed to get bucket (%s)", cfg.Database) + } + + kv, err = js.CreateKeyValue(&nats.KeyValueConfig{ + Bucket: cfg.Database, + }) + if err != nil { + return nil, errors.Wrapf(err, "Failed to create bucket (%s)", cfg.Database) + } + } + + return kv, nil +} diff --git a/services/activitylog/pkg/config/config.go b/services/activitylog/pkg/config/config.go index 4c0417ee0b..a3d94973f4 100644 --- a/services/activitylog/pkg/config/config.go +++ b/services/activitylog/pkg/config/config.go @@ -35,17 +35,21 @@ type Config struct { WriteBufferDuration time.Duration `yaml:"write_buffer_duration" env:"ACTIVITYLOG_WRITE_BUFFER_DURATION" desc:"The duration to wait before flushing the write buffer. This is used to reduce the number of writes to the store." introductionVersion:"4.0.0"` MaxActivities int `yaml:"max_activities" env:"ACTIVITYLOG_MAX_ACTIVITIES" desc:"The maximum number of activities to keep in the store per resource. If the number of activities exceeds this value, the oldest activities will be removed." introductionVersion:"4.0.0"` + NumConsumers int `yaml:"num_consumers" env:"ACTIVITYLOG_NUM_CONSUMERS" desc:"The amount of concurrent event consumers to start. Event consumers are used for updating the list of activities. Multiple consumers increase parallelisation, but will also increase CPU and memory demands." introductionVersion:"%NEXT%"` } // Events combines the configuration options for the event bus. type Events struct { - Endpoint string `yaml:"endpoint" env:"OC_EVENTS_ENDPOINT" desc:"The address of the event system. The event system is the message queuing service. It is used as message broker for the microservice architecture." introductionVersion:"1.0.0"` - Cluster string `yaml:"cluster" env:"OC_EVENTS_CLUSTER" desc:"The clusterID of the event system. The event system is the message queuing service. It is used as message broker for the microservice architecture. Mandatory when using NATS as event system." introductionVersion:"1.0.0"` - TLSInsecure bool `yaml:"tls_insecure" env:"OC_INSECURE;OC_EVENTS_TLS_INSECURE" desc:"Whether to verify the server TLS certificates." introductionVersion:"1.0.0"` - TLSRootCACertificate string `yaml:"tls_root_ca_certificate" env:"OC_EVENTS_TLS_ROOT_CA_CERTIFICATE" desc:"The root CA certificate used to validate the server's TLS certificate. If provided NOTIFICATIONS_EVENTS_TLS_INSECURE will be seen as false." introductionVersion:"1.0.0"` - EnableTLS bool `yaml:"enable_tls" env:"OC_EVENTS_ENABLE_TLS" desc:"Enable TLS for the connection to the events broker. The events broker is the OpenCloud service which receives and delivers events between the services." introductionVersion:"1.0.0"` - AuthUsername string `yaml:"username" env:"OC_EVENTS_AUTH_USERNAME" desc:"The username to authenticate with the events broker. The events broker is the OpenCloud service which receives and delivers events between the services." introductionVersion:"1.0.0"` - AuthPassword string `yaml:"password" env:"OC_EVENTS_AUTH_PASSWORD" desc:"The password to authenticate with the events broker. The events broker is the OpenCloud service which receives and delivers events between the services." introductionVersion:"1.0.0"` + Disabled bool `yaml:"disabled" env:"ACTIVITYLOG_EVENTS_DISABLED" desc:"Disables listening for events. Set this to true if the service should only handle HTTP requests." introductionVersion:"%NEXT%"` + Endpoint string `yaml:"endpoint" env:"OC_EVENTS_ENDPOINT" desc:"The address of the event system. The event system is the message queuing service. It is used as message broker for the microservice architecture." introductionVersion:"1.0.0"` + Cluster string `yaml:"cluster" env:"OC_EVENTS_CLUSTER" desc:"The clusterID of the event system. The event system is the message queuing service. It is used as message broker for the microservice architecture. Mandatory when using NATS as event system." introductionVersion:"1.0.0"` + TLSInsecure bool `yaml:"tls_insecure" env:"OC_INSECURE;OC_EVENTS_TLS_INSECURE" desc:"Whether to verify the server TLS certificates." introductionVersion:"1.0.0"` + TLSRootCACertificate string `yaml:"tls_root_ca_certificate" env:"OC_EVENTS_TLS_ROOT_CA_CERTIFICATE" desc:"The root CA certificate used to validate the server's TLS certificate. If provided NOTIFICATIONS_EVENTS_TLS_INSECURE will be seen as false." introductionVersion:"1.0.0"` + EnableTLS bool `yaml:"enable_tls" env:"OC_EVENTS_ENABLE_TLS" desc:"Enable TLS for the connection to the events broker. The events broker is the OpenCloud service which receives and delivers events between the services." introductionVersion:"1.0.0"` + AuthUsername string `yaml:"username" env:"OC_EVENTS_AUTH_USERNAME" desc:"The username to authenticate with the events broker. The events broker is the OpenCloud service which receives and delivers events between the services." introductionVersion:"1.0.0"` + AuthPassword string `yaml:"password" env:"OC_EVENTS_AUTH_PASSWORD" desc:"The password to authenticate with the events broker. The events broker is the OpenCloud service which receives and delivers events between the services." introductionVersion:"1.0.0"` + MaxAckPending int `yaml:"max_ack_pending" env:"ACTIVITYLOG_EVENTS_MAX_ACK_PENDING" desc:"The maximum number of unacknowledged messages. This is used to limit the number of messages that can be in flight at the same time." introductionVersion:"%NEXT%"` + AckWait time.Duration `yaml:"ack_wait" env:"ACTIVITYLOG_EVENTS_ACK_WAIT" desc:"The time to wait for an ack before the message is redelivered. This is used to ensure that messages are not lost if the consumer crashes." introductionVersion:"%NEXT%"` } // Store configures the store to use @@ -77,6 +81,7 @@ type CORS struct { // HTTP defines the available http configuration. type HTTP struct { + Disabled bool `yaml:"disabled" env:"ACTIVITYLOG_HTTP_DISABLED" desc:"Disables the HTTP service. Set this to true if the service should only handle events." introductionVersion:"1.0.0"` Addr string `yaml:"addr" env:"ACTIVITYLOG_HTTP_ADDR" desc:"The bind address of the HTTP service." introductionVersion:"1.0.0"` Namespace string `yaml:"-"` Root string `yaml:"root" env:"ACTIVITYLOG_HTTP_ROOT" desc:"Subdirectory that serves as the root for this HTTP service." introductionVersion:"1.0.0"` diff --git a/services/activitylog/pkg/config/defaults/defaultconfig.go b/services/activitylog/pkg/config/defaults/defaultconfig.go index 7b6e8306d7..7c9c9073fc 100644 --- a/services/activitylog/pkg/config/defaults/defaultconfig.go +++ b/services/activitylog/pkg/config/defaults/defaultconfig.go @@ -29,9 +29,11 @@ func DefaultConfig() *config.Config { Name: "activitylog", }, Events: config.Events{ - Endpoint: "127.0.0.1:9233", - Cluster: "opencloud-cluster", - EnableTLS: false, + Endpoint: "127.0.0.1:9233", + Cluster: "opencloud-cluster", + EnableTLS: false, + MaxAckPending: 1000, + AckWait: 1 * time.Minute, }, Store: config.Store{ Store: "nats-js-kv", @@ -52,7 +54,9 @@ func DefaultConfig() *config.Config { }, }, WriteBufferDuration: 10 * time.Second, - MaxActivities: 6000, + // Nats runs into max payload exceeded errors at around 7k activities. Let's keep a buffer. + MaxActivities: 6000, + NumConsumers: 1, } } diff --git a/services/activitylog/pkg/data/rawactivity.go b/services/activitylog/pkg/data/rawactivity.go new file mode 100644 index 0000000000..ac77d539ed --- /dev/null +++ b/services/activitylog/pkg/data/rawactivity.go @@ -0,0 +1,10 @@ +package data + +import "time" + +// RawActivity represents an activity as it is stored in the activitylog store +type RawActivity struct { + EventID string `json:"event_id"` + Depth int `json:"depth"` + Timestamp time.Time `json:"timestamp"` +} diff --git a/services/activitylog/pkg/service/event/l10n/.tx/config b/services/activitylog/pkg/server/http/l10n/.tx/config similarity index 100% rename from services/activitylog/pkg/service/event/l10n/.tx/config rename to services/activitylog/pkg/server/http/l10n/.tx/config diff --git a/services/activitylog/pkg/service/event/l10n/locale/ca/LC_MESSAGES/activitylog.po b/services/activitylog/pkg/server/http/l10n/locale/ca/LC_MESSAGES/activitylog.po similarity index 100% rename from services/activitylog/pkg/service/event/l10n/locale/ca/LC_MESSAGES/activitylog.po rename to services/activitylog/pkg/server/http/l10n/locale/ca/LC_MESSAGES/activitylog.po diff --git a/services/activitylog/pkg/service/event/l10n/locale/de/LC_MESSAGES/activitylog.po b/services/activitylog/pkg/server/http/l10n/locale/de/LC_MESSAGES/activitylog.po similarity index 100% rename from services/activitylog/pkg/service/event/l10n/locale/de/LC_MESSAGES/activitylog.po rename to services/activitylog/pkg/server/http/l10n/locale/de/LC_MESSAGES/activitylog.po diff --git a/services/activitylog/pkg/service/event/l10n/locale/el/LC_MESSAGES/activitylog.po b/services/activitylog/pkg/server/http/l10n/locale/el/LC_MESSAGES/activitylog.po similarity index 100% rename from services/activitylog/pkg/service/event/l10n/locale/el/LC_MESSAGES/activitylog.po rename to services/activitylog/pkg/server/http/l10n/locale/el/LC_MESSAGES/activitylog.po diff --git a/services/activitylog/pkg/service/event/l10n/locale/es/LC_MESSAGES/activitylog.po b/services/activitylog/pkg/server/http/l10n/locale/es/LC_MESSAGES/activitylog.po similarity index 100% rename from services/activitylog/pkg/service/event/l10n/locale/es/LC_MESSAGES/activitylog.po rename to services/activitylog/pkg/server/http/l10n/locale/es/LC_MESSAGES/activitylog.po diff --git a/services/activitylog/pkg/service/event/l10n/locale/fi/LC_MESSAGES/activitylog.po b/services/activitylog/pkg/server/http/l10n/locale/fi/LC_MESSAGES/activitylog.po similarity index 100% rename from services/activitylog/pkg/service/event/l10n/locale/fi/LC_MESSAGES/activitylog.po rename to services/activitylog/pkg/server/http/l10n/locale/fi/LC_MESSAGES/activitylog.po diff --git a/services/activitylog/pkg/service/event/l10n/locale/fr/LC_MESSAGES/activitylog.po b/services/activitylog/pkg/server/http/l10n/locale/fr/LC_MESSAGES/activitylog.po similarity index 100% rename from services/activitylog/pkg/service/event/l10n/locale/fr/LC_MESSAGES/activitylog.po rename to services/activitylog/pkg/server/http/l10n/locale/fr/LC_MESSAGES/activitylog.po diff --git a/services/activitylog/pkg/service/event/l10n/locale/hu/LC_MESSAGES/activitylog.po b/services/activitylog/pkg/server/http/l10n/locale/hu/LC_MESSAGES/activitylog.po similarity index 100% rename from services/activitylog/pkg/service/event/l10n/locale/hu/LC_MESSAGES/activitylog.po rename to services/activitylog/pkg/server/http/l10n/locale/hu/LC_MESSAGES/activitylog.po diff --git a/services/activitylog/pkg/service/event/l10n/locale/it/LC_MESSAGES/activitylog.po b/services/activitylog/pkg/server/http/l10n/locale/it/LC_MESSAGES/activitylog.po similarity index 100% rename from services/activitylog/pkg/service/event/l10n/locale/it/LC_MESSAGES/activitylog.po rename to services/activitylog/pkg/server/http/l10n/locale/it/LC_MESSAGES/activitylog.po diff --git a/services/activitylog/pkg/service/event/l10n/locale/ja/LC_MESSAGES/activitylog.po b/services/activitylog/pkg/server/http/l10n/locale/ja/LC_MESSAGES/activitylog.po similarity index 100% rename from services/activitylog/pkg/service/event/l10n/locale/ja/LC_MESSAGES/activitylog.po rename to services/activitylog/pkg/server/http/l10n/locale/ja/LC_MESSAGES/activitylog.po diff --git a/services/activitylog/pkg/service/event/l10n/locale/ko/LC_MESSAGES/activitylog.po b/services/activitylog/pkg/server/http/l10n/locale/ko/LC_MESSAGES/activitylog.po similarity index 100% rename from services/activitylog/pkg/service/event/l10n/locale/ko/LC_MESSAGES/activitylog.po rename to services/activitylog/pkg/server/http/l10n/locale/ko/LC_MESSAGES/activitylog.po diff --git a/services/activitylog/pkg/service/event/l10n/locale/lo/LC_MESSAGES/activitylog.po b/services/activitylog/pkg/server/http/l10n/locale/lo/LC_MESSAGES/activitylog.po similarity index 100% rename from services/activitylog/pkg/service/event/l10n/locale/lo/LC_MESSAGES/activitylog.po rename to services/activitylog/pkg/server/http/l10n/locale/lo/LC_MESSAGES/activitylog.po diff --git a/services/activitylog/pkg/service/event/l10n/locale/nl/LC_MESSAGES/activitylog.po b/services/activitylog/pkg/server/http/l10n/locale/nl/LC_MESSAGES/activitylog.po similarity index 100% rename from services/activitylog/pkg/service/event/l10n/locale/nl/LC_MESSAGES/activitylog.po rename to services/activitylog/pkg/server/http/l10n/locale/nl/LC_MESSAGES/activitylog.po diff --git a/services/activitylog/pkg/service/event/l10n/locale/pl/LC_MESSAGES/activitylog.po b/services/activitylog/pkg/server/http/l10n/locale/pl/LC_MESSAGES/activitylog.po similarity index 100% rename from services/activitylog/pkg/service/event/l10n/locale/pl/LC_MESSAGES/activitylog.po rename to services/activitylog/pkg/server/http/l10n/locale/pl/LC_MESSAGES/activitylog.po diff --git a/services/activitylog/pkg/service/event/l10n/locale/pt/LC_MESSAGES/activitylog.po b/services/activitylog/pkg/server/http/l10n/locale/pt/LC_MESSAGES/activitylog.po similarity index 100% rename from services/activitylog/pkg/service/event/l10n/locale/pt/LC_MESSAGES/activitylog.po rename to services/activitylog/pkg/server/http/l10n/locale/pt/LC_MESSAGES/activitylog.po diff --git a/services/activitylog/pkg/service/event/l10n/locale/ru/LC_MESSAGES/activitylog.po b/services/activitylog/pkg/server/http/l10n/locale/ru/LC_MESSAGES/activitylog.po similarity index 100% rename from services/activitylog/pkg/service/event/l10n/locale/ru/LC_MESSAGES/activitylog.po rename to services/activitylog/pkg/server/http/l10n/locale/ru/LC_MESSAGES/activitylog.po diff --git a/services/activitylog/pkg/service/event/l10n/locale/sv/LC_MESSAGES/activitylog.po b/services/activitylog/pkg/server/http/l10n/locale/sv/LC_MESSAGES/activitylog.po similarity index 100% rename from services/activitylog/pkg/service/event/l10n/locale/sv/LC_MESSAGES/activitylog.po rename to services/activitylog/pkg/server/http/l10n/locale/sv/LC_MESSAGES/activitylog.po diff --git a/services/activitylog/pkg/service/event/l10n/locale/vi/LC_MESSAGES/activitylog.po b/services/activitylog/pkg/server/http/l10n/locale/vi/LC_MESSAGES/activitylog.po similarity index 100% rename from services/activitylog/pkg/service/event/l10n/locale/vi/LC_MESSAGES/activitylog.po rename to services/activitylog/pkg/server/http/l10n/locale/vi/LC_MESSAGES/activitylog.po diff --git a/services/activitylog/pkg/service/event/l10n/locale/zh/LC_MESSAGES/activitylog.po b/services/activitylog/pkg/server/http/l10n/locale/zh/LC_MESSAGES/activitylog.po similarity index 100% rename from services/activitylog/pkg/service/event/l10n/locale/zh/LC_MESSAGES/activitylog.po rename to services/activitylog/pkg/server/http/l10n/locale/zh/LC_MESSAGES/activitylog.po diff --git a/services/activitylog/pkg/server/http/option.go b/services/activitylog/pkg/server/http/option.go index 6fb3d5cf5c..aaf2bc8787 100644 --- a/services/activitylog/pkg/server/http/option.go +++ b/services/activitylog/pkg/server/http/option.go @@ -3,18 +3,12 @@ package http import ( "context" - gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" "github.com/opencloud-eu/opencloud/pkg/log" - ehsvc "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/services/eventhistory/v0" settingssvc "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/services/settings/v0" "github.com/opencloud-eu/opencloud/services/activitylog/pkg/config" - "github.com/opencloud-eu/opencloud/services/activitylog/pkg/metrics" - "github.com/opencloud-eu/reva/v2/pkg/events" - "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" - "github.com/spf13/pflag" - "go-micro.dev/v4/store" "go.opentelemetry.io/otel/trace" + "go.opentelemetry.io/otel/trace/noop" ) // Option defines a single option function. @@ -22,19 +16,15 @@ type Option func(o *Options) // Options defines the available options for this package. type Options struct { - Logger log.Logger - Context context.Context - Config *config.Config - Metrics *metrics.Metrics - Flags []pflag.Flag - Namespace string - Store store.Store - Stream events.Stream - GatewaySelector pool.Selectable[gateway.GatewayAPIClient] - TraceProvider trace.TracerProvider - HistoryClient ehsvc.EventHistoryService - ValueClient settingssvc.ValueService - RegisteredEvents []events.Unmarshaller + Name string + Namespace string + Logger log.Logger + Context context.Context + Config *config.Config + Flags []pflag.Flag + Service ActivityLogService + TraceProvider trace.TracerProvider + ValueClient settingssvc.ValueService } // newOptions initializes the available default options. @@ -69,10 +59,10 @@ func Config(val *config.Config) Option { } } -// Metrics provides a function to set the metrics option. -func Metrics(val *metrics.Metrics) Option { +// Service provides a function to set the service option. +func Service(val ActivityLogService) Option { return func(o *Options) { - o.Metrics = val + o.Service = val } } @@ -83,58 +73,20 @@ func Flags(flags ...pflag.Flag) Option { } } -// Namespace provides a function to set the Namespace option. -func Namespace(val string) Option { - return func(o *Options) { - o.Namespace = val - } -} - -// Store provides a function to configure the store -func Store(store store.Store) Option { - return func(o *Options) { - o.Store = store - } -} - -// Stream provides a function to configure the stream -func Stream(stream events.Stream) Option { - return func(o *Options) { - o.Stream = stream - } -} - -// GatewaySelector provides a function to configure the gateway client selector -func GatewaySelector(gatewaySelector pool.Selectable[gateway.GatewayAPIClient]) Option { - return func(o *Options) { - o.GatewaySelector = gatewaySelector - } -} - -// HistoryClient provides a function to configure the event history client -func HistoryClient(h ehsvc.EventHistoryService) Option { - return func(o *Options) { - o.HistoryClient = h - } -} - -// RegisteredEvents provides a function to register events -func RegisteredEvents(evs []events.Unmarshaller) Option { - return func(o *Options) { - o.RegisteredEvents = evs - } -} - -// TraceProvider provides a function to set the TracerProvider option -func TraceProvider(val trace.TracerProvider) Option { +// TraceProvider provides a function to configure the trace provider +func TraceProvider(traceProvider trace.TracerProvider) Option { return func(o *Options) { - o.TraceProvider = val + if traceProvider != nil { + o.TraceProvider = traceProvider + } else { + o.TraceProvider = noop.NewTracerProvider() + } } } -// ValueClient provides a function to set the ValueClient options -func ValueClient(val settingssvc.ValueService) Option { +// ValueClient adds a grpc client for the value service +func ValueClient(vs settingssvc.ValueService) Option { return func(o *Options) { - o.ValueClient = val + o.ValueClient = vs } } diff --git a/services/activitylog/pkg/server/http/server.go b/services/activitylog/pkg/server/http/server.go index a6200daf76..9a2493560a 100644 --- a/services/activitylog/pkg/server/http/server.go +++ b/services/activitylog/pkg/server/http/server.go @@ -1,49 +1,61 @@ package http import ( - "fmt" - - stdhttp "net/http" + "context" + "embed" + "encoding/json" + "net/http" "github.com/go-chi/chi/v5" chimiddleware "github.com/go-chi/chi/v5/middleware" + libregraph "github.com/opencloud-eu/libre-graph-api-go" "github.com/opencloud-eu/opencloud/pkg/account" "github.com/opencloud-eu/opencloud/pkg/cors" + "github.com/opencloud-eu/opencloud/pkg/l10n" + "github.com/opencloud-eu/opencloud/pkg/log" "github.com/opencloud-eu/opencloud/pkg/middleware" - "github.com/opencloud-eu/opencloud/pkg/service/http" - "github.com/opencloud-eu/opencloud/pkg/tracing" + ohttp "github.com/opencloud-eu/opencloud/pkg/service/http" "github.com/opencloud-eu/opencloud/pkg/version" - svc "github.com/opencloud-eu/opencloud/services/activitylog/pkg/service/event" - "github.com/riandyrn/otelchi" + settingssvc "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/services/settings/v0" + revactx "github.com/opencloud-eu/reva/v2/pkg/ctx" "go-micro.dev/v4" + "google.golang.org/grpc/metadata" ) -// Service is the service interface -type Service any +var ( + //go:embed l10n/locale + _localeFS embed.FS + + // subfolder where the translation files are stored + _localeSubPath = "l10n/locale" + + // domain of the activitylog service (transifex) + _domain = "activitylog" +) // Server initializes the http service and server. -func Server(opts ...Option) (http.Service, error) { +func Server(opts ...Option) (ohttp.Service, error) { options := newOptions(opts...) + service := options.Service - service, err := http.NewService( - http.TLSConfig(options.Config.HTTP.TLS), - http.Logger(options.Logger), - http.Namespace(options.Config.HTTP.Namespace), - http.Name(options.Config.Service.Name), - http.Version(version.GetString()), - http.Address(options.Config.HTTP.Addr), - http.Context(options.Context), - http.Flags(options.Flags...), - http.TraceProvider(options.TraceProvider), + newService, err := ohttp.NewService( + ohttp.TLSConfig(options.Config.HTTP.TLS), + ohttp.Logger(options.Logger), + ohttp.Namespace(options.Config.HTTP.Namespace), + ohttp.Name(options.Config.Service.Name), + ohttp.Version(version.GetString()), + ohttp.Address(options.Config.HTTP.Addr), + ohttp.Context(options.Context), + ohttp.Flags(options.Flags...), ) if err != nil { options.Logger.Error(). Err(err). Msg("Error initializing http service") - return http.Service{}, fmt.Errorf("could not initialize http service: %w", err) + return ohttp.Service{}, err } - middlewares := []func(stdhttp.Handler) stdhttp.Handler{ + middlewares := []func(http.Handler) http.Handler{ chimiddleware.RequestID, middleware.Version( options.Config.Service.Name, @@ -52,6 +64,7 @@ func Server(opts ...Option) (http.Service, error) { middleware.Logger( options.Logger, ), + middleware.TraceContext, middleware.ExtractAccountUUID( account.Logger(options.Logger), account.JWTSecret(options.Config.TokenManager.JWTSecret), @@ -68,33 +81,71 @@ func Server(opts ...Option) (http.Service, error) { mux := chi.NewMux() mux.Use(middlewares...) - mux.Use( - otelchi.Middleware( - "actitivylog", - otelchi.WithChiRoutes(mux), - otelchi.WithTracerProvider(options.TraceProvider), - otelchi.WithPropagators(tracing.GetPropagator()), - ), - ) + t := l10n.NewTranslatorFromCommonConfig(options.Config.DefaultLanguage, _domain, options.Config.TranslationPath, _localeFS, _localeSubPath) + mux.Route(options.Config.HTTP.Root, func(r chi.Router) { + r.Get("/graph/v1beta1/extensions/org.libregraph/activities", GetItemActivitiesHandler(options.Logger, service, options.ValueClient, t)) + }) - handle, err := svc.New( - svc.Logger(options.Logger), - svc.Stream(options.Stream), - svc.Mux(mux), - svc.Config(options.Config), - svc.GatewaySelector(options.GatewaySelector), - svc.TraceProvider(options.TraceProvider), - svc.HistoryClient(options.HistoryClient), - svc.ValueClient(options.ValueClient), - svc.RegisteredEvents(options.RegisteredEvents), - ) + err = micro.RegisterHandler(newService.Server(), mux) if err != nil { - return http.Service{}, err + options.Logger.Fatal().Err(err).Msg("failed to register the handler") } - if err := micro.RegisterHandler(service.Server(), handle); err != nil { - return http.Service{}, err - } + newService.Init() + return newService, nil - return service, nil +} + +// Service defines the business logic implementations need to provide. +type ActivityLogService interface { + GetItemActivities(ctx context.Context, query, loc string, t l10n.Translator) ([]libregraph.Activity, error) +} + +// GetActivitiesResponse is the response on GET activities requests +type GetActivitiesResponse struct { + Activities []libregraph.Activity `json:"value"` +} + +func GetItemActivitiesHandler(log log.Logger, s ActivityLogService, vc settingssvc.ValueService, t l10n.Translator) func(w http.ResponseWriter, r *http.Request) { + return func(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + ctx = metadata.AppendToOutgoingContext(ctx, revactx.TokenHeader, r.Header.Get(revactx.TokenHeader)) + + activeUser, ok := revactx.ContextGetUser(ctx) + if !ok { + w.WriteHeader(http.StatusUnauthorized) + return + } + + loc := l10n.MustGetUserLocale(ctx, activeUser.GetId().GetOpaqueId(), r.Header.Get(l10n.HeaderAcceptLanguage), vc) + + activities, err := s.GetItemActivities(ctx, r.URL.Query().Get("kql"), loc, t) + if err != nil { + log.Error().Err(err).Msg("error getting activities") + w.WriteHeader(http.StatusInternalServerError) + return + } + res := GetActivitiesResponse{ + Activities: activities, + } + + b, err := json.Marshal(res) + if err != nil { + log.Error().Err(err).Msg("error marshalling activities") + w.WriteHeader(http.StatusInternalServerError) + return + } + + w.Header().Set("Content-Type", "application/json; odata.metadata=minimal") + w.Header().Set("OData-Version", "4.0") + if reqID := chimiddleware.GetReqID(ctx); reqID != "" { + w.Header().Set("request-id", reqID) + } + w.Header().Set("Cache-Control", "no-cache") + + w.WriteHeader(http.StatusOK) + if _, err := w.Write(b); err != nil { + log.Error().Err(err).Msg("error writing response") + } + } } diff --git a/services/activitylog/pkg/service/activitylog/activitylog.go b/services/activitylog/pkg/service/activitylog/activitylog.go new file mode 100644 index 0000000000..6483eafabc --- /dev/null +++ b/services/activitylog/pkg/service/activitylog/activitylog.go @@ -0,0 +1,353 @@ +package activitylog + +import ( + "context" + "encoding/base32" + "encoding/json" + "fmt" + "sort" + "strconv" + "strings" + "sync" + "time" + + provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + "github.com/jellydator/ttlcache/v2" + "github.com/nats-io/nats.go" + "github.com/opencloud-eu/opencloud/pkg/log" + "github.com/opencloud-eu/opencloud/services/activitylog/pkg/data" + "github.com/opencloud-eu/reva/v2/pkg/storagespace" + "github.com/vmihailenco/msgpack/v5" + "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/trace" +) + +var tracer trace.Tracer + +func init() { + tracer = otel.Tracer("github.com/opencloud-eu/opencloud/services/activitylog/pkg/service/activitylog") +} + +var ( + _maxActivitiesDefault = 6000 + _writeBufferDuration = 10 * time.Second +) + +// Activitylog stores and retrieves activities for resources and their parents from a nats kv +type ActivityLog struct { + log log.Logger + // FIXME the lock does not protect agains concurrent resource activities on multiple instances + // known since https://github.com/owncloud/ocis/pull/9361#pullrequestreview-2135350157 + // current ocis discussion in https://github.com/owncloud/ocis/issues/12475 + lock sync.RWMutex + debouncer *Debouncer + parentIdCache *ttlcache.Cache + natskv nats.KeyValue + + maxActivities int +} + +type batchInfo struct { + key string + count int + timestamp time.Time +} + +// New creates a new ActivitylogService +func New(kv nats.KeyValue, opts ...Option) (*ActivityLog, error) { + o := &Options{ + MaxActivities: _maxActivitiesDefault, + WriteBufferDuration: _writeBufferDuration, + Logger: log.NopLogger(), + } + for _, opt := range opts { + opt(o) + } + + cache := ttlcache.NewCache() + err := cache.SetTTL(30 * time.Second) + if err != nil { + return nil, err + } + + s := &ActivityLog{ + log: o.Logger, + lock: sync.RWMutex{}, + parentIdCache: cache, + maxActivities: o.MaxActivities, + natskv: kv, + } + s.debouncer = NewDebouncer(o.WriteBufferDuration, s.StoreActivity) + + // run migrations + err = s.runMigrations(context.Background(), kv) + if err != nil { + return nil, err + } + + return s, nil +} + +// RemoveResource removes the resource from the store +func (a *ActivityLog) RemoveResource(rid *provider.ResourceId) error { + if rid == nil { + return fmt.Errorf("resource id is required") + } + + a.lock.Lock() + defer a.lock.Unlock() + + return a.natskv.Delete(storagespace.FormatResourceID(rid)) +} + +func (a *ActivityLog) AddActivity(ctx context.Context, initRef *provider.Reference, parentId *provider.ResourceId, eventID string, timestamp time.Time, getResource func(context.Context, *provider.Reference) (*provider.ResourceInfo, error)) error { + var ( + err error + depth int + ref = initRef + ) + ctx, span := tracer.Start(ctx, "AddActivity") + defer span.End() + for { + var info *provider.ResourceInfo + id := ref.GetResourceId() + if ref.Path != "" { + // Path based reference, we need to resolve the resource id + ctx, span = tracer.Start(ctx, "AddActivity.getResource") + info, err = getResource(ctx, ref) + span.End() + if err != nil { + return fmt.Errorf("could not get resource info: %w", err) + } + id = info.GetId() + } + if id == nil { + return fmt.Errorf("resource id is required") + } + + key := storagespace.FormatResourceID(id) + a.debouncer.Debounce(key, data.RawActivity{ + EventID: eventID, + Depth: depth, + Timestamp: timestamp, + }) + + if id.OpaqueId == id.SpaceId { + // we are at the root of the space, no need to go further + break + } + + // check if parent id is cached + // parent id is cached in the format $! + // if it is not cached, get the resource info and cache it + if parentId == nil { + if v, err := a.parentIdCache.Get(key); err != nil { + if info == nil { + ctx, span := tracer.Start(ctx, "AddActivity.getResource parent") + info, err = getResource(ctx, ref) + span.End() + if err != nil || info.GetParentId() == nil || info.GetParentId().GetOpaqueId() == "" { + return fmt.Errorf("could not get parent id: %w", err) + } + } + parentId = info.GetParentId() + a.parentIdCache.Set(key, parentId) + } else { + parentId = v.(*provider.ResourceId) + } + } else { + a.log.Debug().Msg("parent id is cached") + } + + depth++ + ref = &provider.Reference{ResourceId: parentId} + parentId = nil // reset parent id so it's not reused in the next iteration + } + + return nil +} + +func (a *ActivityLog) StoreActivity(resourceID string, activities []data.RawActivity) error { + a.lock.Lock() + defer a.lock.Unlock() + + ctx, span := tracer.Start(context.Background(), "storeActivity") + defer span.End() + + _, subspan := tracer.Start(ctx, "storeActivity.Marshal") + b, err := msgpack.Marshal(activities) + if err != nil { + return err + } + subspan.End() + + _, subspan = tracer.Start(ctx, "storeActivity.natskv.Put") + key := natsKey(resourceID, len(activities)) + _, err = a.natskv.Put(key, b) + if err != nil { + return err + } + subspan.End() + + ctx, subspan = tracer.Start(ctx, "storeActivity.enforceMaxActivities") + a.enforceMaxActivities(ctx, resourceID) + subspan.End() + return nil +} + +func (a *ActivityLog) enforceMaxActivities(ctx context.Context, resourceID string) { + if a.maxActivities <= 0 { + return + } + + key := fmt.Sprintf("%s.>", base32.StdEncoding.EncodeToString([]byte(resourceID))) + + _, subspan := tracer.Start(ctx, "enforceMaxActivities.watch") + watcher, err := a.natskv.Watch(key, nats.IgnoreDeletes()) + if err != nil { + a.log.Error().Err(err).Str("resourceID", resourceID).Msg("could not watch") + return + } + defer watcher.Stop() + + var keys []string + for update := range watcher.Updates() { + if update == nil { + break + } + + var batchActivities []data.RawActivity + if err := msgpack.Unmarshal(update.Value(), &batchActivities); err != nil { + a.log.Debug().Err(err).Str("resourceID", resourceID).Msg("could not unmarshal messagepack, trying json") + } + keys = append(keys, update.Key()) + } + subspan.End() + + _, subspan = tracer.Start(ctx, "enforceMaxActivities.compile") + // Parse keys into batches + batches := make([]batchInfo, 0) + var activitiesCount int + for _, k := range keys { + parts := strings.SplitN(k, ".", 3) + if len(parts) < 3 { + a.log.Warn().Str("key", k).Msg("skipping key, not enough parts") + continue + } + + c, err := strconv.Atoi(parts[1]) + if err != nil { + a.log.Warn().Str("key", k).Msg("skipping key, can not parse count") + continue + } + + // parse timestamp + nano, err := strconv.ParseInt(parts[2], 10, 64) + if err != nil { + a.log.Warn().Str("key", k).Msg("skipping key, can not parse timestamp") + continue + } + + batches = append(batches, batchInfo{ + key: k, + count: c, + timestamp: time.Unix(0, nano), + }) + activitiesCount += c + } + + // sort batches by timestamp + sort.Slice(batches, func(i, j int) bool { + return batches[i].timestamp.Before(batches[j].timestamp) + }) + subspan.End() + + _, subspan = tracer.Start(ctx, "enforceMaxActivities.delete") + // remove oldest keys until we are at max activities + for _, b := range batches { + if activitiesCount-b.count < a.maxActivities { + break + } + + activitiesCount -= b.count + err = a.natskv.Delete(b.key) + if err != nil { + a.log.Error().Err(err).Str("key", b.key).Msg("could not delete key") + break + } + } + subspan.End() +} + +func (a *ActivityLog) InvalidateCachedParentID(purgeId *provider.ResourceId) { + if err := a.parentIdCache.Remove(storagespace.FormatResourceID(purgeId)); err != nil { + a.log.Error().Interface("event", purgeId).Err(err).Msg("could not delete parent id cache") + } +} + +func natsKey(resourceID string, activitiesCount int) string { + return fmt.Sprintf("%s.%d.%d", + base32.StdEncoding.EncodeToString([]byte(resourceID)), + activitiesCount, + time.Now().UnixNano()) +} + +func (a *ActivityLog) Activities(rid *provider.ResourceId) ([]data.RawActivity, error) { + a.lock.RLock() + defer a.lock.RUnlock() + + return a.activities(rid) +} + +func (a *ActivityLog) activities(rid *provider.ResourceId) ([]data.RawActivity, error) { + resourceID := storagespace.FormatResourceID(rid) + + glob := fmt.Sprintf("%s.>", base32.StdEncoding.EncodeToString([]byte(resourceID))) + + watcher, err := a.natskv.Watch(glob, nats.IgnoreDeletes()) + if err != nil { + return nil, err + } + defer watcher.Stop() + + var activities []data.RawActivity + for update := range watcher.Updates() { + if update == nil { + break + } + + var batchActivities []data.RawActivity + if err := msgpack.Unmarshal(update.Value(), &batchActivities); err != nil { + a.log.Debug().Err(err).Str("resourceID", resourceID).Msg("could not unmarshal messagepack") + } + activities = append(activities, batchActivities...) + } + + return activities, nil +} + +// RemoveActivities removes the activities from the given resource +func (a *ActivityLog) RemoveActivities(rid *provider.ResourceId, toDelete map[string]struct{}) error { + a.lock.Lock() + defer a.lock.Unlock() + + curActivities, err := a.activities(rid) + if err != nil { + return err + } + + var acts []data.RawActivity + for _, a := range curActivities { + if _, ok := toDelete[a.EventID]; !ok { + acts = append(acts, a) + } + } + + b, err := json.Marshal(acts) + if err != nil { + return err + } + + _, err = a.natskv.Put(storagespace.FormatResourceID(rid), b) + return err +} diff --git a/services/activitylog/pkg/service/activitylog/debouncer.go b/services/activitylog/pkg/service/activitylog/debouncer.go new file mode 100644 index 0000000000..ed200c9b01 --- /dev/null +++ b/services/activitylog/pkg/service/activitylog/debouncer.go @@ -0,0 +1,77 @@ +package activitylog + +import ( + "sync" + "time" + + "github.com/opencloud-eu/opencloud/services/activitylog/pkg/data" +) + +type Debouncer struct { + after time.Duration + f func(id string, ra []data.RawActivity) error + pending sync.Map + inProgress sync.Map + + mutex sync.Mutex +} + +type queueItem struct { + activities []data.RawActivity + timer *time.Timer +} + +// NewDebouncer returns a new Debouncer instance +func NewDebouncer(d time.Duration, f func(id string, ra []data.RawActivity) error) *Debouncer { + return &Debouncer{ + after: d, + f: f, + pending: sync.Map{}, + inProgress: sync.Map{}, + } +} + +// Debounce restarts the debounce timer for the given space +func (d *Debouncer) Debounce(id string, ra data.RawActivity) { + if d.after == 0 { + d.f(id, []data.RawActivity{ra}) + return + } + + d.mutex.Lock() + defer d.mutex.Unlock() + + activities := []data.RawActivity{ra} + item := &queueItem{ + activities: activities, + } + if i, ok := d.pending.Load(id); ok { + // if the item is already in the queue, append the new activities + item, ok = i.(*queueItem) + if ok { + item.activities = append(item.activities, ra) + } + } + + if item.timer == nil { + item.timer = time.AfterFunc(d.after, func() { + if _, ok := d.inProgress.Load(id); ok { + // Reschedule this run for when the previous run has finished + d.mutex.Lock() + if i, ok := d.pending.Load(id); ok { + i.(*queueItem).timer.Reset(d.after) + } + + d.mutex.Unlock() + return + } + + d.pending.Delete(id) + d.inProgress.Store(id, true) + defer d.inProgress.Delete(id) + d.f(id, item.activities) + }) + } + + d.pending.Store(id, item) +} diff --git a/services/activitylog/pkg/service/event/migrations.go b/services/activitylog/pkg/service/activitylog/migrations.go similarity index 93% rename from services/activitylog/pkg/service/event/migrations.go rename to services/activitylog/pkg/service/activitylog/migrations.go index ab62a17bb1..1b7e6d5c15 100644 --- a/services/activitylog/pkg/service/event/migrations.go +++ b/services/activitylog/pkg/service/activitylog/migrations.go @@ -1,4 +1,4 @@ -package service +package activitylog import ( "context" @@ -7,6 +7,7 @@ import ( "log" "github.com/nats-io/nats.go" + "github.com/opencloud-eu/opencloud/services/activitylog/pkg/data" "github.com/vmihailenco/msgpack/v5" ) @@ -15,7 +16,7 @@ const currentMigrationVersion = "1" // RunMigrations checks the activitylog data version and runs migrations if necessary. // It should be called during service startup, after the NATS KeyValue store is initialized. -func (a *ActivitylogService) runMigrations(ctx context.Context, kv nats.KeyValue) error { +func (a *ActivityLog) runMigrations(ctx context.Context, kv nats.KeyValue) error { entry, err := kv.Get(activitylogVersionKey) if err == nats.ErrKeyNotFound { a.log.Info().Msg("activitylog version key not found. Running migration to V1...") @@ -40,7 +41,7 @@ func (a *ActivitylogService) runMigrations(ctx context.Context, kv nats.KeyValue // For each such key, it creates a new key in the format "originalKey.count.timestamp" // and stores the original list of strings (re-marshalled to messagepack) as its value. // Finally, it sets the activitylog.version key to "1". -func (a *ActivitylogService) migrateToV1(_ context.Context, kv nats.KeyValue) error { +func (a *ActivityLog) migrateToV1(_ context.Context, kv nats.KeyValue) error { lister, err := kv.ListKeys() if err != nil { return fmt.Errorf("migrateToV1: failed to list keys from NATS KV store: %w", err) @@ -83,7 +84,7 @@ func (a *ActivitylogService) migrateToV1(_ context.Context, kv nats.KeyValue) er } // Unmarshal value into a list of strings - var activities []RawActivity + var activities []data.RawActivity if err := msgpack.Unmarshal(val.Data, &activities); err != nil { if err := json.Unmarshal(val.Data, &activities); err != nil { // This key's value is not a JSON array of strings. Skip it. diff --git a/services/activitylog/pkg/service/activitylog/options.go b/services/activitylog/pkg/service/activitylog/options.go new file mode 100644 index 0000000000..4215c6f316 --- /dev/null +++ b/services/activitylog/pkg/service/activitylog/options.go @@ -0,0 +1,35 @@ +package activitylog + +import ( + "time" + + "github.com/opencloud-eu/opencloud/pkg/log" +) + +// Option for the activitylog service +type Option func(*Options) + +// Options for the activitylog service +type Options struct { + Logger log.Logger + MaxActivities int + WriteBufferDuration time.Duration +} + +// Logger configures a logger for the activitylog service +func Logger(log log.Logger) Option { + return func(o *Options) { + o.Logger = log + } +} + +func MaxActivities(max int) Option { + return func(o *Options) { + o.MaxActivities = max + } +} +func WriteBufferDuration(d time.Duration) Option { + return func(o *Options) { + o.WriteBufferDuration = d + } +} diff --git a/services/activitylog/pkg/service/event/service_suite_test.go b/services/activitylog/pkg/service/activitylog/service_suite_test.go similarity index 87% rename from services/activitylog/pkg/service/event/service_suite_test.go rename to services/activitylog/pkg/service/activitylog/service_suite_test.go index b3d922b571..366053c0b4 100644 --- a/services/activitylog/pkg/service/event/service_suite_test.go +++ b/services/activitylog/pkg/service/activitylog/service_suite_test.go @@ -1,4 +1,4 @@ -package service_test +package activitylog_test import ( "testing" diff --git a/services/activitylog/pkg/service/event/service_test.go b/services/activitylog/pkg/service/activitylog/service_test.go similarity index 81% rename from services/activitylog/pkg/service/event/service_test.go rename to services/activitylog/pkg/service/activitylog/service_test.go index 499ef09ca3..c968a935ec 100644 --- a/services/activitylog/pkg/service/event/service_test.go +++ b/services/activitylog/pkg/service/activitylog/service_test.go @@ -1,4 +1,4 @@ -package service +package activitylog_test import ( "context" @@ -8,15 +8,14 @@ import ( "time" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" - "github.com/go-chi/chi/v5" "github.com/google/uuid" nserver "github.com/nats-io/nats-server/v2/server" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/opencloud-eu/opencloud/services/activitylog/pkg/command" "github.com/opencloud-eu/opencloud/services/activitylog/pkg/config" - eventsmocks "github.com/opencloud-eu/reva/v2/pkg/events/mocks" - "github.com/test-go/testify/mock" - "go.opentelemetry.io/otel/trace/noop" + "github.com/opencloud-eu/opencloud/services/activitylog/pkg/data" + "github.com/opencloud-eu/opencloud/services/activitylog/pkg/service/activitylog" ) var ( @@ -65,31 +64,25 @@ var _ = SynchronizedAfterSuite(func() { var _ = Describe("ActivitylogService", func() { var ( - alog *ActivitylogService + alog *activitylog.ActivityLog getResource func(_ context.Context, ref *provider.Reference) (*provider.ResourceInfo, error) writebufferduration = 100 * time.Millisecond ) JustBeforeEach(func() { var err error - stream := &eventsmocks.Stream{} - stream.EXPECT().Consume(mock.Anything, mock.Anything).Return(nil, nil) - alog, err = New( - Config(&config.Config{ - Service: config.Service{ - Name: "activitylog-test", - }, - Store: config.Store{ - Store: "nats-js-kv", - Nodes: []string{server.Addr().String()}, - Database: "activitylog-test-" + uuid.New().String(), - }, - MaxActivities: 4, - WriteBufferDuration: writebufferduration, - }), - Stream(stream), - TraceProvider(noop.NewTracerProvider()), - Mux(chi.NewMux()), + db := "activitylog-test-" + uuid.New().String() + + kv, err := command.ConnectNatsKV(config.Store{ + Nodes: []string{server.Addr().String()}, + Database: db, + }) + Expect(err).ToNot(HaveOccurred()) + + alog, err = activitylog.New( + kv, + activitylog.MaxActivities(4), + activitylog.WriteBufferDuration(writebufferduration), ) Expect(err).ToNot(HaveOccurred()) }) @@ -104,7 +97,7 @@ var _ = Describe("ActivitylogService", func() { Name string Tree map[string]*provider.ResourceInfo Activities map[string]string - Expected map[string][]RawActivity + Expected map[string][]data.RawActivity } testCases := []testCase{ @@ -118,7 +111,7 @@ var _ = Describe("ActivitylogService", func() { Activities: map[string]string{ "activity": "base", }, - Expected: map[string][]RawActivity{ + Expected: map[string][]data.RawActivity{ "base": activitites("activity", 0), "parent": activitites("activity", 1), "spaceid": activitites("activity", 2), @@ -135,7 +128,7 @@ var _ = Describe("ActivitylogService", func() { "activity1": "base", "activity2": "base", }, - Expected: map[string][]RawActivity{ + Expected: map[string][]data.RawActivity{ "base": activitites("activity1", 0, "activity2", 0), "parent": activitites("activity1", 1, "activity2", 1), "spaceid": activitites("activity1", 2, "activity2", 2), @@ -152,7 +145,7 @@ var _ = Describe("ActivitylogService", func() { } for k, v := range tc.Activities { - err := alog.addActivity(context.Background(), reference(v), nil, k, time.Time{}, getResource) + err := alog.AddActivity(context.Background(), reference(v), nil, k, time.Time{}, getResource) Expect(err).NotTo(HaveOccurred()) } }) @@ -191,9 +184,9 @@ var _ = Describe("ActivitylogService", func() { It("debounces activities", func() { - err := alog.addActivity(context.Background(), reference("base"), nil, "activity1", time.Time{}, getResource) + err := alog.AddActivity(context.Background(), reference("base"), nil, "activity1", time.Time{}, getResource) Expect(err).NotTo(HaveOccurred()) - err = alog.addActivity(context.Background(), reference("base"), nil, "activity2", time.Time{}, getResource) + err = alog.AddActivity(context.Background(), reference("base"), nil, "activity2", time.Time{}, getResource) Expect(err).NotTo(HaveOccurred()) Eventually(func(g Gomega) { @@ -204,7 +197,7 @@ var _ = Describe("ActivitylogService", func() { }) It("adheres to the MaxActivities setting", func() { - err := alog.addActivity(context.Background(), reference("base"), nil, "activity1", time.Time{}, getResource) + err := alog.AddActivity(context.Background(), reference("base"), nil, "activity1", time.Time{}, getResource) Expect(err).NotTo(HaveOccurred()) Eventually(func(g Gomega) { activities, err := alog.Activities(resourceID("base")) @@ -212,7 +205,7 @@ var _ = Describe("ActivitylogService", func() { g.Expect(len(activities)).To(Equal(1)) }).Should(Succeed()) - err = alog.addActivity(context.Background(), reference("base"), nil, "activity2", time.Time{}, getResource) + err = alog.AddActivity(context.Background(), reference("base"), nil, "activity2", time.Time{}, getResource) Expect(err).NotTo(HaveOccurred()) Eventually(func(g Gomega) { activities, err := alog.Activities(resourceID("base")) @@ -220,11 +213,11 @@ var _ = Describe("ActivitylogService", func() { g.Expect(len(activities)).To(Equal(2)) }).Should(Succeed()) - err = alog.addActivity(context.Background(), reference("base"), nil, "activity3", time.Time{}, getResource) + err = alog.AddActivity(context.Background(), reference("base"), nil, "activity3", time.Time{}, getResource) Expect(err).NotTo(HaveOccurred()) - err = alog.addActivity(context.Background(), reference("base"), nil, "activity4", time.Time{}, getResource) + err = alog.AddActivity(context.Background(), reference("base"), nil, "activity4", time.Time{}, getResource) Expect(err).NotTo(HaveOccurred()) - err = alog.addActivity(context.Background(), reference("base"), nil, "activity5", time.Time{}, getResource) + err = alog.AddActivity(context.Background(), reference("base"), nil, "activity5", time.Time{}, getResource) Expect(err).NotTo(HaveOccurred()) Eventually(func(g Gomega) { @@ -241,9 +234,9 @@ var _ = Describe("ActivitylogService", func() { return tree[ref.GetResourceId().GetOpaqueId()], nil } - err := alog.addActivity(context.Background(), reference("base"), nil, "activity1", time.Time{}, getResource) + err := alog.AddActivity(context.Background(), reference("base"), nil, "activity1", time.Time{}, getResource) Expect(err).NotTo(HaveOccurred()) - err = alog.addActivity(context.Background(), reference("base"), nil, "activity2", time.Time{}, getResource) + err = alog.AddActivity(context.Background(), reference("base"), nil, "activity2", time.Time{}, getResource) Expect(err).NotTo(HaveOccurred()) Eventually(func(g Gomega) { @@ -252,9 +245,9 @@ var _ = Describe("ActivitylogService", func() { g.Expect(activities).To(ConsistOf(activitites("activity1", 0, "activity2", 0))) }).Should(Succeed()) - err = alog.addActivity(context.Background(), reference("base"), nil, "activity3", time.Time{}, getResource) + err = alog.AddActivity(context.Background(), reference("base"), nil, "activity3", time.Time{}, getResource) Expect(err).NotTo(HaveOccurred()) - err = alog.addActivity(context.Background(), reference("base"), nil, "activity4", time.Time{}, getResource) + err = alog.AddActivity(context.Background(), reference("base"), nil, "activity4", time.Time{}, getResource) Expect(err).NotTo(HaveOccurred()) Eventually(func(g Gomega) { @@ -267,9 +260,9 @@ var _ = Describe("ActivitylogService", func() { }) }) -func activitites(acts ...any) []RawActivity { - var activities []RawActivity - act := RawActivity{} +func activitites(acts ...any) []data.RawActivity { + var activities []data.RawActivity + act := data.RawActivity{} for _, a := range acts { switch v := a.(type) { case string: diff --git a/services/activitylog/pkg/service/event/service.go b/services/activitylog/pkg/service/event/service.go deleted file mode 100644 index a97ebf09a1..0000000000 --- a/services/activitylog/pkg/service/event/service.go +++ /dev/null @@ -1,652 +0,0 @@ -package service - -import ( - "context" - "crypto/tls" - "encoding/base32" - "encoding/json" - "fmt" - "path/filepath" - "sort" - "strconv" - "strings" - "sync" - "time" - - gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" - provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" - "github.com/go-chi/chi/v5" - "github.com/jellydator/ttlcache/v2" - "github.com/nats-io/nats.go" - "github.com/opencloud-eu/reva/v2/pkg/events" - "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" - "github.com/opencloud-eu/reva/v2/pkg/storagespace" - "github.com/opencloud-eu/reva/v2/pkg/utils" - "github.com/pkg/errors" - "github.com/vmihailenco/msgpack/v5" - "go.opentelemetry.io/otel/trace" - - "github.com/opencloud-eu/opencloud/pkg/log" - settingssvc "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/services/settings/v0" - "github.com/opencloud-eu/opencloud/services/activitylog/pkg/config" -) - -// Nats runs into max payload exceeded errors at around 7k activities. Let's keep a buffer. -var _maxActivitiesDefault = 6000 - -// RawActivity represents an activity as it is stored in the activitylog store -type RawActivity struct { - EventID string `json:"event_id"` - Depth int `json:"depth"` - Timestamp time.Time `json:"timestamp"` -} - -// ActivitylogService logs events per resource -type ActivitylogService struct { - cfg *config.Config - log log.Logger - events <-chan events.Event - gws pool.Selectable[gateway.GatewayAPIClient] - mux *chi.Mux - valService settingssvc.ValueService - lock sync.RWMutex - tp trace.TracerProvider - tracer trace.Tracer - debouncer *Debouncer - parentIdCache *ttlcache.Cache - natskv nats.KeyValue - - maxActivities int -} - -type Debouncer struct { - after time.Duration - f func(id string, ra []RawActivity) error - pending sync.Map - inProgress sync.Map - - mutex sync.Mutex -} - -type queueItem struct { - activities []RawActivity - timer *time.Timer -} - -type batchInfo struct { - key string - count int - timestamp time.Time -} - -// NewDebouncer returns a new Debouncer instance -func NewDebouncer(d time.Duration, f func(id string, ra []RawActivity) error) *Debouncer { - return &Debouncer{ - after: d, - f: f, - pending: sync.Map{}, - inProgress: sync.Map{}, - } -} - -// Debounce restarts the debounce timer for the given space -func (d *Debouncer) Debounce(id string, ra RawActivity) { - if d.after == 0 { - d.f(id, []RawActivity{ra}) - return - } - - d.mutex.Lock() - defer d.mutex.Unlock() - - activities := []RawActivity{ra} - item := &queueItem{ - activities: activities, - } - if i, ok := d.pending.Load(id); ok { - // if the item is already in the queue, append the new activities - item, ok = i.(*queueItem) - if ok { - item.activities = append(item.activities, ra) - } - } - - if item.timer == nil { - item.timer = time.AfterFunc(d.after, func() { - if _, ok := d.inProgress.Load(id); ok { - // Reschedule this run for when the previous run has finished - d.mutex.Lock() - if i, ok := d.pending.Load(id); ok { - i.(*queueItem).timer.Reset(d.after) - } - - d.mutex.Unlock() - return - } - - d.pending.Delete(id) - d.inProgress.Store(id, true) - defer d.inProgress.Delete(id) - d.f(id, item.activities) - }) - } - - d.pending.Store(id, item) -} - -// New creates a new ActivitylogService -func New(opts ...Option) (*ActivitylogService, error) { - o := &Options{ - MaxActivities: _maxActivitiesDefault, - } - for _, opt := range opts { - opt(o) - } - - cache := ttlcache.NewCache() - err := cache.SetTTL(30 * time.Second) - if err != nil { - return nil, err - } - - // Connect to NATS servers - natsOptions := nats.Options{ - Servers: o.Config.Store.Nodes, - } - if o.Config.Store.EnableTLS { - if o.Config.Store.TLSRootCACertificate != "" { - // when root ca is configured use it. an insecure flag is ignored. - nats.RootCAs(o.Config.Store.TLSRootCACertificate)(&natsOptions) - } else { - // enable tls and use insecure flag - nats.Secure(&tls.Config{MinVersion: tls.VersionTLS12, InsecureSkipVerify: o.Config.Store.TLSInsecure})(&natsOptions) - } - } - if o.Config.Store.AuthUsername != "" && o.Config.Store.AuthPassword != "" { - nats.UserInfo(o.Config.Store.AuthUsername, o.Config.Store.AuthPassword)(&natsOptions) - } - conn, err := natsOptions.Connect() - if err != nil { - return nil, err - } - - js, err := conn.JetStream() - if err != nil { - return nil, err - } - - kv, err := js.KeyValue(o.Config.Store.Database) - if err != nil { - if !errors.Is(err, nats.ErrBucketNotFound) { - return nil, errors.Wrapf(err, "Failed to get bucket (%s)", o.Config.Store.Database) - } - - kv, err = js.CreateKeyValue(&nats.KeyValueConfig{ - Bucket: o.Config.Store.Database, - }) - if err != nil { - return nil, errors.Wrapf(err, "Failed to create bucket (%s)", o.Config.Store.Database) - } - } - if err != nil { - return nil, err - } - - s := &ActivitylogService{ - log: o.Logger, - cfg: o.Config, - gws: o.GatewaySelector, - mux: o.Mux, - valService: o.ValueClient, - lock: sync.RWMutex{}, - tp: o.TraceProvider, - tracer: o.TraceProvider.Tracer("github.com/opencloud-eu/opencloud/services/activitylog/pkg/service"), - parentIdCache: cache, - maxActivities: o.Config.MaxActivities, - natskv: kv, - } - s.debouncer = NewDebouncer(o.Config.WriteBufferDuration, s.storeActivity) - - // run migrations - err = s.runMigrations(context.Background(), kv) - if err != nil { - return nil, err - } - - s.mux.Get("/graph/v1beta1/extensions/org.libregraph/activities", s.HandleGetItemActivities) - - go s.Run() - - return s, nil -} - -// Run runs the service -func (a *ActivitylogService) Run() { - for e := range a.events { - var err error - switch ev := e.Event.(type) { - case events.UploadReady: - err = a.AddActivity(ev.FileRef, ev.ParentID, e.ID, utils.TSToTime(ev.Timestamp)) - case events.FileTouched: - err = a.AddActivity(ev.Ref, ev.ParentID, e.ID, utils.TSToTime(ev.Timestamp)) - // Disabled https://github.com/owncloud/ocis/issues/10293 - //case events.FileDownloaded: - // we are only interested in public link downloads - so no need to store others. - //if ev.ImpersonatingUser.GetDisplayName() == "Public" { - // err = a.AddActivity(ev.Ref, e.ID, utils.TSToTime(ev.Timestamp)) - //} - case events.ContainerCreated: - err = a.AddActivity(ev.Ref, ev.ParentID, e.ID, utils.TSToTime(ev.Timestamp)) - case events.ItemTrashed: - err = a.AddActivityTrashed(ev.ID, ev.Ref, nil, e.ID, utils.TSToTime(ev.Timestamp)) - case events.ItemPurged: - err = a.RemoveResource(ev.ID) - case events.ItemMoved: - // remove the cached parent id for this resource - a.removeCachedParentID(ev.Ref) - - err = a.AddActivity(ev.Ref, nil, e.ID, utils.TSToTime(ev.Timestamp)) - case events.ShareCreated: - err = a.AddActivity(toRef(ev.ItemID), nil, e.ID, utils.TSToTime(ev.CTime)) - case events.ShareUpdated: - if ev.Sharer != nil && ev.ItemID != nil && ev.Sharer.GetOpaqueId() != ev.ItemID.GetSpaceId() { - err = a.AddActivity(toRef(ev.ItemID), nil, e.ID, utils.TSToTime(ev.MTime)) - } - case events.ShareRemoved: - err = a.AddActivity(toRef(ev.ItemID), nil, e.ID, ev.Timestamp) - case events.LinkCreated: - err = a.AddActivity(toRef(ev.ItemID), nil, e.ID, utils.TSToTime(ev.CTime)) - case events.LinkUpdated: - if ev.Sharer != nil && ev.ItemID != nil && ev.Sharer.GetOpaqueId() != ev.ItemID.GetSpaceId() { - err = a.AddActivity(toRef(ev.ItemID), nil, e.ID, utils.TSToTime(ev.MTime)) - } - case events.LinkRemoved: - err = a.AddActivity(toRef(ev.ItemID), nil, e.ID, utils.TSToTime(ev.Timestamp)) - case events.SpaceShared: - err = a.AddSpaceActivity(ev.ID, e.ID, ev.Timestamp) - case events.SpaceUnshared: - err = a.AddSpaceActivity(ev.ID, e.ID, ev.Timestamp) - } - - if err != nil { - a.log.Error().Err(err).Interface("event", e).Msg("could not process event") - } - } -} - -// AddActivity adds the activity to the given resource and all its parents -func (a *ActivitylogService) AddActivity(initRef *provider.Reference, parentId *provider.ResourceId, eventID string, timestamp time.Time) error { - gwc, err := a.gws.Next() - if err != nil { - return fmt.Errorf("cant get gateway client: %w", err) - } - - ctx, err := utils.GetServiceUserContext(a.cfg.ServiceAccount.ServiceAccountID, gwc, a.cfg.ServiceAccount.ServiceAccountSecret) - if err != nil { - return fmt.Errorf("cant get service user context: %w", err) - } - var span trace.Span - ctx, span = a.tracer.Start(ctx, "AddActivity") - defer span.End() - - return a.addActivity(ctx, initRef, parentId, eventID, timestamp, func(ctx context.Context, ref *provider.Reference) (*provider.ResourceInfo, error) { - return utils.GetResource(ctx, ref, gwc) - }) -} - -// AddActivityTrashed adds the activity to given trashed resource and all its former parents -func (a *ActivitylogService) AddActivityTrashed(resourceID *provider.ResourceId, reference *provider.Reference, parentId *provider.ResourceId, eventID string, timestamp time.Time) error { - gwc, err := a.gws.Next() - if err != nil { - return fmt.Errorf("cant get gateway client: %w", err) - } - - ctx, err := utils.GetServiceUserContext(a.cfg.ServiceAccount.ServiceAccountID, gwc, a.cfg.ServiceAccount.ServiceAccountSecret) - if err != nil { - return fmt.Errorf("cant get service user context: %w", err) - } - - // store activity on trashed item - if err := a.storeActivity(storagespace.FormatResourceID(resourceID), []RawActivity{ - { - EventID: eventID, - Depth: 0, - Timestamp: timestamp, - }, - }); err != nil { - return fmt.Errorf("could not store activity: %w", err) - } - - // get previous parent - ref := &provider.Reference{ - ResourceId: reference.GetResourceId(), - Path: filepath.Dir(reference.GetPath()), - } - - var span trace.Span - ctx, span = a.tracer.Start(ctx, "AddActivityTrashed") - defer span.End() - - return a.addActivity(ctx, ref, parentId, eventID, timestamp, func(ctx context.Context, ref *provider.Reference) (*provider.ResourceInfo, error) { - return utils.GetResource(ctx, ref, gwc) - }) -} - -// AddSpaceActivity adds the activity to the given spaceroot -func (a *ActivitylogService) AddSpaceActivity(spaceID *provider.StorageSpaceId, eventID string, timestamp time.Time) error { - // spaceID is in format $ - // activitylog service uses format $! - // lets do some converting, shall we? - rid, err := storagespace.ParseID(spaceID.GetOpaqueId()) - if err != nil { - return fmt.Errorf("could not parse space id: %w", err) - } - rid.OpaqueId = rid.GetSpaceId() - return a.storeActivity(storagespace.FormatResourceID(&rid), []RawActivity{ - { - EventID: eventID, - Depth: 0, - Timestamp: timestamp, - }, - }) - -} - -// Activities returns the activities for the given resource -func (a *ActivitylogService) Activities(rid *provider.ResourceId) ([]RawActivity, error) { - a.lock.RLock() - defer a.lock.RUnlock() - - return a.activities(rid) -} - -// RemoveActivities removes the activities from the given resource -func (a *ActivitylogService) RemoveActivities(rid *provider.ResourceId, toDelete map[string]struct{}) error { - a.lock.Lock() - defer a.lock.Unlock() - - curActivities, err := a.activities(rid) - if err != nil { - return err - } - - var acts []RawActivity - for _, a := range curActivities { - if _, ok := toDelete[a.EventID]; !ok { - acts = append(acts, a) - } - } - - b, err := json.Marshal(acts) - if err != nil { - return err - } - - _, err = a.natskv.Put(storagespace.FormatResourceID(rid), b) - return err -} - -// 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() - - return a.natskv.Delete(storagespace.FormatResourceID(rid)) -} - -func (a *ActivitylogService) activities(rid *provider.ResourceId) ([]RawActivity, error) { - resourceID := storagespace.FormatResourceID(rid) - - glob := fmt.Sprintf("%s.>", base32.StdEncoding.EncodeToString([]byte(resourceID))) - - watcher, err := a.natskv.Watch(glob, nats.IgnoreDeletes()) - if err != nil { - return nil, err - } - defer watcher.Stop() - - var activities []RawActivity - for update := range watcher.Updates() { - if update == nil { - break - } - - var batchActivities []RawActivity - if err := msgpack.Unmarshal(update.Value(), &batchActivities); err != nil { - a.log.Debug().Err(err).Str("resourceID", resourceID).Msg("could not unmarshal messagepack, trying json") - } - activities = append(activities, batchActivities...) - } - - return activities, nil -} - -// note: getResource is abstracted to allow unit testing, in general this will just be utils.GetResource -func (a *ActivitylogService) addActivity(ctx context.Context, initRef *provider.Reference, parentId *provider.ResourceId, eventID string, timestamp time.Time, getResource func(context.Context, *provider.Reference) (*provider.ResourceInfo, error)) error { - var ( - err error - depth int - ref = initRef - ) - ctx, span := a.tracer.Start(ctx, "addActivity") - defer span.End() - for { - var info *provider.ResourceInfo - id := ref.GetResourceId() - if ref.Path != "" { - // Path based reference, we need to resolve the resource id - ctx, span = a.tracer.Start(ctx, "addActivity.getResource") - info, err = getResource(ctx, ref) - span.End() - if err != nil { - return fmt.Errorf("could not get resource info: %w", err) - } - id = info.GetId() - } - if id == nil { - return fmt.Errorf("resource id is required") - } - - key := storagespace.FormatResourceID(id) - a.debouncer.Debounce(key, RawActivity{ - EventID: eventID, - Depth: depth, - Timestamp: timestamp, - }) - - if id.OpaqueId == id.SpaceId { - // we are at the root of the space, no need to go further - break - } - - // check if parent id is cached - // parent id is cached in the format $! - // if it is not cached, get the resource info and cache it - if parentId == nil { - if v, err := a.parentIdCache.Get(key); err != nil { - if info == nil { - ctx, span := a.tracer.Start(ctx, "addActivity.getResource parent") - info, err = getResource(ctx, ref) - span.End() - if err != nil || info.GetParentId() == nil || info.GetParentId().GetOpaqueId() == "" { - return fmt.Errorf("could not get parent id: %w", err) - } - } - parentId = info.GetParentId() - a.parentIdCache.Set(key, parentId) - } else { - parentId = v.(*provider.ResourceId) - } - } else { - a.log.Debug().Msg("parent id is cached") - } - - depth++ - ref = &provider.Reference{ResourceId: parentId} - parentId = nil // reset parent id so it's not reused in the next iteration - } - - return nil -} - -func (a *ActivitylogService) storeActivity(resourceID string, activities []RawActivity) error { - a.lock.Lock() - defer a.lock.Unlock() - - ctx, span := a.tracer.Start(context.Background(), "storeActivity") - defer span.End() - - _, subspan := a.tracer.Start(ctx, "storeActivity.Marshal") - b, err := msgpack.Marshal(activities) - if err != nil { - return err - } - subspan.End() - - _, subspan = a.tracer.Start(ctx, "storeActivity.natskv.Put") - key := natsKey(resourceID, len(activities)) - _, err = a.natskv.Put(key, b) - if err != nil { - return err - } - subspan.End() - - ctx, subspan = a.tracer.Start(ctx, "storeActivity.enforceMaxActivities") - a.enforceMaxActivities(ctx, resourceID) - subspan.End() - return nil -} - -func (a *ActivitylogService) enforceMaxActivities(ctx context.Context, resourceID string) { - if a.maxActivities <= 0 { - return - } - - key := fmt.Sprintf("%s.>", base32.StdEncoding.EncodeToString([]byte(resourceID))) - - _, subspan := a.tracer.Start(ctx, "enforceMaxActivities.watch") - watcher, err := a.natskv.Watch(key, nats.IgnoreDeletes()) - if err != nil { - a.log.Error().Err(err).Str("resourceID", resourceID).Msg("could not watch") - return - } - defer watcher.Stop() - - var keys []string - for update := range watcher.Updates() { - if update == nil { - break - } - - var batchActivities []RawActivity - if err := msgpack.Unmarshal(update.Value(), &batchActivities); err != nil { - a.log.Debug().Err(err).Str("resourceID", resourceID).Msg("could not unmarshal messagepack, trying json") - } - keys = append(keys, update.Key()) - } - subspan.End() - - _, subspan = a.tracer.Start(ctx, "enforceMaxActivities.compile") - // Parse keys into batches - batches := make([]batchInfo, 0) - var activitiesCount int - for _, k := range keys { - parts := strings.SplitN(k, ".", 3) - if len(parts) < 3 { - a.log.Warn().Str("key", k).Msg("skipping key, not enough parts") - continue - } - - c, err := strconv.Atoi(parts[1]) - if err != nil { - a.log.Warn().Str("key", k).Msg("skipping key, can not parse count") - continue - } - - // parse timestamp - nano, err := strconv.ParseInt(parts[2], 10, 64) - if err != nil { - a.log.Warn().Str("key", k).Msg("skipping key, can not parse timestamp") - continue - } - - batches = append(batches, batchInfo{ - key: k, - count: c, - timestamp: time.Unix(0, nano), - }) - activitiesCount += c - } - - // sort batches by timestamp - sort.Slice(batches, func(i, j int) bool { - return batches[i].timestamp.Before(batches[j].timestamp) - }) - subspan.End() - - _, subspan = a.tracer.Start(ctx, "enforceMaxActivities.delete") - // remove oldest keys until we are at max activities - for _, b := range batches { - if activitiesCount-b.count < a.maxActivities { - break - } - - activitiesCount -= b.count - err = a.natskv.Delete(b.key) - if err != nil { - a.log.Error().Err(err).Str("key", b.key).Msg("could not delete key") - break - } - } - subspan.End() -} - -func toRef(r *provider.ResourceId) *provider.Reference { - return &provider.Reference{ - ResourceId: r, - } -} - -func toSpace(r *provider.Reference) *provider.StorageSpaceId { - return &provider.StorageSpaceId{ - OpaqueId: storagespace.FormatStorageID(r.GetResourceId().GetStorageId(), r.GetResourceId().GetSpaceId()), - } -} - -func (a *ActivitylogService) removeCachedParentID(ref *provider.Reference) { - purgeId := ref.GetResourceId() - if ref.GetPath() != "" { - gwc, err := a.gws.Next() - if err != nil { - a.log.Error().Err(err).Msg("could not get gateway client") - return - } - - ctx, err := utils.GetServiceUserContext(a.cfg.ServiceAccount.ServiceAccountID, gwc, a.cfg.ServiceAccount.ServiceAccountSecret) - if err != nil { - a.log.Error().Err(err).Msg("could not get service user context") - return - } - - info, err := utils.GetResource(ctx, ref, gwc) - if err != nil { - a.log.Error().Err(err).Msg("could not get resource info") - return - } - purgeId = info.GetId() - } - if err := a.parentIdCache.Remove(storagespace.FormatResourceID(purgeId)); err != nil { - a.log.Error().Interface("event", ref).Err(err).Msg("could not delete parent id cache") - } -} - -func natsKey(resourceID string, activitiesCount int) string { - return fmt.Sprintf("%s.%d.%d", - base32.StdEncoding.EncodeToString([]byte(resourceID)), - activitiesCount, - time.Now().UnixNano()) -} diff --git a/services/activitylog/pkg/service/events/debouncer.go b/services/activitylog/pkg/service/events/debouncer.go new file mode 100644 index 0000000000..b5cfb81410 --- /dev/null +++ b/services/activitylog/pkg/service/events/debouncer.go @@ -0,0 +1,77 @@ +package events + +import ( + "sync" + "time" + + "github.com/opencloud-eu/opencloud/services/activitylog/pkg/data" +) + +type Debouncer struct { + after time.Duration + f func(id string, ra []data.RawActivity) error + pending sync.Map + inProgress sync.Map + + mutex sync.Mutex +} + +type queueItem struct { + activities []data.RawActivity + timer *time.Timer +} + +// NewDebouncer returns a new Debouncer instance +func NewDebouncer(d time.Duration, f func(id string, ra []data.RawActivity) error) *Debouncer { + return &Debouncer{ + after: d, + f: f, + pending: sync.Map{}, + inProgress: sync.Map{}, + } +} + +// Debounce restarts the debounce timer for the given space +func (d *Debouncer) Debounce(id string, ra data.RawActivity) { + if d.after == 0 { + d.f(id, []data.RawActivity{ra}) + return + } + + d.mutex.Lock() + defer d.mutex.Unlock() + + activities := []data.RawActivity{ra} + item := &queueItem{ + activities: activities, + } + if i, ok := d.pending.Load(id); ok { + // if the item is already in the queue, append the new activities + item, ok = i.(*queueItem) + if ok { + item.activities = append(item.activities, ra) + } + } + + if item.timer == nil { + item.timer = time.AfterFunc(d.after, func() { + if _, ok := d.inProgress.Load(id); ok { + // Reschedule this run for when the previous run has finished + d.mutex.Lock() + if i, ok := d.pending.Load(id); ok { + i.(*queueItem).timer.Reset(d.after) + } + + d.mutex.Unlock() + return + } + + d.pending.Delete(id) + d.inProgress.Store(id, true) + defer d.inProgress.Delete(id) + d.f(id, item.activities) + }) + } + + d.pending.Store(id, item) +} diff --git a/services/activitylog/pkg/service/event/options.go b/services/activitylog/pkg/service/events/options.go similarity index 55% rename from services/activitylog/pkg/service/event/options.go rename to services/activitylog/pkg/service/events/options.go index 282b1147f1..cef20b44fa 100644 --- a/services/activitylog/pkg/service/event/options.go +++ b/services/activitylog/pkg/service/events/options.go @@ -1,17 +1,14 @@ -package service +package events import ( + "context" "time" gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" - "github.com/go-chi/chi/v5" "github.com/opencloud-eu/opencloud/pkg/log" - ehsvc "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/services/eventhistory/v0" - settingssvc "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/services/settings/v0" "github.com/opencloud-eu/opencloud/services/activitylog/pkg/config" "github.com/opencloud-eu/reva/v2/pkg/events" "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" - "go.opentelemetry.io/otel/trace" ) // Option for the activitylog service @@ -19,37 +16,33 @@ type Option func(*Options) // Options for the activitylog service type Options struct { + Context context.Context Logger log.Logger - Config *config.Config - TraceProvider trace.TracerProvider + ServiceAccount config.ServiceAccount Stream events.Stream RegisteredEvents []events.Unmarshaller GatewaySelector pool.Selectable[gateway.GatewayAPIClient] - Mux *chi.Mux - HistoryClient ehsvc.EventHistoryService - ValueClient settingssvc.ValueService WriteBufferDuration time.Duration - MaxActivities int + NumConsumers int } -// Logger configures a logger for the activitylog service -func Logger(log log.Logger) Option { +func Context(ctx context.Context) Option { return func(o *Options) { - o.Logger = log + o.Context = ctx } } -// Config adds the config for the activitylog service -func Config(c *config.Config) Option { +// Logger configures a logger for the activitylog service +func Logger(log log.Logger) Option { return func(o *Options) { - o.Config = c + o.Logger = log } } -// TraceProvider adds a tracer provider for the activitylog service -func TraceProvider(tp trace.TracerProvider) Option { +// ServiceAccount configures a service account for the activitylog service +func ServiceAccount(sa config.ServiceAccount) Option { return func(o *Options) { - o.TraceProvider = tp + o.ServiceAccount = sa } } @@ -74,23 +67,8 @@ func GatewaySelector(gatewaySelector pool.Selectable[gateway.GatewayAPIClient]) } } -// Mux defines the muxer for the service -func Mux(m *chi.Mux) Option { - return func(o *Options) { - o.Mux = m - } -} - -// HistoryClient adds a grpc client for the eventhistory service -func HistoryClient(hc ehsvc.EventHistoryService) Option { - return func(o *Options) { - o.HistoryClient = hc - } -} - -// ValueClient adds a grpc client for the value service -func ValueClient(vs settingssvc.ValueService) Option { +func NumConsumers(num int) Option { return func(o *Options) { - o.ValueClient = vs + o.NumConsumers = num } } diff --git a/services/activitylog/pkg/service/events/service.go b/services/activitylog/pkg/service/events/service.go new file mode 100644 index 0000000000..41f8517571 --- /dev/null +++ b/services/activitylog/pkg/service/events/service.go @@ -0,0 +1,312 @@ +package events + +import ( + "context" + "fmt" + "path/filepath" + "sync" + "sync/atomic" + "time" + + gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" + provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" + "github.com/opencloud-eu/opencloud/pkg/log" + "github.com/opencloud-eu/opencloud/services/activitylog/pkg/config" + "github.com/opencloud-eu/opencloud/services/activitylog/pkg/data" + "github.com/opencloud-eu/opencloud/services/activitylog/pkg/service/activitylog" + "github.com/opencloud-eu/reva/v2/pkg/events" + "github.com/opencloud-eu/reva/v2/pkg/events/raw" + "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" + "github.com/opencloud-eu/reva/v2/pkg/storagespace" + "github.com/opencloud-eu/reva/v2/pkg/utils" + "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/trace" +) + +var tracer trace.Tracer + +func init() { + tracer = otel.Tracer("github.com/opencloud-eu/opencloud/services/activitylog/pkg/service/events") +} + +var ( + _numConsumersDefault = 1 +) + +// ActivitylogService logs events per resource +type ActivitylogService struct { + ctx context.Context + sa config.ServiceAccount + log log.Logger + stream raw.Stream + gws pool.Selectable[gateway.GatewayAPIClient] + debouncer *Debouncer + al *activitylog.ActivityLog + + numConsumers int + + events []events.Unmarshaller + + stopCh chan struct{} + stopped *atomic.Bool +} + +// New creates a new ActivitylogService +func New(al *activitylog.ActivityLog, stream raw.Stream, opts ...Option) (*ActivitylogService, error) { + o := &Options{ + NumConsumers: _numConsumersDefault, + } + for _, opt := range opts { + opt(o) + } + + s := &ActivitylogService{ + ctx: o.Context, + log: o.Logger, + sa: o.ServiceAccount, + stream: stream, + gws: o.GatewaySelector, + events: o.RegisteredEvents, + numConsumers: o.NumConsumers, + al: al, + stopCh: make(chan struct{}, 1), + stopped: new(atomic.Bool), + } + + return s, nil +} + +// Run to fulfil Runner interface +func (s *ActivitylogService) Run() error { + ch, err := s.stream.Consume("activitylog-pull", s.events...) + if err != nil { + return err + } + + // if s.m != nil { + // monitorMetrics(s.ctx, s.stream, "activitylog", s.m, s.log) + // } + + var wg sync.WaitGroup + ctx, cancel := context.WithCancel(s.ctx) + defer cancel() + + s.log.Debug().Int("worker.count", s.numConsumers). + Str("messaging.consumer.group.name", "activitylog"). + Str("messaging.system", "nats"). + Str("messaging.operation.name", "receive"). + Msg("starting event processing workers") + + // start workers + for i := 0; i < s.numConsumers; i++ { + wg.Add(1) + go func(workerID int) { + defer wg.Done() + for { + select { + case <-ctx.Done(): + return + case e, ok := <-ch: + if !ok { + return + } + if err := s.processEvent(e); err != nil { + s.log.Error().Err(err). + Int("worker", workerID). + Interface("event", e). + Msg("failed to process event") + } + } + } + }(i) + } + + // wait for stop signal + <-s.stopCh + cancel() // signal workers to stop + wg.Wait() + + return nil +} + +// Close will make the service to stop processing, so the `Run` +// method can finish. +// TODO: Underlying services can't be stopped. This means that some goroutines +// will get stuck trying to push events through a channel nobody is reading +// from, so resources won't be freed and there will be memory leaks. For now, +// if the service is stopped, you should close the app soon after. +func (s *ActivitylogService) Close() { + if s.stopped.CompareAndSwap(false, true) { + close(s.stopCh) + } +} + +// Run runs the service +func (s *ActivitylogService) processEvent(e raw.Event) error { + ctx := e.GetTraceContext(s.ctx) + ctx, span := tracer.Start(ctx, "processEvent") + defer span.End() + + e.InProgress() // let nats know that we are processing this event + s.log.Debug().Interface("event", e).Msg("updating activitylog") + + switch ev := e.Event.Event.(type) { + case events.UploadReady: + return s.AddActivity(ctx, ev.FileRef, ev.ParentID, e.ID, utils.TSToTime(ev.Timestamp)) + case events.FileTouched: + return s.AddActivity(ctx, ev.Ref, ev.ParentID, e.ID, utils.TSToTime(ev.Timestamp)) + // Disabled https://github.com/owncloud/ocis/issues/10293 + //case events.FileDownloaded: + // we are only interested in public link downloads - so no need to store others. + //if ev.ImpersonatingUser.GetDisplayName() == "Public" { + // err = a.AddActivity(ev.Ref, e.ID, utils.TSToTime(ev.Timestamp)) + //} + case events.ContainerCreated: + return s.AddActivity(ctx, ev.Ref, ev.ParentID, e.ID, utils.TSToTime(ev.Timestamp)) + case events.ItemTrashed: + return s.AddActivityTrashed(ctx, ev.ID, ev.Ref, nil, e.ID, utils.TSToTime(ev.Timestamp)) + case events.ItemPurged: + return s.al.RemoveResource(ev.ID) + case events.ItemMoved: + // remove the cached parent id for this resource + s.removeCachedParentID(ctx, ev.Ref) + + return s.AddActivity(ctx, ev.Ref, nil, e.ID, utils.TSToTime(ev.Timestamp)) + case events.ShareCreated: + return s.AddActivity(ctx, toRef(ev.ItemID), nil, e.ID, utils.TSToTime(ev.CTime)) + case events.ShareUpdated: + if ev.Sharer != nil && ev.ItemID != nil && ev.Sharer.GetOpaqueId() != ev.ItemID.GetSpaceId() { + return s.AddActivity(ctx, toRef(ev.ItemID), nil, e.ID, utils.TSToTime(ev.MTime)) + } + case events.ShareRemoved: + return s.AddActivity(ctx, toRef(ev.ItemID), nil, e.ID, ev.Timestamp) + case events.LinkCreated: + return s.AddActivity(ctx, toRef(ev.ItemID), nil, e.ID, utils.TSToTime(ev.CTime)) + case events.LinkUpdated: + if ev.Sharer != nil && ev.ItemID != nil && ev.Sharer.GetOpaqueId() != ev.ItemID.GetSpaceId() { + return s.AddActivity(ctx, toRef(ev.ItemID), nil, e.ID, utils.TSToTime(ev.MTime)) + } + case events.LinkRemoved: + return s.AddActivity(ctx, toRef(ev.ItemID), nil, e.ID, utils.TSToTime(ev.Timestamp)) + case events.SpaceShared: + return s.AddSpaceActivity(ctx, ev.ID, e.ID, ev.Timestamp) + case events.SpaceUnshared: + return s.AddSpaceActivity(ctx, ev.ID, e.ID, ev.Timestamp) + } + + return nil +} + +// AddActivity adds the activity to the given resource and all its parents +func (a *ActivitylogService) AddActivity(ctx context.Context, initRef *provider.Reference, parentId *provider.ResourceId, eventID string, timestamp time.Time) error { + ctx, span := tracer.Start(ctx, "AddActivity") + defer span.End() + + gwc, err := a.gws.Next() + if err != nil { + return fmt.Errorf("cant get gateway client: %w", err) + } + + ctx, err = utils.GetServiceUserContextWithContext(ctx, gwc, a.sa.ServiceAccountID, a.sa.ServiceAccountSecret) + if err != nil { + return fmt.Errorf("cant get service user context: %w", err) + + } + return a.al.AddActivity(ctx, initRef, parentId, eventID, timestamp, func(ctx context.Context, ref *provider.Reference) (*provider.ResourceInfo, error) { + return utils.GetResource(ctx, ref, gwc) + }) +} + +// AddActivityTrashed adds the activity to given trashed resource and all its former parents +func (a *ActivitylogService) AddActivityTrashed(ctx context.Context, resourceID *provider.ResourceId, reference *provider.Reference, parentId *provider.ResourceId, eventID string, timestamp time.Time) error { + ctx, span := tracer.Start(ctx, "AddActivityTrashed") + defer span.End() + + gwc, err := a.gws.Next() + if err != nil { + return fmt.Errorf("cant get gateway client: %w", err) + } + + ctx, err = utils.GetServiceUserContextWithContext(ctx, gwc, a.sa.ServiceAccountID, a.sa.ServiceAccountSecret) + if err != nil { + return fmt.Errorf("cant get service user context: %w", err) + } + + // store activity on trashed item + if err := a.al.StoreActivity(storagespace.FormatResourceID(resourceID), []data.RawActivity{ + { + EventID: eventID, + Depth: 0, + Timestamp: timestamp, + }, + }); err != nil { + return fmt.Errorf("could not store activity: %w", err) + } + + // get previous parent + ref := &provider.Reference{ + ResourceId: reference.GetResourceId(), + Path: filepath.Dir(reference.GetPath()), + } + + return a.al.AddActivity(ctx, ref, parentId, eventID, timestamp, func(ctx context.Context, ref *provider.Reference) (*provider.ResourceInfo, error) { + return utils.GetResource(ctx, ref, gwc) + }) +} + +// AddSpaceActivity adds the activity to the given spaceroot +func (a *ActivitylogService) AddSpaceActivity(ctx context.Context, spaceID *provider.StorageSpaceId, eventID string, timestamp time.Time) error { + _, span := tracer.Start(ctx, "AddSpaceActivity") + defer span.End() + // spaceID is in format $ + // activitylog service uses format $! + // lets do some converting, shall we? + rid, err := storagespace.ParseID(spaceID.GetOpaqueId()) + if err != nil { + return fmt.Errorf("could not parse space id: %w", err) + } + rid.OpaqueId = rid.GetSpaceId() + return a.al.StoreActivity(storagespace.FormatResourceID(&rid), []data.RawActivity{ + { + EventID: eventID, + Depth: 0, + Timestamp: timestamp, + }, + }) + +} + +func toRef(r *provider.ResourceId) *provider.Reference { + return &provider.Reference{ + ResourceId: r, + } +} + +func (a *ActivitylogService) removeCachedParentID(ctx context.Context, ref *provider.Reference) { + var span trace.Span + ctx, span = tracer.Start(ctx, "removeCachedParentID") + defer span.End() + + purgeId := ref.GetResourceId() + if ref.GetPath() != "" { + gwc, err := a.gws.Next() + if err != nil { + a.log.Error().Err(err).Msg("could not get gateway client") + return + } + + ctx, err = utils.GetServiceUserContextWithContext(ctx, gwc, a.sa.ServiceAccountID, a.sa.ServiceAccountSecret) + if err != nil { + a.log.Error().Err(err).Msg("could not get service user context") + return + } + + info, err := utils.GetResource(ctx, ref, gwc) + if err != nil { + a.log.Error().Err(err).Msg("could not get resource info") + return + } + purgeId = info.GetId() + } + a.al.InvalidateCachedParentID(purgeId) +} diff --git a/services/activitylog/pkg/service/http/option.go b/services/activitylog/pkg/service/http/option.go new file mode 100644 index 0000000000..0f61d59cf7 --- /dev/null +++ b/services/activitylog/pkg/service/http/option.go @@ -0,0 +1,59 @@ +package http + +import ( + gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" + "github.com/opencloud-eu/opencloud/pkg/log" + ehsvc "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/services/eventhistory/v0" + "github.com/opencloud-eu/reva/v2/pkg/events" + "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" +) + +// Option defines a single option function. +type Option func(o *Options) + +// Options defines the available options for this package. +type Options struct { + Logger log.Logger + RegisteredEvents []events.Unmarshaller + GatewaySelector pool.Selectable[gateway.GatewayAPIClient] + HistoryClient ehsvc.EventHistoryService +} + +// newOptions initializes the available default options. +func newOptions(opts ...Option) Options { + opt := Options{} + + for _, o := range opts { + o(&opt) + } + + return opt +} + +// Logger provides a function to set the logger option. +func Logger(val log.Logger) Option { + return func(o *Options) { + o.Logger = val + } +} + +// RegisteredEvents registers the events the service should listen to +func RegisteredEvents(e []events.Unmarshaller) Option { + return func(o *Options) { + o.RegisteredEvents = e + } +} + +// GatewaySelector adds a grpc client selector for the gateway service +func GatewaySelector(gatewaySelector pool.Selectable[gateway.GatewayAPIClient]) Option { + return func(o *Options) { + o.GatewaySelector = gatewaySelector + } +} + +// HistoryClient adds a grpc client for the eventhistory service +func HistoryClient(hc ehsvc.EventHistoryService) Option { + return func(o *Options) { + o.HistoryClient = hc + } +} diff --git a/services/activitylog/pkg/service/event/response.go b/services/activitylog/pkg/service/http/response.go similarity index 96% rename from services/activitylog/pkg/service/event/response.go rename to services/activitylog/pkg/service/http/response.go index 8867770d34..0a4ad62090 100644 --- a/services/activitylog/pkg/service/event/response.go +++ b/services/activitylog/pkg/service/http/response.go @@ -1,4 +1,4 @@ -package service +package http import ( "context" @@ -44,11 +44,6 @@ var ( StrDescription = l10n.Template("description") ) -// GetActivitiesResponse is the response on GET activities requests -type GetActivitiesResponse struct { - Activities []libregraph.Activity `json:"value"` -} - // Resource represents an item such as a file or folder type Resource struct { ID string `json:"id"` @@ -311,7 +306,7 @@ func NewActivity(message string, ts time.Time, eventID string, vars map[string]a } // GetVars calls other service to gather the required data for the activity variables -func (s *ActivitylogService) GetVars(ctx context.Context, opts ...ActivityOption) (map[string]any, error) { +func (s *svc) GetVars(ctx context.Context, opts ...ActivityOption) (map[string]any, error) { gwc, err := s.gws.Next() if err != nil { return nil, err @@ -327,6 +322,12 @@ func (s *ActivitylogService) GetVars(ctx context.Context, opts ...ActivityOption return vars, nil } +func toSpace(r *provider.Reference) *provider.StorageSpaceId { + return &provider.StorageSpaceId{ + OpaqueId: storagespace.FormatStorageID(r.GetResourceId().GetStorageId(), r.GetResourceId().GetSpaceId()), + } +} + func getFolderName(ctx context.Context, gwc gateway.GatewayAPIClient, ref *provider.Reference) string { n := filepath.Base(filepath.Dir(ref.GetPath())) if n == "." || n == "/" { diff --git a/services/activitylog/pkg/service/event/http.go b/services/activitylog/pkg/service/http/service.go similarity index 74% rename from services/activitylog/pkg/service/event/http.go rename to services/activitylog/pkg/service/http/service.go index 6b3c08f0f9..e86c74c148 100644 --- a/services/activitylog/pkg/service/event/http.go +++ b/services/activitylog/pkg/service/http/service.go @@ -1,89 +1,94 @@ -package service +package http import ( - "embed" - "encoding/json" - "errors" - "net/http" + "context" "path/filepath" + "reflect" "slices" "strconv" "strings" "time" + gateway "github.com/cs3org/go-cs3apis/cs3/gateway/v1beta1" provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" - revactx "github.com/opencloud-eu/reva/v2/pkg/ctx" - "github.com/opencloud-eu/reva/v2/pkg/events" - "github.com/opencloud-eu/reva/v2/pkg/storagespace" - "github.com/opencloud-eu/reva/v2/pkg/utils" - "google.golang.org/grpc/metadata" - + "github.com/olekukonko/errors" libregraph "github.com/opencloud-eu/libre-graph-api-go" "github.com/opencloud-eu/opencloud/pkg/ast" "github.com/opencloud-eu/opencloud/pkg/kql" "github.com/opencloud-eu/opencloud/pkg/l10n" + "github.com/opencloud-eu/opencloud/pkg/log" ehmsg "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/messages/eventhistory/v0" ehsvc "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/services/eventhistory/v0" + "github.com/opencloud-eu/opencloud/services/activitylog/pkg/apierrors" + "github.com/opencloud-eu/opencloud/services/activitylog/pkg/data" + "github.com/opencloud-eu/opencloud/services/activitylog/pkg/service/activitylog" + "github.com/opencloud-eu/reva/v2/pkg/events" + "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" + "github.com/opencloud-eu/reva/v2/pkg/storagespace" + "github.com/opencloud-eu/reva/v2/pkg/utils" + "go.opentelemetry.io/otel" + "go.opentelemetry.io/otel/trace" ) -var ( - //go:embed l10n/locale - _localeFS embed.FS - - // subfolder where the translation files are stored - _localeSubPath = "l10n/locale" - - // domain of the activitylog service (transifex) - _domain = "activitylog" -) +var tracer trace.Tracer -// ServeHTTP implements the http.Handler interface. -func (s *ActivitylogService) ServeHTTP(w http.ResponseWriter, r *http.Request) { - s.mux.ServeHTTP(w, r) +func init() { + tracer = otel.Tracer("github.com/opencloud-eu/opencloud/services/activitylog/pkg/service/http") } -// HandleGetItemActivities handles the request to get the activities of an item. -func (s *ActivitylogService) HandleGetItemActivities(w http.ResponseWriter, r *http.Request) { - ctx := r.Context() - ctx = metadata.AppendToOutgoingContext(ctx, revactx.TokenHeader, r.Header.Get(revactx.TokenHeader)) +// New returns a new instance of Service +func New(al *activitylog.ActivityLog, opts ...Option) (*svc, error) { + o := newOptions(opts...) - activeUser, ok := revactx.ContextGetUser(ctx) - if !ok { - w.WriteHeader(http.StatusUnauthorized) - return + registeredEvents := make(map[string]events.Unmarshaller) + for _, e := range o.RegisteredEvents { + typ := reflect.TypeOf(e) + registeredEvents[typ.String()] = e } + return &svc{ + log: o.Logger, + evHistory: o.HistoryClient, + al: al, + registeredEvents: registeredEvents, + gws: o.GatewaySelector, + }, nil +} + +type svc struct { + log log.Logger + evHistory ehsvc.EventHistoryService + gws pool.Selectable[gateway.GatewayAPIClient] + al *activitylog.ActivityLog + registeredEvents map[string]events.Unmarshaller +} + +func (s *svc) GetItemActivities(ctx context.Context, query, loc string, t l10n.Translator) ([]libregraph.Activity, error) { gwc, err := s.gws.Next() if err != nil { - w.WriteHeader(http.StatusInternalServerError) - return + return nil, err } - rid, limit, rawActivityAccepted, activityAccepted, sort, err := s.getFilters(r.URL.Query().Get("kql")) + rid, limit, rawActivityAccepted, activityAccepted, sort, err := s.getFilters(query) if err != nil { - s.log.Info().Str("query", r.URL.Query().Get("kql")).Err(err).Msg("error getting filters") - _, _ = w.Write([]byte(err.Error())) - w.WriteHeader(http.StatusBadRequest) - return + s.log.Info().Str("query", query).Err(err).Msg("error getting filters") + return nil, apierrors.ErrBadRequest } info, err := utils.GetResourceByID(ctx, rid, gwc) if err != nil { - w.WriteHeader(http.StatusForbidden) - return + return nil, apierrors.ErrForbidden } // you need ListGrants to see activities if !info.GetPermissionSet().GetListGrants() { - w.WriteHeader(http.StatusForbidden) - return + return nil, apierrors.ErrForbidden } - raw, err := s.Activities(rid) + raw, err := s.al.Activities(rid) if err != nil { s.log.Error().Err(err).Msg("error getting activities") - w.WriteHeader(http.StatusInternalServerError) - return + return nil, err } ids := make([]string, 0, len(raw)) @@ -96,21 +101,21 @@ func (s *ActivitylogService) HandleGetItemActivities(w http.ResponseWriter, r *h toDelete[a.EventID] = struct{}{} } - evRes, err := s.evHistory.GetEvents(r.Context(), &ehsvc.GetEventsRequest{Ids: ids}) + evRes, err := s.evHistory.GetEvents(ctx, &ehsvc.GetEventsRequest{Ids: ids}) if err != nil { s.log.Error().Err(err).Msg("error getting events") - w.WriteHeader(http.StatusInternalServerError) - return + return nil, err } evs := evRes.GetEvents() sort(evs) - resp := GetActivitiesResponse{Activities: make([]libregraph.Activity, 0, len(evRes.GetEvents()))} + // TODO cut the interface here? + activities := make([]libregraph.Activity, 0, len(evRes.GetEvents())) for _, e := range evs { delete(toDelete, e.GetId()) - if limit > 0 && limit <= len(resp.Activities) { + if limit > 0 && limit <= len(activities) { continue } @@ -124,9 +129,6 @@ func (s *ActivitylogService) HandleGetItemActivities(w http.ResponseWriter, r *h vars map[string]any ) - loc := l10n.MustGetUserLocale(r.Context(), activeUser.GetId().GetOpaqueId(), r.Header.Get(l10n.HeaderAcceptLanguage), s.valService) - t := l10n.NewTranslatorFromCommonConfig(s.cfg.DefaultLanguage, _domain, s.cfg.TranslationPath, _localeFS, _localeSubPath) - switch ev := s.unwrapEvent(e).(type) { case nil: // error already logged in unwrapEvent @@ -224,35 +226,29 @@ func (s *ActivitylogService) HandleGetItemActivities(w http.ResponseWriter, r *h continue } - resp.Activities = append(resp.Activities, NewActivity(t.Translate(message, loc), ts, e.GetId(), vars)) + activities = append(activities, NewActivity(t.Translate(message, loc), ts, e.GetId(), vars)) } // delete activities in separate go routine if len(toDelete) > 0 { go func() { - err := s.RemoveActivities(rid, toDelete) + err := s.al.RemoveActivities(rid, toDelete) if err != nil { s.log.Error().Err(err).Msg("error removing activities") } }() } + return activities, nil - b, err := json.Marshal(resp) - if err != nil { - s.log.Error().Err(err).Msg("error marshalling activities") - w.WriteHeader(http.StatusInternalServerError) - return - } +} - if _, err := w.Write(b); err != nil { - s.log.Error().Err(err).Msg("error writing response") - w.WriteHeader(http.StatusInternalServerError) - return +func toRef(r *provider.ResourceId) *provider.Reference { + return &provider.Reference{ + ResourceId: r, } - w.WriteHeader(http.StatusOK) } -func (s *ActivitylogService) unwrapEvent(e *ehmsg.Event) any { +func (s *svc) unwrapEvent(e *ehmsg.Event) any { etype, ok := s.registeredEvents[e.GetType()] if !ok { s.log.Error().Str("eventid", e.GetId()).Str("eventtype", e.GetType()).Msg("event not registered") @@ -268,13 +264,13 @@ func (s *ActivitylogService) unwrapEvent(e *ehmsg.Event) any { return einterface } -func (s *ActivitylogService) getFilters(query string) (*provider.ResourceId, int, func(RawActivity) bool, func(*ehmsg.Event) bool, func([]*ehmsg.Event), error) { +func (s *svc) getFilters(query string) (*provider.ResourceId, int, func(data.RawActivity) bool, func(*ehmsg.Event) bool, func([]*ehmsg.Event), error) { qast, err := kql.Builder{}.Build(query) if err != nil { return nil, 0, nil, nil, nil, err } - prefilters := make([]func(RawActivity) bool, 0) + prefilters := make([]func(data.RawActivity) bool, 0) postfilters := make([]func(*ehmsg.Event) bool, 0) sortby := func(_ []*ehmsg.Event) {} @@ -299,7 +295,7 @@ func (s *ActivitylogService) getFilters(query string) (*provider.ResourceId, int break } - prefilters = append(prefilters, func(a RawActivity) bool { + prefilters = append(prefilters, func(a data.RawActivity) bool { return a.Depth <= depth }) case "limit": @@ -322,11 +318,11 @@ func (s *ActivitylogService) getFilters(query string) (*provider.ResourceId, int case *ast.DateTimeNode: switch v.Operator.Value { case "<", "<=": - prefilters = append(prefilters, func(a RawActivity) bool { + prefilters = append(prefilters, func(a data.RawActivity) bool { return a.Timestamp.Before(v.Value) }) case ">", ">=": - prefilters = append(prefilters, func(a RawActivity) bool { + prefilters = append(prefilters, func(a data.RawActivity) bool { return a.Timestamp.After(v.Value) }) } @@ -345,7 +341,7 @@ func (s *ActivitylogService) getFilters(query string) (*provider.ResourceId, int // space root requested - fix format rid.OpaqueId = rid.GetSpaceId() } - pref := func(a RawActivity) bool { + pref := func(a data.RawActivity) bool { for _, f := range prefilters { if !f(a) { return false diff --git a/services/webfinger/pkg/server/http/server.go b/services/webfinger/pkg/server/http/server.go index dbe91b0ca4..61a070ab00 100644 --- a/services/webfinger/pkg/server/http/server.go +++ b/services/webfinger/pkg/server/http/server.go @@ -60,7 +60,7 @@ func Server(opts ...Option) (ohttp.Service, error) { mux.Use(middleware.Version( options.Name, - version.String, + version.GetString(), )) mux.Use( From 0139a41402ddbf4ab3b0d5ca78baeaa09d0a6c5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Thu, 13 Aug 2026 12:09:56 +0200 Subject: [PATCH 03/11] add explicit ack MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../pkg/service/activitylog/activitylog.go | 23 ++++-- .../pkg/service/activitylog/debouncer.go | 14 +++- .../pkg/service/activitylog/service_test.go | 28 ++++--- .../pkg/service/events/debouncer.go | 77 ------------------ .../activitylog/pkg/service/events/service.go | 78 ++++++++++++------- 5 files changed, 95 insertions(+), 125 deletions(-) delete mode 100644 services/activitylog/pkg/service/events/debouncer.go diff --git a/services/activitylog/pkg/service/activitylog/activitylog.go b/services/activitylog/pkg/service/activitylog/activitylog.go index 6483eafabc..3eec740cd8 100644 --- a/services/activitylog/pkg/service/activitylog/activitylog.go +++ b/services/activitylog/pkg/service/activitylog/activitylog.go @@ -77,7 +77,7 @@ func New(kv nats.KeyValue, opts ...Option) (*ActivityLog, error) { maxActivities: o.MaxActivities, natskv: kv, } - s.debouncer = NewDebouncer(o.WriteBufferDuration, s.StoreActivity) + s.debouncer = NewDebouncer(o.Logger, o.WriteBufferDuration, s.StoreActivity) // run migrations err = s.runMigrations(context.Background(), kv) @@ -89,7 +89,7 @@ func New(kv nats.KeyValue, opts ...Option) (*ActivityLog, error) { } // RemoveResource removes the resource from the store -func (a *ActivityLog) RemoveResource(rid *provider.ResourceId) error { +func (a *ActivityLog) RemoveResource(rid *provider.ResourceId, ack func() error) error { if rid == nil { return fmt.Errorf("resource id is required") } @@ -97,10 +97,20 @@ func (a *ActivityLog) RemoveResource(rid *provider.ResourceId) error { a.lock.Lock() defer a.lock.Unlock() - return a.natskv.Delete(storagespace.FormatResourceID(rid)) + err := a.natskv.Delete(storagespace.FormatResourceID(rid)) + if err != nil { + return fmt.Errorf("could not delete resource %s: %w", rid.OpaqueId, err) + } + go func() { + err := ack() + if err != nil { + a.log.Error().Err(err).Msg("error while acknowledging resource removal") + } + }() + return nil } -func (a *ActivityLog) AddActivity(ctx context.Context, initRef *provider.Reference, parentId *provider.ResourceId, eventID string, timestamp time.Time, getResource func(context.Context, *provider.Reference) (*provider.ResourceInfo, error)) error { +func (a *ActivityLog) AddActivity(ctx context.Context, initRef *provider.Reference, parentId *provider.ResourceId, eventID string, timestamp time.Time, getResource func(context.Context, *provider.Reference) (*provider.ResourceInfo, error), ack func() error) error { var ( err error depth int @@ -117,7 +127,8 @@ func (a *ActivityLog) AddActivity(ctx context.Context, initRef *provider.Referen info, err = getResource(ctx, ref) span.End() if err != nil { - return fmt.Errorf("could not get resource info: %w", err) + // TODO If the resource was deleted should we still log an activity in the parent? + return fmt.Errorf("could not get resource info for reference %v: %w", ref, err) } id = info.GetId() } @@ -130,7 +141,7 @@ func (a *ActivityLog) AddActivity(ctx context.Context, initRef *provider.Referen EventID: eventID, Depth: depth, Timestamp: timestamp, - }) + }, ack) if id.OpaqueId == id.SpaceId { // we are at the root of the space, no need to go further diff --git a/services/activitylog/pkg/service/activitylog/debouncer.go b/services/activitylog/pkg/service/activitylog/debouncer.go index ed200c9b01..e3de92cb6c 100644 --- a/services/activitylog/pkg/service/activitylog/debouncer.go +++ b/services/activitylog/pkg/service/activitylog/debouncer.go @@ -4,6 +4,7 @@ import ( "sync" "time" + "github.com/opencloud-eu/opencloud/pkg/log" "github.com/opencloud-eu/opencloud/services/activitylog/pkg/data" ) @@ -12,6 +13,7 @@ type Debouncer struct { f func(id string, ra []data.RawActivity) error pending sync.Map inProgress sync.Map + log log.Logger mutex sync.Mutex } @@ -22,17 +24,18 @@ type queueItem struct { } // NewDebouncer returns a new Debouncer instance -func NewDebouncer(d time.Duration, f func(id string, ra []data.RawActivity) error) *Debouncer { +func NewDebouncer(log log.Logger, d time.Duration, f func(id string, ra []data.RawActivity) error) *Debouncer { return &Debouncer{ after: d, f: f, pending: sync.Map{}, inProgress: sync.Map{}, + log: log, } } // Debounce restarts the debounce timer for the given space -func (d *Debouncer) Debounce(id string, ra data.RawActivity) { +func (d *Debouncer) Debounce(id string, ra data.RawActivity, ack func() error) { if d.after == 0 { d.f(id, []data.RawActivity{ra}) return @@ -70,6 +73,13 @@ func (d *Debouncer) Debounce(id string, ra data.RawActivity) { d.inProgress.Store(id, true) defer d.inProgress.Delete(id) d.f(id, item.activities) + go func() { + if ack != nil { + if err := ack(); err != nil { + d.log.Error().Err(err).Msg("error while acknowledging event") + } + } + }() }) } diff --git a/services/activitylog/pkg/service/activitylog/service_test.go b/services/activitylog/pkg/service/activitylog/service_test.go index c968a935ec..5bc9a7e48c 100644 --- a/services/activitylog/pkg/service/activitylog/service_test.go +++ b/services/activitylog/pkg/service/activitylog/service_test.go @@ -34,6 +34,10 @@ func getFreeLocalhostPort() (int, error) { return port, nil } +func NoOpAck() error { + return nil +} + // Spawn a nats server and a JetStream instance for the duration of the test suite. // The different tests need to make sure to use different databases to avoid conflicts. var _ = SynchronizedBeforeSuite(func() { @@ -145,7 +149,7 @@ var _ = Describe("ActivitylogService", func() { } for k, v := range tc.Activities { - err := alog.AddActivity(context.Background(), reference(v), nil, k, time.Time{}, getResource) + err := alog.AddActivity(context.Background(), reference(v), nil, k, time.Time{}, getResource, NoOpAck) Expect(err).NotTo(HaveOccurred()) } }) @@ -184,9 +188,9 @@ var _ = Describe("ActivitylogService", func() { It("debounces activities", func() { - err := alog.AddActivity(context.Background(), reference("base"), nil, "activity1", time.Time{}, getResource) + err := alog.AddActivity(context.Background(), reference("base"), nil, "activity1", time.Time{}, getResource, NoOpAck) Expect(err).NotTo(HaveOccurred()) - err = alog.AddActivity(context.Background(), reference("base"), nil, "activity2", time.Time{}, getResource) + err = alog.AddActivity(context.Background(), reference("base"), nil, "activity2", time.Time{}, getResource, NoOpAck) Expect(err).NotTo(HaveOccurred()) Eventually(func(g Gomega) { @@ -197,7 +201,7 @@ var _ = Describe("ActivitylogService", func() { }) It("adheres to the MaxActivities setting", func() { - err := alog.AddActivity(context.Background(), reference("base"), nil, "activity1", time.Time{}, getResource) + err := alog.AddActivity(context.Background(), reference("base"), nil, "activity1", time.Time{}, getResource, NoOpAck) Expect(err).NotTo(HaveOccurred()) Eventually(func(g Gomega) { activities, err := alog.Activities(resourceID("base")) @@ -205,7 +209,7 @@ var _ = Describe("ActivitylogService", func() { g.Expect(len(activities)).To(Equal(1)) }).Should(Succeed()) - err = alog.AddActivity(context.Background(), reference("base"), nil, "activity2", time.Time{}, getResource) + err = alog.AddActivity(context.Background(), reference("base"), nil, "activity2", time.Time{}, getResource, NoOpAck) Expect(err).NotTo(HaveOccurred()) Eventually(func(g Gomega) { activities, err := alog.Activities(resourceID("base")) @@ -213,11 +217,11 @@ var _ = Describe("ActivitylogService", func() { g.Expect(len(activities)).To(Equal(2)) }).Should(Succeed()) - err = alog.AddActivity(context.Background(), reference("base"), nil, "activity3", time.Time{}, getResource) + err = alog.AddActivity(context.Background(), reference("base"), nil, "activity3", time.Time{}, getResource, NoOpAck) Expect(err).NotTo(HaveOccurred()) - err = alog.AddActivity(context.Background(), reference("base"), nil, "activity4", time.Time{}, getResource) + err = alog.AddActivity(context.Background(), reference("base"), nil, "activity4", time.Time{}, getResource, NoOpAck) Expect(err).NotTo(HaveOccurred()) - err = alog.AddActivity(context.Background(), reference("base"), nil, "activity5", time.Time{}, getResource) + err = alog.AddActivity(context.Background(), reference("base"), nil, "activity5", time.Time{}, getResource, NoOpAck) Expect(err).NotTo(HaveOccurred()) Eventually(func(g Gomega) { @@ -234,9 +238,9 @@ var _ = Describe("ActivitylogService", func() { return tree[ref.GetResourceId().GetOpaqueId()], nil } - err := alog.AddActivity(context.Background(), reference("base"), nil, "activity1", time.Time{}, getResource) + err := alog.AddActivity(context.Background(), reference("base"), nil, "activity1", time.Time{}, getResource, NoOpAck) Expect(err).NotTo(HaveOccurred()) - err = alog.AddActivity(context.Background(), reference("base"), nil, "activity2", time.Time{}, getResource) + err = alog.AddActivity(context.Background(), reference("base"), nil, "activity2", time.Time{}, getResource, NoOpAck) Expect(err).NotTo(HaveOccurred()) Eventually(func(g Gomega) { @@ -245,9 +249,9 @@ var _ = Describe("ActivitylogService", func() { g.Expect(activities).To(ConsistOf(activitites("activity1", 0, "activity2", 0))) }).Should(Succeed()) - err = alog.AddActivity(context.Background(), reference("base"), nil, "activity3", time.Time{}, getResource) + err = alog.AddActivity(context.Background(), reference("base"), nil, "activity3", time.Time{}, getResource, NoOpAck) Expect(err).NotTo(HaveOccurred()) - err = alog.AddActivity(context.Background(), reference("base"), nil, "activity4", time.Time{}, getResource) + err = alog.AddActivity(context.Background(), reference("base"), nil, "activity4", time.Time{}, getResource, NoOpAck) Expect(err).NotTo(HaveOccurred()) Eventually(func(g Gomega) { diff --git a/services/activitylog/pkg/service/events/debouncer.go b/services/activitylog/pkg/service/events/debouncer.go deleted file mode 100644 index b5cfb81410..0000000000 --- a/services/activitylog/pkg/service/events/debouncer.go +++ /dev/null @@ -1,77 +0,0 @@ -package events - -import ( - "sync" - "time" - - "github.com/opencloud-eu/opencloud/services/activitylog/pkg/data" -) - -type Debouncer struct { - after time.Duration - f func(id string, ra []data.RawActivity) error - pending sync.Map - inProgress sync.Map - - mutex sync.Mutex -} - -type queueItem struct { - activities []data.RawActivity - timer *time.Timer -} - -// NewDebouncer returns a new Debouncer instance -func NewDebouncer(d time.Duration, f func(id string, ra []data.RawActivity) error) *Debouncer { - return &Debouncer{ - after: d, - f: f, - pending: sync.Map{}, - inProgress: sync.Map{}, - } -} - -// Debounce restarts the debounce timer for the given space -func (d *Debouncer) Debounce(id string, ra data.RawActivity) { - if d.after == 0 { - d.f(id, []data.RawActivity{ra}) - return - } - - d.mutex.Lock() - defer d.mutex.Unlock() - - activities := []data.RawActivity{ra} - item := &queueItem{ - activities: activities, - } - if i, ok := d.pending.Load(id); ok { - // if the item is already in the queue, append the new activities - item, ok = i.(*queueItem) - if ok { - item.activities = append(item.activities, ra) - } - } - - if item.timer == nil { - item.timer = time.AfterFunc(d.after, func() { - if _, ok := d.inProgress.Load(id); ok { - // Reschedule this run for when the previous run has finished - d.mutex.Lock() - if i, ok := d.pending.Load(id); ok { - i.(*queueItem).timer.Reset(d.after) - } - - d.mutex.Unlock() - return - } - - d.pending.Delete(id) - d.inProgress.Store(id, true) - defer d.inProgress.Delete(id) - d.f(id, item.activities) - }) - } - - d.pending.Store(id, item) -} diff --git a/services/activitylog/pkg/service/events/service.go b/services/activitylog/pkg/service/events/service.go index 41f8517571..58347a4709 100644 --- a/services/activitylog/pkg/service/events/service.go +++ b/services/activitylog/pkg/service/events/service.go @@ -35,13 +35,12 @@ var ( // ActivitylogService logs events per resource type ActivitylogService struct { - ctx context.Context - sa config.ServiceAccount - log log.Logger - stream raw.Stream - gws pool.Selectable[gateway.GatewayAPIClient] - debouncer *Debouncer - al *activitylog.ActivityLog + ctx context.Context + sa config.ServiceAccount + log log.Logger + stream raw.Stream + gws pool.Selectable[gateway.GatewayAPIClient] + al *activitylog.ActivityLog numConsumers int @@ -152,9 +151,9 @@ func (s *ActivitylogService) processEvent(e raw.Event) error { switch ev := e.Event.Event.(type) { case events.UploadReady: - return s.AddActivity(ctx, ev.FileRef, ev.ParentID, e.ID, utils.TSToTime(ev.Timestamp)) + return s.AddActivity(ctx, ev.FileRef, ev.ParentID, e.ID, utils.TSToTime(ev.Timestamp), e.Ack) case events.FileTouched: - return s.AddActivity(ctx, ev.Ref, ev.ParentID, e.ID, utils.TSToTime(ev.Timestamp)) + return s.AddActivity(ctx, ev.Ref, ev.ParentID, e.ID, utils.TSToTime(ev.Timestamp), e.Ack) // Disabled https://github.com/owncloud/ocis/issues/10293 //case events.FileDownloaded: // we are only interested in public link downloads - so no need to store others. @@ -162,43 +161,57 @@ func (s *ActivitylogService) processEvent(e raw.Event) error { // err = a.AddActivity(ev.Ref, e.ID, utils.TSToTime(ev.Timestamp)) //} case events.ContainerCreated: - return s.AddActivity(ctx, ev.Ref, ev.ParentID, e.ID, utils.TSToTime(ev.Timestamp)) + return s.AddActivity(ctx, ev.Ref, ev.ParentID, e.ID, utils.TSToTime(ev.Timestamp), e.Ack) case events.ItemTrashed: - return s.AddActivityTrashed(ctx, ev.ID, ev.Ref, nil, e.ID, utils.TSToTime(ev.Timestamp)) + return s.AddActivityTrashed(ctx, ev.ID, ev.Ref, nil, e.ID, utils.TSToTime(ev.Timestamp), e.Ack) case events.ItemPurged: - return s.al.RemoveResource(ev.ID) + return s.al.RemoveResource(ev.ID, e.Ack) case events.ItemMoved: // remove the cached parent id for this resource s.removeCachedParentID(ctx, ev.Ref) - return s.AddActivity(ctx, ev.Ref, nil, e.ID, utils.TSToTime(ev.Timestamp)) + return s.AddActivity(ctx, ev.Ref, nil, e.ID, utils.TSToTime(ev.Timestamp), e.Ack) case events.ShareCreated: - return s.AddActivity(ctx, toRef(ev.ItemID), nil, e.ID, utils.TSToTime(ev.CTime)) + return s.AddActivity(ctx, toRef(ev.ItemID), nil, e.ID, utils.TSToTime(ev.CTime), e.Ack) case events.ShareUpdated: if ev.Sharer != nil && ev.ItemID != nil && ev.Sharer.GetOpaqueId() != ev.ItemID.GetSpaceId() { - return s.AddActivity(ctx, toRef(ev.ItemID), nil, e.ID, utils.TSToTime(ev.MTime)) + return s.AddActivity(ctx, toRef(ev.ItemID), nil, e.ID, utils.TSToTime(ev.MTime), e.Ack) + } else { + go func() { + err := e.Ack() + if err != nil { + s.log.Error().Err(err).Msg("error while acknowledging event") + } + }() } case events.ShareRemoved: - return s.AddActivity(ctx, toRef(ev.ItemID), nil, e.ID, ev.Timestamp) + return s.AddActivity(ctx, toRef(ev.ItemID), nil, e.ID, ev.Timestamp, e.Ack) case events.LinkCreated: - return s.AddActivity(ctx, toRef(ev.ItemID), nil, e.ID, utils.TSToTime(ev.CTime)) + return s.AddActivity(ctx, toRef(ev.ItemID), nil, e.ID, utils.TSToTime(ev.CTime), e.Ack) case events.LinkUpdated: if ev.Sharer != nil && ev.ItemID != nil && ev.Sharer.GetOpaqueId() != ev.ItemID.GetSpaceId() { - return s.AddActivity(ctx, toRef(ev.ItemID), nil, e.ID, utils.TSToTime(ev.MTime)) + return s.AddActivity(ctx, toRef(ev.ItemID), nil, e.ID, utils.TSToTime(ev.MTime), e.Ack) + } else { + go func() { + err := e.Ack() + if err != nil { + s.log.Error().Err(err).Msg("error while acknowledging event") + } + }() } case events.LinkRemoved: - return s.AddActivity(ctx, toRef(ev.ItemID), nil, e.ID, utils.TSToTime(ev.Timestamp)) + return s.AddActivity(ctx, toRef(ev.ItemID), nil, e.ID, utils.TSToTime(ev.Timestamp), e.Ack) case events.SpaceShared: - return s.AddSpaceActivity(ctx, ev.ID, e.ID, ev.Timestamp) + return s.AddSpaceActivity(ctx, ev.ID, e.ID, ev.Timestamp, e.Ack) case events.SpaceUnshared: - return s.AddSpaceActivity(ctx, ev.ID, e.ID, ev.Timestamp) + return s.AddSpaceActivity(ctx, ev.ID, e.ID, ev.Timestamp, e.Ack) } return nil } // AddActivity adds the activity to the given resource and all its parents -func (a *ActivitylogService) AddActivity(ctx context.Context, initRef *provider.Reference, parentId *provider.ResourceId, eventID string, timestamp time.Time) error { +func (a *ActivitylogService) AddActivity(ctx context.Context, initRef *provider.Reference, parentId *provider.ResourceId, eventID string, timestamp time.Time, ack func() error) error { ctx, span := tracer.Start(ctx, "AddActivity") defer span.End() @@ -214,11 +227,11 @@ func (a *ActivitylogService) AddActivity(ctx context.Context, initRef *provider. } return a.al.AddActivity(ctx, initRef, parentId, eventID, timestamp, func(ctx context.Context, ref *provider.Reference) (*provider.ResourceInfo, error) { return utils.GetResource(ctx, ref, gwc) - }) + }, ack) } // AddActivityTrashed adds the activity to given trashed resource and all its former parents -func (a *ActivitylogService) AddActivityTrashed(ctx context.Context, resourceID *provider.ResourceId, reference *provider.Reference, parentId *provider.ResourceId, eventID string, timestamp time.Time) error { +func (a *ActivitylogService) AddActivityTrashed(ctx context.Context, resourceID *provider.ResourceId, reference *provider.Reference, parentId *provider.ResourceId, eventID string, timestamp time.Time, ack func() error) error { ctx, span := tracer.Start(ctx, "AddActivityTrashed") defer span.End() @@ -251,11 +264,11 @@ func (a *ActivitylogService) AddActivityTrashed(ctx context.Context, resourceID return a.al.AddActivity(ctx, ref, parentId, eventID, timestamp, func(ctx context.Context, ref *provider.Reference) (*provider.ResourceInfo, error) { return utils.GetResource(ctx, ref, gwc) - }) + }, ack) } // AddSpaceActivity adds the activity to the given spaceroot -func (a *ActivitylogService) AddSpaceActivity(ctx context.Context, spaceID *provider.StorageSpaceId, eventID string, timestamp time.Time) error { +func (a *ActivitylogService) AddSpaceActivity(ctx context.Context, spaceID *provider.StorageSpaceId, eventID string, timestamp time.Time, ack func() error) error { _, span := tracer.Start(ctx, "AddSpaceActivity") defer span.End() // spaceID is in format $ @@ -266,14 +279,23 @@ func (a *ActivitylogService) AddSpaceActivity(ctx context.Context, spaceID *prov return fmt.Errorf("could not parse space id: %w", err) } rid.OpaqueId = rid.GetSpaceId() - return a.al.StoreActivity(storagespace.FormatResourceID(&rid), []data.RawActivity{ + err = a.al.StoreActivity(storagespace.FormatResourceID(&rid), []data.RawActivity{ { EventID: eventID, Depth: 0, Timestamp: timestamp, }, }) - + if err != nil { + return fmt.Errorf("could not store activity: %w", err) + } + go func() { + err := ack() + if err != nil { + a.log.Error().Err(err).Msg("error while acknowledging event") + } + }() + return nil } func toRef(r *provider.ResourceId) *provider.Reference { From 0fd6bc645858f70fdc0b6c96e4d9f2c60c474471 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Thu, 13 Aug 2026 12:16:52 +0200 Subject: [PATCH 04/11] add debouncer tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../pkg/service/activitylog/debouncer_test.go | 255 ++++++++++++++++++ 1 file changed, 255 insertions(+) create mode 100644 services/activitylog/pkg/service/activitylog/debouncer_test.go diff --git a/services/activitylog/pkg/service/activitylog/debouncer_test.go b/services/activitylog/pkg/service/activitylog/debouncer_test.go new file mode 100644 index 0000000000..671c39590a --- /dev/null +++ b/services/activitylog/pkg/service/activitylog/debouncer_test.go @@ -0,0 +1,255 @@ +package activitylog_test + +import ( + "fmt" + "sync" + "time" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/opencloud-eu/opencloud/pkg/log" + "github.com/opencloud-eu/opencloud/services/activitylog/pkg/data" + "github.com/opencloud-eu/opencloud/services/activitylog/pkg/service/activitylog" +) + +var _ = Describe("Debouncer", func() { + var ( + testlog = log.NopLogger() + callbacks []data.RawActivity + mu sync.Mutex + cbErr error + ackCalled int + ackMu sync.Mutex + ) + + newCallback := func(_ string, ras []data.RawActivity) error { + mu.Lock() + defer mu.Unlock() + callbacks = append(callbacks, ras...) + return cbErr + } + + newAck := func() error { + ackMu.Lock() + defer ackMu.Unlock() + ackCalled++ + return nil + } + + BeforeEach(func() { + callbacks = nil + cbErr = nil + ackMu.Lock() + ackCalled = 0 + ackMu.Unlock() + }) + + Context("with zero duration", func() { + It("calls the function immediately", func() { + d := activitylog.NewDebouncer(testlog, 0, newCallback) + + ra := data.RawActivity{EventID: "immediate"} + d.Debounce("space1", ra, nil) + + mu.Lock() + defer mu.Unlock() + Expect(callbacks).To(HaveLen(1)) + Expect(callbacks[0].EventID).To(Equal("immediate")) + }) + + It("calls the function immediately for each call without batching", func() { + d := activitylog.NewDebouncer(testlog, 0, newCallback) + + ra1 := data.RawActivity{EventID: "act1"} + ra2 := data.RawActivity{EventID: "act2"} + d.Debounce("space1", ra1, nil) + d.Debounce("space1", ra2, nil) + + mu.Lock() + defer mu.Unlock() + Expect(callbacks).To(HaveLen(2)) + }) + }) + + Context("with non-zero duration", func() { + It("batches activities with the same id", func() { + d := activitylog.NewDebouncer(testlog, 50*time.Millisecond, newCallback) + + ra1 := data.RawActivity{EventID: "act1"} + ra2 := data.RawActivity{EventID: "act2"} + ra3 := data.RawActivity{EventID: "act3"} + + d.Debounce("space1", ra1, nil) + d.Debounce("space1", ra2, nil) + d.Debounce("space1", ra3, nil) + + Eventually(func() int { + mu.Lock() + defer mu.Unlock() + return len(callbacks) + }).Should(Equal(3)) + + mu.Lock() + defer mu.Unlock() + Expect(callbacks[0].EventID).To(Equal("act1")) + Expect(callbacks[1].EventID).To(Equal("act2")) + Expect(callbacks[2].EventID).To(Equal("act3")) + }) + + It("handles different ids independently", func() { + d := activitylog.NewDebouncer(testlog, 50*time.Millisecond, newCallback) + + ra1 := data.RawActivity{EventID: "space1-act"} + ra2 := data.RawActivity{EventID: "space2-act"} + + d.Debounce("space1", ra1, nil) + d.Debounce("space2", ra2, nil) + + Eventually(func() int { + mu.Lock() + defer mu.Unlock() + return len(callbacks) + }).Should(Equal(2)) + }) + + It("calls the ack function after callback completes", func() { + d := activitylog.NewDebouncer(testlog, 50*time.Millisecond, newCallback) + + ra := data.RawActivity{EventID: "act1"} + d.Debounce("space1", ra, newAck) + + Eventually(func() int { + ackMu.Lock() + defer ackMu.Unlock() + return ackCalled + }).Should(Equal(1)) + }) + + It("does not panic when ack is nil", func() { + d := activitylog.NewDebouncer(testlog, 50*time.Millisecond, newCallback) + + ra := data.RawActivity{EventID: "act1"} + Expect(func() { d.Debounce("space1", ra, nil) }).ToNot(Panic()) + + Eventually(func() int { + mu.Lock() + defer mu.Unlock() + return len(callbacks) + }).Should(Equal(1)) + }) + + It("does not panic when ack returns an error", func() { + d := activitylog.NewDebouncer(testlog, 50*time.Millisecond, newCallback) + + errAck := func() error { return fmt.Errorf("ack error") } + ra := data.RawActivity{EventID: "act1"} + Expect(func() { d.Debounce("space1", ra, errAck) }).ToNot(Panic()) + + Eventually(func() int { + mu.Lock() + defer mu.Unlock() + return len(callbacks) + }).Should(Equal(1)) + }) + + It("batches activities that arrive within the debounce window", func() { + d := activitylog.NewDebouncer(testlog, 200*time.Millisecond, newCallback) + + ra1 := data.RawActivity{EventID: "act1"} + d.Debounce("space1", ra1, nil) + + time.Sleep(50 * time.Millisecond) + + ra2 := data.RawActivity{EventID: "act2"} + ra3 := data.RawActivity{EventID: "act3"} + d.Debounce("space1", ra2, nil) + d.Debounce("space1", ra3, nil) + + Eventually(func() int { + mu.Lock() + defer mu.Unlock() + return len(callbacks) + }).Should(Equal(3)) + }) + + It("processes new batch after previous completes", func() { + d := activitylog.NewDebouncer(testlog, 50*time.Millisecond, newCallback) + + // First batch + ra1 := data.RawActivity{EventID: "batch1-act1"} + ra2 := data.RawActivity{EventID: "batch1-act2"} + d.Debounce("space1", ra1, nil) + d.Debounce("space1", ra2, nil) + + // Wait for first batch to be processed + Eventually(func() int { + mu.Lock() + defer mu.Unlock() + return len(callbacks) + }).Should(Equal(2)) + + // Second batch + callbacks = nil + ra3 := data.RawActivity{EventID: "batch2-act1"} + d.Debounce("space1", ra3, nil) + + Eventually(func() int { + mu.Lock() + defer mu.Unlock() + return len(callbacks) + }).Should(Equal(1)) + mu.Lock() + Expect(callbacks[0].EventID).To(Equal("batch2-act1")) + mu.Unlock() + }) + + It("reschedules when timer fires during in-progress callback", func() { + unblockCh := make(chan struct{}) + + blockingCallback := func(_ string, ras []data.RawActivity) error { + mu.Lock() + callbacks = append(callbacks, ras...) + mu.Unlock() + + <-unblockCh + return nil + } + + d := activitylog.NewDebouncer(testlog, 50*time.Millisecond, blockingCallback) + + // First activity starts the timer + ra1 := data.RawActivity{EventID: "act1"} + d.Debounce("space1", ra1, nil) + + // Second activity gets queued while first is being processed + time.Sleep(60 * time.Millisecond) + // Timer fires for act1, callback starts (and blocks) + Eventually(func() int { + mu.Lock() + defer mu.Unlock() + return len(callbacks) + }, 1*time.Second).Should(Equal(1)) + + // Queue another activity while the first callback is still running + ra2 := data.RawActivity{EventID: "act2"} + d.Debounce("space1", ra2, nil) + + // Unblock the first callback + close(unblockCh) + + // The second activity should eventually be processed + Eventually(func() int { + mu.Lock() + defer mu.Unlock() + return len(callbacks) + }, 2*time.Second).Should(Equal(2)) + + mu.Lock() + defer mu.Unlock() + Expect(callbacks[0].EventID).To(Equal("act1")) + Expect(callbacks[1].EventID).To(Equal("act2")) + }) + }) +}) + + From dd1aa305de67a8b5cfade24bde8c1cd1505b324b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Thu, 13 Aug 2026 12:18:57 +0200 Subject: [PATCH 05/11] rename test files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../{service_suite_test.go => activitylog_suite_test.go} | 4 ++-- .../activitylog/{service_test.go => activitylog_test.go} | 0 2 files changed, 2 insertions(+), 2 deletions(-) rename services/activitylog/pkg/service/activitylog/{service_suite_test.go => activitylog_suite_test.go} (66%) rename services/activitylog/pkg/service/activitylog/{service_test.go => activitylog_test.go} (100%) diff --git a/services/activitylog/pkg/service/activitylog/service_suite_test.go b/services/activitylog/pkg/service/activitylog/activitylog_suite_test.go similarity index 66% rename from services/activitylog/pkg/service/activitylog/service_suite_test.go rename to services/activitylog/pkg/service/activitylog/activitylog_suite_test.go index 366053c0b4..dad20fe9c2 100644 --- a/services/activitylog/pkg/service/activitylog/service_suite_test.go +++ b/services/activitylog/pkg/service/activitylog/activitylog_suite_test.go @@ -7,7 +7,7 @@ import ( . "github.com/onsi/gomega" ) -func TestService(t *testing.T) { +func TestActivitylog(t *testing.T) { RegisterFailHandler(Fail) - RunSpecs(t, "Service Suite") + RunSpecs(t, "Activitylog Suite") } diff --git a/services/activitylog/pkg/service/activitylog/service_test.go b/services/activitylog/pkg/service/activitylog/activitylog_test.go similarity index 100% rename from services/activitylog/pkg/service/activitylog/service_test.go rename to services/activitylog/pkg/service/activitylog/activitylog_test.go From 9c9bf3a934fc0e1465f6d26ea746bf3328860678 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Thu, 13 Aug 2026 12:37:11 +0200 Subject: [PATCH 06/11] add tests to cover more ack scenarios MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../pkg/service/activitylog/debouncer_test.go | 183 ++++++++++++++++++ 1 file changed, 183 insertions(+) diff --git a/services/activitylog/pkg/service/activitylog/debouncer_test.go b/services/activitylog/pkg/service/activitylog/debouncer_test.go index 671c39590a..6305d51f80 100644 --- a/services/activitylog/pkg/service/activitylog/debouncer_test.go +++ b/services/activitylog/pkg/service/activitylog/debouncer_test.go @@ -12,6 +12,37 @@ import ( "github.com/opencloud-eu/opencloud/services/activitylog/pkg/service/activitylog" ) +// ackTracker tracks per-name ack invocations so we can assert each event's ack fires. +type ackTracker struct { + mu sync.Mutex + calls map[string]int + errors map[string]error +} + +func newAckTracker() *ackTracker { + return &ackTracker{ + calls: make(map[string]int), + errors: make(map[string]error), + } +} + +func (t *ackTracker) track(name string, fn func() error) func() error { + return func() error { + t.mu.Lock() + t.calls[name]++ + err := fn() + t.errors[name] = err + t.mu.Unlock() + return err + } +} + +func (t *ackTracker) count(name string) int { + t.mu.Lock() + defer t.mu.Unlock() + return t.calls[name] +} + var _ = Describe("Debouncer", func() { var ( testlog = log.NopLogger() @@ -249,6 +280,158 @@ var _ = Describe("Debouncer", func() { Expect(callbacks[0].EventID).To(Equal("act1")) Expect(callbacks[1].EventID).To(Equal("act2")) }) + + It("calls ack even when callback returns an error", func() { + cbErr = fmt.Errorf("store failed") + d := activitylog.NewDebouncer(testlog, 50*time.Millisecond, newCallback) + + ra := data.RawActivity{EventID: "act1"} + d.Debounce("space1", ra, newAck) + + Eventually(func() int { + ackMu.Lock() + defer ackMu.Unlock() + return ackCalled + }).Should(Equal(1)) + }) + + It("calls each ack when multiple events target the same id within one batch", func() { + tracker := newAckTracker() + d := activitylog.NewDebouncer(testlog, 200*time.Millisecond, newCallback) + + ra1 := data.RawActivity{EventID: "act1"} + ra2 := data.RawActivity{EventID: "act2"} + ra3 := data.RawActivity{EventID: "act3"} + + d.Debounce("space1", ra1, tracker.track("ack1", func() error { return nil })) + d.Debounce("space1", ra2, tracker.track("ack2", func() error { return nil })) + d.Debounce("space1", ra3, tracker.track("ack3", func() error { return nil })) + + // All three activities should be stored ... + Eventually(func() int { + mu.Lock() + defer mu.Unlock() + return len(callbacks) + }).Should(Equal(3)) + + // ... and all three acks should have been called. + Eventually(func() int { return tracker.count("ack1") }).Should(Equal(1)) + Eventually(func() int { return tracker.count("ack2") }).Should(Equal(1)) + Eventually(func() int { return tracker.count("ack3") }).Should(Equal(1)) + }) + + It("calls ack on the re-schedule path when timer fires during in-progress callback", func() { + tracker := newAckTracker() + unblockCh := make(chan struct{}) + + blockingCallback := func(_ string, ras []data.RawActivity) error { + mu.Lock() + callbacks = append(callbacks, ras...) + mu.Unlock() + + <-unblockCh + return nil + } + + d := activitylog.NewDebouncer(testlog, 50*time.Millisecond, blockingCallback) + + // First activity starts the timer. + ra1 := data.RawActivity{EventID: "act1"} + d.Debounce("space1", ra1, tracker.track("ack1", func() error { return nil })) + + // Wait for timer to fire so the blocking callback begins. + Eventually(func() int { + mu.Lock() + defer mu.Unlock() + return len(callbacks) + }, 1*time.Second).Should(Equal(1)) + + // Queue a second activity on a *different* id so its own timer fires + // while the first callback is still blocked. When that timer fires, + // it will complete immediately (no blocking), then we'll queue onto + // space1 again to hit the re-schedule path. + ra2 := data.RawActivity{EventID: "act2"} + d.Debounce("space2", ra2, tracker.track("ack2", func() error { return nil })) + + // Wait for space2 batch to complete (non-blocking callback). + Eventually(func() int { + mu.Lock() + defer mu.Unlock() + return len(callbacks) + }, 1*time.Second).Should(Equal(2)) + + // Now queue onto space1 while its callback is still in-progress. + // The timer for space1 has already fired once; new Debounce calls + // will append to the existing pending item and reset the timer, + // which triggers the re-schedule path when it fires again. + ra3 := data.RawActivity{EventID: "act3"} + d.Debounce("space1", ra3, tracker.track("ack3", func() error { return nil })) + + // Unblock the first callback so space1 can process act3. + close(unblockCh) + + Eventually(func() int { + mu.Lock() + defer mu.Unlock() + return len(callbacks) + }, 2*time.Second).Should(Equal(3)) + + // All three acks should have been called. + Eventually(func() int { return tracker.count("ack1") }).Should(Equal(1)) + Eventually(func() int { return tracker.count("ack2") }).Should(Equal(1)) + Eventually(func() int { return tracker.count("ack3") }).Should(Equal(1)) + }) + + It("acks event even when store callback fails (data-loss risk)", func() { + // This test documents current behaviour: ack fires regardless of + // whether StoreActivity succeeded. If storage fails but the event + // is still acked, it will never be replayed by JetStream. + cbErr = fmt.Errorf("nats kv put failed") + d := activitylog.NewDebouncer(testlog, 50*time.Millisecond, newCallback) + + ra := data.RawActivity{EventID: "act1"} + d.Debounce("space1", ra, newAck) + + Eventually(func() int { + mu.Lock() + defer mu.Unlock() + return len(callbacks) + }).Should(Equal(1)) + + // ack was called despite the store error -- event is lost. + Eventually(func() int { + ackMu.Lock() + defer ackMu.Unlock() + return ackCalled + }).Should(Equal(1)) + }) + }) + + Context("zero-duration ack behaviour", func() { + It("calls ack when duration is zero (immediate path)", func() { + d := activitylog.NewDebouncer(testlog, 0, newCallback) + + ra := data.RawActivity{EventID: "immediate"} + d.Debounce("space1", ra, newAck) + + ackMu.Lock() + defer ackMu.Unlock() + Expect(ackCalled).To(Equal(1)) + }) + + It("calls each ack when multiple immediate events target the same id", func() { + tracker := newAckTracker() + d := activitylog.NewDebouncer(testlog, 0, newCallback) + + ra1 := data.RawActivity{EventID: "act1"} + ra2 := data.RawActivity{EventID: "act2"} + + d.Debounce("space1", ra1, tracker.track("ack1", func() error { return nil })) + d.Debounce("space1", ra2, tracker.track("ack2", func() error { return nil })) + + Expect(tracker.count("ack1")).To(Equal(1)) + Expect(tracker.count("ack2")).To(Equal(1)) + }) }) }) From 2c2650136871072948c5c882fa2d1799253091c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Thu, 13 Aug 2026 16:31:39 +0200 Subject: [PATCH 07/11] revert to legacy autoack events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- services/activitylog/pkg/command/server.go | 16 +- services/activitylog/pkg/config/config.go | 18 +- .../pkg/config/defaults/defaultconfig.go | 8 +- .../pkg/service/activitylog/activitylog.go | 14 +- .../service/activitylog/activitylog_test.go | 28 +- .../pkg/service/activitylog/debouncer.go | 23 +- .../pkg/service/activitylog/debouncer_test.go | 406 ++---------------- .../activitylog/pkg/service/events/service.go | 79 ++-- 8 files changed, 110 insertions(+), 482 deletions(-) diff --git a/services/activitylog/pkg/command/server.go b/services/activitylog/pkg/command/server.go index af4c74eec5..b417b9ddde 100644 --- a/services/activitylog/pkg/command/server.go +++ b/services/activitylog/pkg/command/server.go @@ -28,7 +28,7 @@ import ( svcEvents "github.com/opencloud-eu/opencloud/services/activitylog/pkg/service/events" svcHttp "github.com/opencloud-eu/opencloud/services/activitylog/pkg/service/http" "github.com/opencloud-eu/reva/v2/pkg/events" - "github.com/opencloud-eu/reva/v2/pkg/events/raw" + "github.com/opencloud-eu/reva/v2/pkg/events/stream" "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" ) @@ -156,7 +156,7 @@ func Server(cfg *config.Config) *cobra.Command { if !cfg.Events.Disabled { connName := generators.GenerateConnectionName(cfg.Service.Name, generators.NTypeBus) - evStream, err := raw.FromConfig(ctx, connName, raw.Config{ + evStream, err := stream.NatsFromConfig(connName, false, stream.NatsConfig{ Endpoint: cfg.Events.Endpoint, Cluster: cfg.Events.Cluster, EnableTLS: cfg.Events.EnableTLS, @@ -164,8 +164,6 @@ func Server(cfg *config.Config) *cobra.Command { TLSRootCACertificate: cfg.Events.TLSRootCACertificate, AuthUsername: cfg.Events.AuthUsername, AuthPassword: cfg.Events.AuthPassword, - MaxAckPending: cfg.Events.MaxAckPending, - AckWait: cfg.Events.AckWait, }) if err != nil { logger.Error().Err(err).Msg("Failed to initialize event stream") @@ -187,11 +185,11 @@ func Server(cfg *config.Config) *cobra.Command { return err } - gr.Add(runner.New(cfg.Service.Name+".svc", func() error { - return eventSvc.Run() - }, func() { - eventSvc.Close() - })) + gr.Add(runner.New(cfg.Service.Name+".svc", func() error { + return eventSvc.Run() + }, func() { + eventSvc.Close() + })) } else { logger.Info().Msg("event listening disabled, not starting event service") } diff --git a/services/activitylog/pkg/config/config.go b/services/activitylog/pkg/config/config.go index a3d94973f4..b2fd0e4a26 100644 --- a/services/activitylog/pkg/config/config.go +++ b/services/activitylog/pkg/config/config.go @@ -40,16 +40,14 @@ type Config struct { // Events combines the configuration options for the event bus. type Events struct { - Disabled bool `yaml:"disabled" env:"ACTIVITYLOG_EVENTS_DISABLED" desc:"Disables listening for events. Set this to true if the service should only handle HTTP requests." introductionVersion:"%NEXT%"` - Endpoint string `yaml:"endpoint" env:"OC_EVENTS_ENDPOINT" desc:"The address of the event system. The event system is the message queuing service. It is used as message broker for the microservice architecture." introductionVersion:"1.0.0"` - Cluster string `yaml:"cluster" env:"OC_EVENTS_CLUSTER" desc:"The clusterID of the event system. The event system is the message queuing service. It is used as message broker for the microservice architecture. Mandatory when using NATS as event system." introductionVersion:"1.0.0"` - TLSInsecure bool `yaml:"tls_insecure" env:"OC_INSECURE;OC_EVENTS_TLS_INSECURE" desc:"Whether to verify the server TLS certificates." introductionVersion:"1.0.0"` - TLSRootCACertificate string `yaml:"tls_root_ca_certificate" env:"OC_EVENTS_TLS_ROOT_CA_CERTIFICATE" desc:"The root CA certificate used to validate the server's TLS certificate. If provided NOTIFICATIONS_EVENTS_TLS_INSECURE will be seen as false." introductionVersion:"1.0.0"` - EnableTLS bool `yaml:"enable_tls" env:"OC_EVENTS_ENABLE_TLS" desc:"Enable TLS for the connection to the events broker. The events broker is the OpenCloud service which receives and delivers events between the services." introductionVersion:"1.0.0"` - AuthUsername string `yaml:"username" env:"OC_EVENTS_AUTH_USERNAME" desc:"The username to authenticate with the events broker. The events broker is the OpenCloud service which receives and delivers events between the services." introductionVersion:"1.0.0"` - AuthPassword string `yaml:"password" env:"OC_EVENTS_AUTH_PASSWORD" desc:"The password to authenticate with the events broker. The events broker is the OpenCloud service which receives and delivers events between the services." introductionVersion:"1.0.0"` - MaxAckPending int `yaml:"max_ack_pending" env:"ACTIVITYLOG_EVENTS_MAX_ACK_PENDING" desc:"The maximum number of unacknowledged messages. This is used to limit the number of messages that can be in flight at the same time." introductionVersion:"%NEXT%"` - AckWait time.Duration `yaml:"ack_wait" env:"ACTIVITYLOG_EVENTS_ACK_WAIT" desc:"The time to wait for an ack before the message is redelivered. This is used to ensure that messages are not lost if the consumer crashes." introductionVersion:"%NEXT%"` + Disabled bool `yaml:"disabled" env:"ACTIVITYLOG_EVENTS_DISABLED" desc:"Disables listening for events. Set this to true if the service should only handle HTTP requests." introductionVersion:"%NEXT%"` + Endpoint string `yaml:"endpoint" env:"OC_EVENTS_ENDPOINT" desc:"The address of the event system. The event system is the message queuing service. It is used as message broker for the microservice architecture." introductionVersion:"1.0.0"` + Cluster string `yaml:"cluster" env:"OC_EVENTS_CLUSTER" desc:"The clusterID of the event system. The event system is the message queuing service. It is used as message broker for the microservice architecture. Mandatory when using NATS as event system." introductionVersion:"1.0.0"` + TLSInsecure bool `yaml:"tls_insecure" env:"OC_INSECURE;OC_EVENTS_TLS_INSECURE" desc:"Whether to verify the server TLS certificates." introductionVersion:"1.0.0"` + TLSRootCACertificate string `yaml:"tls_root_ca_certificate" env:"OC_EVENTS_TLS_ROOT_CA_CERTIFICATE" desc:"The root CA certificate used to validate the server's TLS certificate. If provided NOTIFICATIONS_EVENTS_TLS_INSECURE will be seen as false." introductionVersion:"1.0.0"` + EnableTLS bool `yaml:"enable_tls" env:"OC_EVENTS_ENABLE_TLS" desc:"Enable TLS for the connection to the events broker. The events broker is the OpenCloud service which receives and delivers events between the services." introductionVersion:"1.0.0"` + AuthUsername string `yaml:"username" env:"OC_EVENTS_AUTH_USERNAME" desc:"The username to authenticate with the events broker. The events broker is the OpenCloud service which receives and delivers events between the services." introductionVersion:"1.0.0"` + AuthPassword string `yaml:"password" env:"OC_EVENTS_AUTH_PASSWORD" desc:"The password to authenticate with the events broker. The events broker is the OpenCloud service which receives and delivers events between the services." introductionVersion:"1.0.0"` } // Store configures the store to use diff --git a/services/activitylog/pkg/config/defaults/defaultconfig.go b/services/activitylog/pkg/config/defaults/defaultconfig.go index 7c9c9073fc..6bb7ba65b0 100644 --- a/services/activitylog/pkg/config/defaults/defaultconfig.go +++ b/services/activitylog/pkg/config/defaults/defaultconfig.go @@ -29,11 +29,9 @@ func DefaultConfig() *config.Config { Name: "activitylog", }, Events: config.Events{ - Endpoint: "127.0.0.1:9233", - Cluster: "opencloud-cluster", - EnableTLS: false, - MaxAckPending: 1000, - AckWait: 1 * time.Minute, + Endpoint: "127.0.0.1:9233", + Cluster: "opencloud-cluster", + EnableTLS: false, }, Store: config.Store{ Store: "nats-js-kv", diff --git a/services/activitylog/pkg/service/activitylog/activitylog.go b/services/activitylog/pkg/service/activitylog/activitylog.go index 3eec740cd8..50cba5130d 100644 --- a/services/activitylog/pkg/service/activitylog/activitylog.go +++ b/services/activitylog/pkg/service/activitylog/activitylog.go @@ -77,7 +77,7 @@ func New(kv nats.KeyValue, opts ...Option) (*ActivityLog, error) { maxActivities: o.MaxActivities, natskv: kv, } - s.debouncer = NewDebouncer(o.Logger, o.WriteBufferDuration, s.StoreActivity) + s.debouncer = NewDebouncer(o.WriteBufferDuration, s.StoreActivity) // run migrations err = s.runMigrations(context.Background(), kv) @@ -89,7 +89,7 @@ func New(kv nats.KeyValue, opts ...Option) (*ActivityLog, error) { } // RemoveResource removes the resource from the store -func (a *ActivityLog) RemoveResource(rid *provider.ResourceId, ack func() error) error { +func (a *ActivityLog) RemoveResource(rid *provider.ResourceId) error { if rid == nil { return fmt.Errorf("resource id is required") } @@ -101,16 +101,10 @@ func (a *ActivityLog) RemoveResource(rid *provider.ResourceId, ack func() error) if err != nil { return fmt.Errorf("could not delete resource %s: %w", rid.OpaqueId, err) } - go func() { - err := ack() - if err != nil { - a.log.Error().Err(err).Msg("error while acknowledging resource removal") - } - }() return nil } -func (a *ActivityLog) AddActivity(ctx context.Context, initRef *provider.Reference, parentId *provider.ResourceId, eventID string, timestamp time.Time, getResource func(context.Context, *provider.Reference) (*provider.ResourceInfo, error), ack func() error) error { +func (a *ActivityLog) AddActivity(ctx context.Context, initRef *provider.Reference, parentId *provider.ResourceId, eventID string, timestamp time.Time, getResource func(context.Context, *provider.Reference) (*provider.ResourceInfo, error)) error { var ( err error depth int @@ -141,7 +135,7 @@ func (a *ActivityLog) AddActivity(ctx context.Context, initRef *provider.Referen EventID: eventID, Depth: depth, Timestamp: timestamp, - }, ack) + }) if id.OpaqueId == id.SpaceId { // we are at the root of the space, no need to go further diff --git a/services/activitylog/pkg/service/activitylog/activitylog_test.go b/services/activitylog/pkg/service/activitylog/activitylog_test.go index 5bc9a7e48c..c968a935ec 100644 --- a/services/activitylog/pkg/service/activitylog/activitylog_test.go +++ b/services/activitylog/pkg/service/activitylog/activitylog_test.go @@ -34,10 +34,6 @@ func getFreeLocalhostPort() (int, error) { return port, nil } -func NoOpAck() error { - return nil -} - // Spawn a nats server and a JetStream instance for the duration of the test suite. // The different tests need to make sure to use different databases to avoid conflicts. var _ = SynchronizedBeforeSuite(func() { @@ -149,7 +145,7 @@ var _ = Describe("ActivitylogService", func() { } for k, v := range tc.Activities { - err := alog.AddActivity(context.Background(), reference(v), nil, k, time.Time{}, getResource, NoOpAck) + err := alog.AddActivity(context.Background(), reference(v), nil, k, time.Time{}, getResource) Expect(err).NotTo(HaveOccurred()) } }) @@ -188,9 +184,9 @@ var _ = Describe("ActivitylogService", func() { It("debounces activities", func() { - err := alog.AddActivity(context.Background(), reference("base"), nil, "activity1", time.Time{}, getResource, NoOpAck) + err := alog.AddActivity(context.Background(), reference("base"), nil, "activity1", time.Time{}, getResource) Expect(err).NotTo(HaveOccurred()) - err = alog.AddActivity(context.Background(), reference("base"), nil, "activity2", time.Time{}, getResource, NoOpAck) + err = alog.AddActivity(context.Background(), reference("base"), nil, "activity2", time.Time{}, getResource) Expect(err).NotTo(HaveOccurred()) Eventually(func(g Gomega) { @@ -201,7 +197,7 @@ var _ = Describe("ActivitylogService", func() { }) It("adheres to the MaxActivities setting", func() { - err := alog.AddActivity(context.Background(), reference("base"), nil, "activity1", time.Time{}, getResource, NoOpAck) + err := alog.AddActivity(context.Background(), reference("base"), nil, "activity1", time.Time{}, getResource) Expect(err).NotTo(HaveOccurred()) Eventually(func(g Gomega) { activities, err := alog.Activities(resourceID("base")) @@ -209,7 +205,7 @@ var _ = Describe("ActivitylogService", func() { g.Expect(len(activities)).To(Equal(1)) }).Should(Succeed()) - err = alog.AddActivity(context.Background(), reference("base"), nil, "activity2", time.Time{}, getResource, NoOpAck) + err = alog.AddActivity(context.Background(), reference("base"), nil, "activity2", time.Time{}, getResource) Expect(err).NotTo(HaveOccurred()) Eventually(func(g Gomega) { activities, err := alog.Activities(resourceID("base")) @@ -217,11 +213,11 @@ var _ = Describe("ActivitylogService", func() { g.Expect(len(activities)).To(Equal(2)) }).Should(Succeed()) - err = alog.AddActivity(context.Background(), reference("base"), nil, "activity3", time.Time{}, getResource, NoOpAck) + err = alog.AddActivity(context.Background(), reference("base"), nil, "activity3", time.Time{}, getResource) Expect(err).NotTo(HaveOccurred()) - err = alog.AddActivity(context.Background(), reference("base"), nil, "activity4", time.Time{}, getResource, NoOpAck) + err = alog.AddActivity(context.Background(), reference("base"), nil, "activity4", time.Time{}, getResource) Expect(err).NotTo(HaveOccurred()) - err = alog.AddActivity(context.Background(), reference("base"), nil, "activity5", time.Time{}, getResource, NoOpAck) + err = alog.AddActivity(context.Background(), reference("base"), nil, "activity5", time.Time{}, getResource) Expect(err).NotTo(HaveOccurred()) Eventually(func(g Gomega) { @@ -238,9 +234,9 @@ var _ = Describe("ActivitylogService", func() { return tree[ref.GetResourceId().GetOpaqueId()], nil } - err := alog.AddActivity(context.Background(), reference("base"), nil, "activity1", time.Time{}, getResource, NoOpAck) + err := alog.AddActivity(context.Background(), reference("base"), nil, "activity1", time.Time{}, getResource) Expect(err).NotTo(HaveOccurred()) - err = alog.AddActivity(context.Background(), reference("base"), nil, "activity2", time.Time{}, getResource, NoOpAck) + err = alog.AddActivity(context.Background(), reference("base"), nil, "activity2", time.Time{}, getResource) Expect(err).NotTo(HaveOccurred()) Eventually(func(g Gomega) { @@ -249,9 +245,9 @@ var _ = Describe("ActivitylogService", func() { g.Expect(activities).To(ConsistOf(activitites("activity1", 0, "activity2", 0))) }).Should(Succeed()) - err = alog.AddActivity(context.Background(), reference("base"), nil, "activity3", time.Time{}, getResource, NoOpAck) + err = alog.AddActivity(context.Background(), reference("base"), nil, "activity3", time.Time{}, getResource) Expect(err).NotTo(HaveOccurred()) - err = alog.AddActivity(context.Background(), reference("base"), nil, "activity4", time.Time{}, getResource, NoOpAck) + err = alog.AddActivity(context.Background(), reference("base"), nil, "activity4", time.Time{}, getResource) Expect(err).NotTo(HaveOccurred()) Eventually(func(g Gomega) { diff --git a/services/activitylog/pkg/service/activitylog/debouncer.go b/services/activitylog/pkg/service/activitylog/debouncer.go index e3de92cb6c..5c60923462 100644 --- a/services/activitylog/pkg/service/activitylog/debouncer.go +++ b/services/activitylog/pkg/service/activitylog/debouncer.go @@ -4,16 +4,15 @@ import ( "sync" "time" - "github.com/opencloud-eu/opencloud/pkg/log" "github.com/opencloud-eu/opencloud/services/activitylog/pkg/data" ) +// Debouncer is used to debounce writes to the activity log store. type Debouncer struct { after time.Duration f func(id string, ra []data.RawActivity) error pending sync.Map inProgress sync.Map - log log.Logger mutex sync.Mutex } @@ -23,19 +22,18 @@ type queueItem struct { timer *time.Timer } -// NewDebouncer returns a new Debouncer instance -func NewDebouncer(log log.Logger, d time.Duration, f func(id string, ra []data.RawActivity) error) *Debouncer { +// NewDebouncer returns a new Debouncer instance. +func NewDebouncer(d time.Duration, f func(id string, ra []data.RawActivity) error) *Debouncer { return &Debouncer{ after: d, f: f, pending: sync.Map{}, inProgress: sync.Map{}, - log: log, } } -// Debounce restarts the debounce timer for the given space -func (d *Debouncer) Debounce(id string, ra data.RawActivity, ack func() error) { +// Debounce restarts the debounce timer for the given space. +func (d *Debouncer) Debounce(id string, ra data.RawActivity) { if d.after == 0 { d.f(id, []data.RawActivity{ra}) return @@ -44,10 +42,10 @@ func (d *Debouncer) Debounce(id string, ra data.RawActivity, ack func() error) { d.mutex.Lock() defer d.mutex.Unlock() - activities := []data.RawActivity{ra} item := &queueItem{ - activities: activities, + activities: []data.RawActivity{ra}, } + if i, ok := d.pending.Load(id); ok { // if the item is already in the queue, append the new activities item, ok = i.(*queueItem) @@ -73,13 +71,6 @@ func (d *Debouncer) Debounce(id string, ra data.RawActivity, ack func() error) { d.inProgress.Store(id, true) defer d.inProgress.Delete(id) d.f(id, item.activities) - go func() { - if ack != nil { - if err := ack(); err != nil { - d.log.Error().Err(err).Msg("error while acknowledging event") - } - } - }() }) } diff --git a/services/activitylog/pkg/service/activitylog/debouncer_test.go b/services/activitylog/pkg/service/activitylog/debouncer_test.go index 6305d51f80..5a44d33d6c 100644 --- a/services/activitylog/pkg/service/activitylog/debouncer_test.go +++ b/services/activitylog/pkg/service/activitylog/debouncer_test.go @@ -1,140 +1,68 @@ package activitylog_test import ( - "fmt" "sync" "time" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - "github.com/opencloud-eu/opencloud/pkg/log" "github.com/opencloud-eu/opencloud/services/activitylog/pkg/data" "github.com/opencloud-eu/opencloud/services/activitylog/pkg/service/activitylog" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" ) -// ackTracker tracks per-name ack invocations so we can assert each event's ack fires. -type ackTracker struct { - mu sync.Mutex - calls map[string]int - errors map[string]error -} - -func newAckTracker() *ackTracker { - return &ackTracker{ - calls: make(map[string]int), - errors: make(map[string]error), - } -} - -func (t *ackTracker) track(name string, fn func() error) func() error { - return func() error { - t.mu.Lock() - t.calls[name]++ - err := fn() - t.errors[name] = err - t.mu.Unlock() - return err - } -} - -func (t *ackTracker) count(name string) int { - t.mu.Lock() - defer t.mu.Unlock() - return t.calls[name] -} - var _ = Describe("Debouncer", func() { var ( - testlog = log.NopLogger() - callbacks []data.RawActivity - mu sync.Mutex - cbErr error - ackCalled int - ackMu sync.Mutex + mu sync.Mutex + callbacks []data.RawActivity + newCallback func(id string, ra []data.RawActivity) error ) - newCallback := func(_ string, ras []data.RawActivity) error { - mu.Lock() - defer mu.Unlock() - callbacks = append(callbacks, ras...) - return cbErr - } - - newAck := func() error { - ackMu.Lock() - defer ackMu.Unlock() - ackCalled++ - return nil - } - BeforeEach(func() { + mu.Lock() callbacks = nil - cbErr = nil - ackMu.Lock() - ackCalled = 0 - ackMu.Unlock() + mu.Unlock() + newCallback = func(id string, ra []data.RawActivity) error { + mu.Lock() + defer mu.Unlock() + callbacks = append(callbacks, ra...) + return nil + } }) Context("with zero duration", func() { - It("calls the function immediately", func() { - d := activitylog.NewDebouncer(testlog, 0, newCallback) - - ra := data.RawActivity{EventID: "immediate"} - d.Debounce("space1", ra, nil) - - mu.Lock() - defer mu.Unlock() + It("calls the callback immediately", func() { + d := activitylog.NewDebouncer(0, newCallback) + d.Debounce("space1", data.RawActivity{EventID: "activity1"}) Expect(callbacks).To(HaveLen(1)) - Expect(callbacks[0].EventID).To(Equal("immediate")) + Expect(callbacks[0].EventID).To(Equal("activity1")) }) - It("calls the function immediately for each call without batching", func() { - d := activitylog.NewDebouncer(testlog, 0, newCallback) - - ra1 := data.RawActivity{EventID: "act1"} - ra2 := data.RawActivity{EventID: "act2"} - d.Debounce("space1", ra1, nil) - d.Debounce("space1", ra2, nil) - - mu.Lock() - defer mu.Unlock() + It("calls the callback immediately for each event", func() { + d := activitylog.NewDebouncer(0, newCallback) + d.Debounce("space1", data.RawActivity{EventID: "activity1"}) + d.Debounce("space2", data.RawActivity{EventID: "activity2"}) Expect(callbacks).To(HaveLen(2)) }) }) Context("with non-zero duration", func() { It("batches activities with the same id", func() { - d := activitylog.NewDebouncer(testlog, 50*time.Millisecond, newCallback) - - ra1 := data.RawActivity{EventID: "act1"} - ra2 := data.RawActivity{EventID: "act2"} - ra3 := data.RawActivity{EventID: "act3"} - - d.Debounce("space1", ra1, nil) - d.Debounce("space1", ra2, nil) - d.Debounce("space1", ra3, nil) + d := activitylog.NewDebouncer(10*time.Millisecond, newCallback) + d.Debounce("space1", data.RawActivity{EventID: "activity1"}) + d.Debounce("space1", data.RawActivity{EventID: "activity2"}) + d.Debounce("space1", data.RawActivity{EventID: "activity3"}) Eventually(func() int { mu.Lock() defer mu.Unlock() return len(callbacks) }).Should(Equal(3)) - - mu.Lock() - defer mu.Unlock() - Expect(callbacks[0].EventID).To(Equal("act1")) - Expect(callbacks[1].EventID).To(Equal("act2")) - Expect(callbacks[2].EventID).To(Equal("act3")) }) It("handles different ids independently", func() { - d := activitylog.NewDebouncer(testlog, 50*time.Millisecond, newCallback) - - ra1 := data.RawActivity{EventID: "space1-act"} - ra2 := data.RawActivity{EventID: "space2-act"} - - d.Debounce("space1", ra1, nil) - d.Debounce("space2", ra2, nil) + d := activitylog.NewDebouncer(10*time.Millisecond, newCallback) + d.Debounce("space1", data.RawActivity{EventID: "activity1"}) + d.Debounce("space2", data.RawActivity{EventID: "activity2"}) Eventually(func() int { mu.Lock() @@ -143,58 +71,13 @@ var _ = Describe("Debouncer", func() { }).Should(Equal(2)) }) - It("calls the ack function after callback completes", func() { - d := activitylog.NewDebouncer(testlog, 50*time.Millisecond, newCallback) - - ra := data.RawActivity{EventID: "act1"} - d.Debounce("space1", ra, newAck) - - Eventually(func() int { - ackMu.Lock() - defer ackMu.Unlock() - return ackCalled - }).Should(Equal(1)) - }) - - It("does not panic when ack is nil", func() { - d := activitylog.NewDebouncer(testlog, 50*time.Millisecond, newCallback) - - ra := data.RawActivity{EventID: "act1"} - Expect(func() { d.Debounce("space1", ra, nil) }).ToNot(Panic()) - - Eventually(func() int { - mu.Lock() - defer mu.Unlock() - return len(callbacks) - }).Should(Equal(1)) - }) - - It("does not panic when ack returns an error", func() { - d := activitylog.NewDebouncer(testlog, 50*time.Millisecond, newCallback) - - errAck := func() error { return fmt.Errorf("ack error") } - ra := data.RawActivity{EventID: "act1"} - Expect(func() { d.Debounce("space1", ra, errAck) }).ToNot(Panic()) - - Eventually(func() int { - mu.Lock() - defer mu.Unlock() - return len(callbacks) - }).Should(Equal(1)) - }) - It("batches activities that arrive within the debounce window", func() { - d := activitylog.NewDebouncer(testlog, 200*time.Millisecond, newCallback) - - ra1 := data.RawActivity{EventID: "act1"} - d.Debounce("space1", ra1, nil) - - time.Sleep(50 * time.Millisecond) - - ra2 := data.RawActivity{EventID: "act2"} - ra3 := data.RawActivity{EventID: "act3"} - d.Debounce("space1", ra2, nil) - d.Debounce("space1", ra3, nil) + d := activitylog.NewDebouncer(100*time.Millisecond, newCallback) + d.Debounce("space1", data.RawActivity{EventID: "activity1"}) + time.Sleep(20 * time.Millisecond) + d.Debounce("space1", data.RawActivity{EventID: "activity2"}) + time.Sleep(20 * time.Millisecond) + d.Debounce("space1", data.RawActivity{EventID: "activity3"}) Eventually(func() int { mu.Lock() @@ -204,235 +87,36 @@ var _ = Describe("Debouncer", func() { }) It("processes new batch after previous completes", func() { - d := activitylog.NewDebouncer(testlog, 50*time.Millisecond, newCallback) - - // First batch - ra1 := data.RawActivity{EventID: "batch1-act1"} - ra2 := data.RawActivity{EventID: "batch1-act2"} - d.Debounce("space1", ra1, nil) - d.Debounce("space1", ra2, nil) + d := activitylog.NewDebouncer(5*time.Millisecond, newCallback) + d.Debounce("space1", data.RawActivity{EventID: "activity1"}) + time.Sleep(20 * time.Millisecond) // let first batch complete + d.Debounce("space1", data.RawActivity{EventID: "activity2"}) - // Wait for first batch to be processed Eventually(func() int { mu.Lock() defer mu.Unlock() return len(callbacks) }).Should(Equal(2)) - - // Second batch - callbacks = nil - ra3 := data.RawActivity{EventID: "batch2-act1"} - d.Debounce("space1", ra3, nil) - - Eventually(func() int { - mu.Lock() - defer mu.Unlock() - return len(callbacks) - }).Should(Equal(1)) - mu.Lock() - Expect(callbacks[0].EventID).To(Equal("batch2-act1")) - mu.Unlock() }) - It("reschedules when timer fires during in-progress callback", func() { - unblockCh := make(chan struct{}) - - blockingCallback := func(_ string, ras []data.RawActivity) error { - mu.Lock() - callbacks = append(callbacks, ras...) - mu.Unlock() - - <-unblockCh - return nil - } - - d := activitylog.NewDebouncer(testlog, 50*time.Millisecond, blockingCallback) - - // First activity starts the timer - ra1 := data.RawActivity{EventID: "act1"} - d.Debounce("space1", ra1, nil) - - // Second activity gets queued while first is being processed - time.Sleep(60 * time.Millisecond) - // Timer fires for act1, callback starts (and blocks) - Eventually(func() int { - mu.Lock() - defer mu.Unlock() - return len(callbacks) - }, 1*time.Second).Should(Equal(1)) - - // Queue another activity while the first callback is still running - ra2 := data.RawActivity{EventID: "act2"} - d.Debounce("space1", ra2, nil) - - // Unblock the first callback - close(unblockCh) - - // The second activity should eventually be processed - Eventually(func() int { - mu.Lock() - defer mu.Unlock() - return len(callbacks) - }, 2*time.Second).Should(Equal(2)) - - mu.Lock() - defer mu.Unlock() - Expect(callbacks[0].EventID).To(Equal("act1")) - Expect(callbacks[1].EventID).To(Equal("act2")) - }) - - It("calls ack even when callback returns an error", func() { - cbErr = fmt.Errorf("store failed") - d := activitylog.NewDebouncer(testlog, 50*time.Millisecond, newCallback) - - ra := data.RawActivity{EventID: "act1"} - d.Debounce("space1", ra, newAck) - - Eventually(func() int { - ackMu.Lock() - defer ackMu.Unlock() - return ackCalled - }).Should(Equal(1)) - }) - - It("calls each ack when multiple events target the same id within one batch", func() { - tracker := newAckTracker() - d := activitylog.NewDebouncer(testlog, 200*time.Millisecond, newCallback) - - ra1 := data.RawActivity{EventID: "act1"} - ra2 := data.RawActivity{EventID: "act2"} - ra3 := data.RawActivity{EventID: "act3"} - - d.Debounce("space1", ra1, tracker.track("ack1", func() error { return nil })) - d.Debounce("space1", ra2, tracker.track("ack2", func() error { return nil })) - d.Debounce("space1", ra3, tracker.track("ack3", func() error { return nil })) - - // All three activities should be stored ... - Eventually(func() int { + It("skips duplicate write when timer fires during in-progress callback", func() { + slowCallback := func(id string, ra []data.RawActivity) error { + time.Sleep(50 * time.Millisecond) // simulate slow write mu.Lock() defer mu.Unlock() - return len(callbacks) - }).Should(Equal(3)) - - // ... and all three acks should have been called. - Eventually(func() int { return tracker.count("ack1") }).Should(Equal(1)) - Eventually(func() int { return tracker.count("ack2") }).Should(Equal(1)) - Eventually(func() int { return tracker.count("ack3") }).Should(Equal(1)) - }) - - It("calls ack on the re-schedule path when timer fires during in-progress callback", func() { - tracker := newAckTracker() - unblockCh := make(chan struct{}) - - blockingCallback := func(_ string, ras []data.RawActivity) error { - mu.Lock() - callbacks = append(callbacks, ras...) - mu.Unlock() - - <-unblockCh + callbacks = append(callbacks, ra...) return nil } - d := activitylog.NewDebouncer(testlog, 50*time.Millisecond, blockingCallback) - - // First activity starts the timer. - ra1 := data.RawActivity{EventID: "act1"} - d.Debounce("space1", ra1, tracker.track("ack1", func() error { return nil })) - - // Wait for timer to fire so the blocking callback begins. - Eventually(func() int { - mu.Lock() - defer mu.Unlock() - return len(callbacks) - }, 1*time.Second).Should(Equal(1)) - - // Queue a second activity on a *different* id so its own timer fires - // while the first callback is still blocked. When that timer fires, - // it will complete immediately (no blocking), then we'll queue onto - // space1 again to hit the re-schedule path. - ra2 := data.RawActivity{EventID: "act2"} - d.Debounce("space2", ra2, tracker.track("ack2", func() error { return nil })) - - // Wait for space2 batch to complete (non-blocking callback). - Eventually(func() int { - mu.Lock() - defer mu.Unlock() - return len(callbacks) - }, 1*time.Second).Should(Equal(2)) - - // Now queue onto space1 while its callback is still in-progress. - // The timer for space1 has already fired once; new Debounce calls - // will append to the existing pending item and reset the timer, - // which triggers the re-schedule path when it fires again. - ra3 := data.RawActivity{EventID: "act3"} - d.Debounce("space1", ra3, tracker.track("ack3", func() error { return nil })) - - // Unblock the first callback so space1 can process act3. - close(unblockCh) - - Eventually(func() int { - mu.Lock() - defer mu.Unlock() - return len(callbacks) - }, 2*time.Second).Should(Equal(3)) - - // All three acks should have been called. - Eventually(func() int { return tracker.count("ack1") }).Should(Equal(1)) - Eventually(func() int { return tracker.count("ack2") }).Should(Equal(1)) - Eventually(func() int { return tracker.count("ack3") }).Should(Equal(1)) - }) - - It("acks event even when store callback fails (data-loss risk)", func() { - // This test documents current behaviour: ack fires regardless of - // whether StoreActivity succeeded. If storage fails but the event - // is still acked, it will never be replayed by JetStream. - cbErr = fmt.Errorf("nats kv put failed") - d := activitylog.NewDebouncer(testlog, 50*time.Millisecond, newCallback) - - ra := data.RawActivity{EventID: "act1"} - d.Debounce("space1", ra, newAck) + d := activitylog.NewDebouncer(10*time.Millisecond, slowCallback) + d.Debounce("space1", data.RawActivity{EventID: "activity1"}) + time.Sleep(20 * time.Millisecond) // timer fires while callback is running Eventually(func() int { mu.Lock() defer mu.Unlock() return len(callbacks) }).Should(Equal(1)) - - // ack was called despite the store error -- event is lost. - Eventually(func() int { - ackMu.Lock() - defer ackMu.Unlock() - return ackCalled - }).Should(Equal(1)) - }) - }) - - Context("zero-duration ack behaviour", func() { - It("calls ack when duration is zero (immediate path)", func() { - d := activitylog.NewDebouncer(testlog, 0, newCallback) - - ra := data.RawActivity{EventID: "immediate"} - d.Debounce("space1", ra, newAck) - - ackMu.Lock() - defer ackMu.Unlock() - Expect(ackCalled).To(Equal(1)) - }) - - It("calls each ack when multiple immediate events target the same id", func() { - tracker := newAckTracker() - d := activitylog.NewDebouncer(testlog, 0, newCallback) - - ra1 := data.RawActivity{EventID: "act1"} - ra2 := data.RawActivity{EventID: "act2"} - - d.Debounce("space1", ra1, tracker.track("ack1", func() error { return nil })) - d.Debounce("space1", ra2, tracker.track("ack2", func() error { return nil })) - - Expect(tracker.count("ack1")).To(Equal(1)) - Expect(tracker.count("ack2")).To(Equal(1)) }) }) }) - - diff --git a/services/activitylog/pkg/service/events/service.go b/services/activitylog/pkg/service/events/service.go index 58347a4709..60678d5085 100644 --- a/services/activitylog/pkg/service/events/service.go +++ b/services/activitylog/pkg/service/events/service.go @@ -15,7 +15,6 @@ import ( "github.com/opencloud-eu/opencloud/services/activitylog/pkg/data" "github.com/opencloud-eu/opencloud/services/activitylog/pkg/service/activitylog" "github.com/opencloud-eu/reva/v2/pkg/events" - "github.com/opencloud-eu/reva/v2/pkg/events/raw" "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" "github.com/opencloud-eu/reva/v2/pkg/storagespace" "github.com/opencloud-eu/reva/v2/pkg/utils" @@ -38,7 +37,7 @@ type ActivitylogService struct { ctx context.Context sa config.ServiceAccount log log.Logger - stream raw.Stream + stream events.Stream gws pool.Selectable[gateway.GatewayAPIClient] al *activitylog.ActivityLog @@ -51,7 +50,7 @@ type ActivitylogService struct { } // New creates a new ActivitylogService -func New(al *activitylog.ActivityLog, stream raw.Stream, opts ...Option) (*ActivitylogService, error) { +func New(al *activitylog.ActivityLog, stream events.Stream, opts ...Option) (*ActivitylogService, error) { o := &Options{ NumConsumers: _numConsumersDefault, } @@ -77,15 +76,11 @@ func New(al *activitylog.ActivityLog, stream raw.Stream, opts ...Option) (*Activ // Run to fulfil Runner interface func (s *ActivitylogService) Run() error { - ch, err := s.stream.Consume("activitylog-pull", s.events...) + ch, err := events.Consume(s.stream, "activitylog-pull", s.events...) if err != nil { return err } - // if s.m != nil { - // monitorMetrics(s.ctx, s.stream, "activitylog", s.m, s.log) - // } - var wg sync.WaitGroup ctx, cancel := context.WithCancel(s.ctx) defer cancel() @@ -130,30 +125,24 @@ func (s *ActivitylogService) Run() error { // Close will make the service to stop processing, so the `Run` // method can finish. -// TODO: Underlying services can't be stopped. This means that some goroutines -// will get stuck trying to push events through a channel nobody is reading -// from, so resources won't be freed and there will be memory leaks. For now, -// if the service is stopped, you should close the app soon after. func (s *ActivitylogService) Close() { if s.stopped.CompareAndSwap(false, true) { close(s.stopCh) } } -// Run runs the service -func (s *ActivitylogService) processEvent(e raw.Event) error { +func (s *ActivitylogService) processEvent(e events.Event) error { ctx := e.GetTraceContext(s.ctx) ctx, span := tracer.Start(ctx, "processEvent") defer span.End() - e.InProgress() // let nats know that we are processing this event s.log.Debug().Interface("event", e).Msg("updating activitylog") - switch ev := e.Event.Event.(type) { + switch ev := e.Event.(type) { case events.UploadReady: - return s.AddActivity(ctx, ev.FileRef, ev.ParentID, e.ID, utils.TSToTime(ev.Timestamp), e.Ack) + return s.AddActivity(ctx, ev.FileRef, ev.ParentID, e.ID, utils.TSToTime(ev.Timestamp)) case events.FileTouched: - return s.AddActivity(ctx, ev.Ref, ev.ParentID, e.ID, utils.TSToTime(ev.Timestamp), e.Ack) + return s.AddActivity(ctx, ev.Ref, ev.ParentID, e.ID, utils.TSToTime(ev.Timestamp)) // Disabled https://github.com/owncloud/ocis/issues/10293 //case events.FileDownloaded: // we are only interested in public link downloads - so no need to store others. @@ -161,57 +150,43 @@ func (s *ActivitylogService) processEvent(e raw.Event) error { // err = a.AddActivity(ev.Ref, e.ID, utils.TSToTime(ev.Timestamp)) //} case events.ContainerCreated: - return s.AddActivity(ctx, ev.Ref, ev.ParentID, e.ID, utils.TSToTime(ev.Timestamp), e.Ack) + return s.AddActivity(ctx, ev.Ref, ev.ParentID, e.ID, utils.TSToTime(ev.Timestamp)) case events.ItemTrashed: - return s.AddActivityTrashed(ctx, ev.ID, ev.Ref, nil, e.ID, utils.TSToTime(ev.Timestamp), e.Ack) + return s.AddActivityTrashed(ctx, ev.ID, ev.Ref, nil, e.ID, utils.TSToTime(ev.Timestamp)) case events.ItemPurged: - return s.al.RemoveResource(ev.ID, e.Ack) + return s.al.RemoveResource(ev.ID) case events.ItemMoved: // remove the cached parent id for this resource s.removeCachedParentID(ctx, ev.Ref) - return s.AddActivity(ctx, ev.Ref, nil, e.ID, utils.TSToTime(ev.Timestamp), e.Ack) + return s.AddActivity(ctx, ev.Ref, nil, e.ID, utils.TSToTime(ev.Timestamp)) case events.ShareCreated: - return s.AddActivity(ctx, toRef(ev.ItemID), nil, e.ID, utils.TSToTime(ev.CTime), e.Ack) + return s.AddActivity(ctx, toRef(ev.ItemID), nil, e.ID, utils.TSToTime(ev.CTime)) case events.ShareUpdated: if ev.Sharer != nil && ev.ItemID != nil && ev.Sharer.GetOpaqueId() != ev.ItemID.GetSpaceId() { - return s.AddActivity(ctx, toRef(ev.ItemID), nil, e.ID, utils.TSToTime(ev.MTime), e.Ack) - } else { - go func() { - err := e.Ack() - if err != nil { - s.log.Error().Err(err).Msg("error while acknowledging event") - } - }() + return s.AddActivity(ctx, toRef(ev.ItemID), nil, e.ID, utils.TSToTime(ev.MTime)) } case events.ShareRemoved: - return s.AddActivity(ctx, toRef(ev.ItemID), nil, e.ID, ev.Timestamp, e.Ack) + return s.AddActivity(ctx, toRef(ev.ItemID), nil, e.ID, ev.Timestamp) case events.LinkCreated: - return s.AddActivity(ctx, toRef(ev.ItemID), nil, e.ID, utils.TSToTime(ev.CTime), e.Ack) + return s.AddActivity(ctx, toRef(ev.ItemID), nil, e.ID, utils.TSToTime(ev.CTime)) case events.LinkUpdated: if ev.Sharer != nil && ev.ItemID != nil && ev.Sharer.GetOpaqueId() != ev.ItemID.GetSpaceId() { - return s.AddActivity(ctx, toRef(ev.ItemID), nil, e.ID, utils.TSToTime(ev.MTime), e.Ack) - } else { - go func() { - err := e.Ack() - if err != nil { - s.log.Error().Err(err).Msg("error while acknowledging event") - } - }() + return s.AddActivity(ctx, toRef(ev.ItemID), nil, e.ID, utils.TSToTime(ev.MTime)) } case events.LinkRemoved: - return s.AddActivity(ctx, toRef(ev.ItemID), nil, e.ID, utils.TSToTime(ev.Timestamp), e.Ack) + return s.AddActivity(ctx, toRef(ev.ItemID), nil, e.ID, utils.TSToTime(ev.Timestamp)) case events.SpaceShared: - return s.AddSpaceActivity(ctx, ev.ID, e.ID, ev.Timestamp, e.Ack) + return s.AddSpaceActivity(ctx, ev.ID, e.ID, ev.Timestamp) case events.SpaceUnshared: - return s.AddSpaceActivity(ctx, ev.ID, e.ID, ev.Timestamp, e.Ack) + return s.AddSpaceActivity(ctx, ev.ID, e.ID, ev.Timestamp) } return nil } // AddActivity adds the activity to the given resource and all its parents -func (a *ActivitylogService) AddActivity(ctx context.Context, initRef *provider.Reference, parentId *provider.ResourceId, eventID string, timestamp time.Time, ack func() error) error { +func (a *ActivitylogService) AddActivity(ctx context.Context, initRef *provider.Reference, parentId *provider.ResourceId, eventID string, timestamp time.Time) error { ctx, span := tracer.Start(ctx, "AddActivity") defer span.End() @@ -227,11 +202,11 @@ func (a *ActivitylogService) AddActivity(ctx context.Context, initRef *provider. } return a.al.AddActivity(ctx, initRef, parentId, eventID, timestamp, func(ctx context.Context, ref *provider.Reference) (*provider.ResourceInfo, error) { return utils.GetResource(ctx, ref, gwc) - }, ack) + }) } // AddActivityTrashed adds the activity to given trashed resource and all its former parents -func (a *ActivitylogService) AddActivityTrashed(ctx context.Context, resourceID *provider.ResourceId, reference *provider.Reference, parentId *provider.ResourceId, eventID string, timestamp time.Time, ack func() error) error { +func (a *ActivitylogService) AddActivityTrashed(ctx context.Context, resourceID *provider.ResourceId, reference *provider.Reference, parentId *provider.ResourceId, eventID string, timestamp time.Time) error { ctx, span := tracer.Start(ctx, "AddActivityTrashed") defer span.End() @@ -264,11 +239,11 @@ func (a *ActivitylogService) AddActivityTrashed(ctx context.Context, resourceID return a.al.AddActivity(ctx, ref, parentId, eventID, timestamp, func(ctx context.Context, ref *provider.Reference) (*provider.ResourceInfo, error) { return utils.GetResource(ctx, ref, gwc) - }, ack) + }) } // AddSpaceActivity adds the activity to the given spaceroot -func (a *ActivitylogService) AddSpaceActivity(ctx context.Context, spaceID *provider.StorageSpaceId, eventID string, timestamp time.Time, ack func() error) error { +func (a *ActivitylogService) AddSpaceActivity(ctx context.Context, spaceID *provider.StorageSpaceId, eventID string, timestamp time.Time) error { _, span := tracer.Start(ctx, "AddSpaceActivity") defer span.End() // spaceID is in format $ @@ -289,12 +264,6 @@ func (a *ActivitylogService) AddSpaceActivity(ctx context.Context, spaceID *prov if err != nil { return fmt.Errorf("could not store activity: %w", err) } - go func() { - err := ack() - if err != nil { - a.log.Error().Err(err).Msg("error while acknowledging event") - } - }() return nil } From 90dbb5e30423832dbfb0de7b6ca63a457f4e763e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Fri, 14 Aug 2026 09:30:55 +0200 Subject: [PATCH 08/11] handle error types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- .../activitylog/pkg/server/http/server.go | 19 ++++++++++++++++--- .../activitylog/pkg/service/events/service.go | 2 +- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/services/activitylog/pkg/server/http/server.go b/services/activitylog/pkg/server/http/server.go index 9a2493560a..ddaeac9793 100644 --- a/services/activitylog/pkg/server/http/server.go +++ b/services/activitylog/pkg/server/http/server.go @@ -4,6 +4,7 @@ import ( "context" "embed" "encoding/json" + "errors" "net/http" "github.com/go-chi/chi/v5" @@ -17,6 +18,7 @@ import ( ohttp "github.com/opencloud-eu/opencloud/pkg/service/http" "github.com/opencloud-eu/opencloud/pkg/version" settingssvc "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/services/settings/v0" + "github.com/opencloud-eu/opencloud/services/activitylog/pkg/apierrors" revactx "github.com/opencloud-eu/reva/v2/pkg/ctx" "go-micro.dev/v4" "google.golang.org/grpc/metadata" @@ -121,9 +123,20 @@ func GetItemActivitiesHandler(log log.Logger, s ActivityLogService, vc settingss activities, err := s.GetItemActivities(ctx, r.URL.Query().Get("kql"), loc, t) if err != nil { - log.Error().Err(err).Msg("error getting activities") - w.WriteHeader(http.StatusInternalServerError) - return + switch { + case errors.Is(err, apierrors.ErrBadRequest): + log.Debug().Str("query", r.URL.Query().Get("kql")).Err(err).Msg("error getting activities") + w.WriteHeader(http.StatusBadRequest) + return + case errors.Is(err, apierrors.ErrForbidden): + log.Debug().Err(err).Msg("error getting activities") + w.WriteHeader(http.StatusForbidden) + return + default: + log.Error().Err(err).Msg("error getting activities") + w.WriteHeader(http.StatusInternalServerError) + return + } } res := GetActivitiesResponse{ Activities: activities, diff --git a/services/activitylog/pkg/service/events/service.go b/services/activitylog/pkg/service/events/service.go index 60678d5085..34cacf42d0 100644 --- a/services/activitylog/pkg/service/events/service.go +++ b/services/activitylog/pkg/service/events/service.go @@ -76,7 +76,7 @@ func New(al *activitylog.ActivityLog, stream events.Stream, opts ...Option) (*Ac // Run to fulfil Runner interface func (s *ActivitylogService) Run() error { - ch, err := events.Consume(s.stream, "activitylog-pull", s.events...) + ch, err := events.Consume(s.stream, "activitylog", s.events...) if err != nil { return err } From 7dd9b3a0a07dc718de457942b86f5df532d0a8b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Fri, 14 Aug 2026 10:07:41 +0200 Subject: [PATCH 09/11] use json encode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- services/activitylog/pkg/server/http/server.go | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/services/activitylog/pkg/server/http/server.go b/services/activitylog/pkg/server/http/server.go index ddaeac9793..7eb6b05e31 100644 --- a/services/activitylog/pkg/server/http/server.go +++ b/services/activitylog/pkg/server/http/server.go @@ -142,23 +142,16 @@ func GetItemActivitiesHandler(log log.Logger, s ActivityLogService, vc settingss Activities: activities, } - b, err := json.Marshal(res) - if err != nil { - log.Error().Err(err).Msg("error marshalling activities") - w.WriteHeader(http.StatusInternalServerError) - return - } - w.Header().Set("Content-Type", "application/json; odata.metadata=minimal") w.Header().Set("OData-Version", "4.0") if reqID := chimiddleware.GetReqID(ctx); reqID != "" { w.Header().Set("request-id", reqID) } w.Header().Set("Cache-Control", "no-cache") - - w.WriteHeader(http.StatusOK) - if _, err := w.Write(b); err != nil { - log.Error().Err(err).Msg("error writing response") + if err := json.NewEncoder(w).Encode(res); err != nil { + log.Error().Err(err).Msg("error encoding activities") + w.WriteHeader(http.StatusInternalServerError) + return } } } From 5068e04200fe581ae37822d3ca4c46134c31dc85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Fri, 14 Aug 2026 13:29:20 +0200 Subject: [PATCH 10/11] reduce packages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- services/activitylog/pkg/apierrors/errors.go | 12 ------ services/activitylog/pkg/data/rawactivity.go | 10 ----- .../activitylog/pkg/server/http/server.go | 6 +-- .../pkg/service/activitylog/activitylog.go | 24 +++++++---- .../service/activitylog/activitylog_test.go | 13 +++--- .../pkg/service/activitylog/debouncer.go | 14 +++---- .../pkg/service/activitylog/debouncer_test.go | 41 +++++++++---------- .../pkg/service/activitylog/migrations.go | 3 +- .../activitylog/pkg/service/events/service.go | 5 +-- .../activitylog/pkg/service/http/service.go | 25 ++++++----- 10 files changed, 67 insertions(+), 86 deletions(-) delete mode 100644 services/activitylog/pkg/apierrors/errors.go delete mode 100644 services/activitylog/pkg/data/rawactivity.go diff --git a/services/activitylog/pkg/apierrors/errors.go b/services/activitylog/pkg/apierrors/errors.go deleted file mode 100644 index 25eaf42031..0000000000 --- a/services/activitylog/pkg/apierrors/errors.go +++ /dev/null @@ -1,12 +0,0 @@ -// apierrors package defines common business API errors that can be used across the service. It is intended to be used by both the service and the API layer to ensure consistent error handling and messaging. -package apierrors - -import "errors" - -var ( - ErrNotFound = errors.New("query target not found") - ErrBadRequest = errors.New("bad request") - ErrUnauthorized = errors.New("unauthorized") - ErrForbidden = errors.New("forbidden") - ErrMissingEmail = errors.New("missing email address") -) diff --git a/services/activitylog/pkg/data/rawactivity.go b/services/activitylog/pkg/data/rawactivity.go deleted file mode 100644 index ac77d539ed..0000000000 --- a/services/activitylog/pkg/data/rawactivity.go +++ /dev/null @@ -1,10 +0,0 @@ -package data - -import "time" - -// RawActivity represents an activity as it is stored in the activitylog store -type RawActivity struct { - EventID string `json:"event_id"` - Depth int `json:"depth"` - Timestamp time.Time `json:"timestamp"` -} diff --git a/services/activitylog/pkg/server/http/server.go b/services/activitylog/pkg/server/http/server.go index 7eb6b05e31..f143f88953 100644 --- a/services/activitylog/pkg/server/http/server.go +++ b/services/activitylog/pkg/server/http/server.go @@ -18,7 +18,7 @@ import ( ohttp "github.com/opencloud-eu/opencloud/pkg/service/http" "github.com/opencloud-eu/opencloud/pkg/version" settingssvc "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/services/settings/v0" - "github.com/opencloud-eu/opencloud/services/activitylog/pkg/apierrors" + activityloghttp "github.com/opencloud-eu/opencloud/services/activitylog/pkg/service/http" revactx "github.com/opencloud-eu/reva/v2/pkg/ctx" "go-micro.dev/v4" "google.golang.org/grpc/metadata" @@ -124,11 +124,11 @@ func GetItemActivitiesHandler(log log.Logger, s ActivityLogService, vc settingss activities, err := s.GetItemActivities(ctx, r.URL.Query().Get("kql"), loc, t) if err != nil { switch { - case errors.Is(err, apierrors.ErrBadRequest): + case errors.Is(err, activityloghttp.ErrBadRequest): log.Debug().Str("query", r.URL.Query().Get("kql")).Err(err).Msg("error getting activities") w.WriteHeader(http.StatusBadRequest) return - case errors.Is(err, apierrors.ErrForbidden): + case errors.Is(err, activityloghttp.ErrForbidden): log.Debug().Err(err).Msg("error getting activities") w.WriteHeader(http.StatusForbidden) return diff --git a/services/activitylog/pkg/service/activitylog/activitylog.go b/services/activitylog/pkg/service/activitylog/activitylog.go index 50cba5130d..6e64dab5f5 100644 --- a/services/activitylog/pkg/service/activitylog/activitylog.go +++ b/services/activitylog/pkg/service/activitylog/activitylog.go @@ -15,7 +15,6 @@ import ( "github.com/jellydator/ttlcache/v2" "github.com/nats-io/nats.go" "github.com/opencloud-eu/opencloud/pkg/log" - "github.com/opencloud-eu/opencloud/services/activitylog/pkg/data" "github.com/opencloud-eu/reva/v2/pkg/storagespace" "github.com/vmihailenco/msgpack/v5" "go.opentelemetry.io/otel" @@ -104,6 +103,13 @@ func (a *ActivityLog) RemoveResource(rid *provider.ResourceId) error { return nil } +// RawActivity represents an activity as it is stored in the activitylog store +type RawActivity struct { + EventID string `json:"event_id"` + Depth int `json:"depth"` + Timestamp time.Time `json:"timestamp"` +} + func (a *ActivityLog) AddActivity(ctx context.Context, initRef *provider.Reference, parentId *provider.ResourceId, eventID string, timestamp time.Time, getResource func(context.Context, *provider.Reference) (*provider.ResourceInfo, error)) error { var ( err error @@ -131,7 +137,7 @@ func (a *ActivityLog) AddActivity(ctx context.Context, initRef *provider.Referen } key := storagespace.FormatResourceID(id) - a.debouncer.Debounce(key, data.RawActivity{ + a.debouncer.Debounce(key, RawActivity{ EventID: eventID, Depth: depth, Timestamp: timestamp, @@ -172,7 +178,7 @@ func (a *ActivityLog) AddActivity(ctx context.Context, initRef *provider.Referen return nil } -func (a *ActivityLog) StoreActivity(resourceID string, activities []data.RawActivity) error { +func (a *ActivityLog) StoreActivity(resourceID string, activities []RawActivity) error { a.lock.Lock() defer a.lock.Unlock() @@ -221,7 +227,7 @@ func (a *ActivityLog) enforceMaxActivities(ctx context.Context, resourceID strin break } - var batchActivities []data.RawActivity + var batchActivities []RawActivity if err := msgpack.Unmarshal(update.Value(), &batchActivities); err != nil { a.log.Debug().Err(err).Str("resourceID", resourceID).Msg("could not unmarshal messagepack, trying json") } @@ -297,14 +303,14 @@ func natsKey(resourceID string, activitiesCount int) string { time.Now().UnixNano()) } -func (a *ActivityLog) Activities(rid *provider.ResourceId) ([]data.RawActivity, error) { +func (a *ActivityLog) Activities(rid *provider.ResourceId) ([]RawActivity, error) { a.lock.RLock() defer a.lock.RUnlock() return a.activities(rid) } -func (a *ActivityLog) activities(rid *provider.ResourceId) ([]data.RawActivity, error) { +func (a *ActivityLog) activities(rid *provider.ResourceId) ([]RawActivity, error) { resourceID := storagespace.FormatResourceID(rid) glob := fmt.Sprintf("%s.>", base32.StdEncoding.EncodeToString([]byte(resourceID))) @@ -315,13 +321,13 @@ func (a *ActivityLog) activities(rid *provider.ResourceId) ([]data.RawActivity, } defer watcher.Stop() - var activities []data.RawActivity + var activities []RawActivity for update := range watcher.Updates() { if update == nil { break } - var batchActivities []data.RawActivity + var batchActivities []RawActivity if err := msgpack.Unmarshal(update.Value(), &batchActivities); err != nil { a.log.Debug().Err(err).Str("resourceID", resourceID).Msg("could not unmarshal messagepack") } @@ -341,7 +347,7 @@ func (a *ActivityLog) RemoveActivities(rid *provider.ResourceId, toDelete map[st return err } - var acts []data.RawActivity + var acts []RawActivity for _, a := range curActivities { if _, ok := toDelete[a.EventID]; !ok { acts = append(acts, a) diff --git a/services/activitylog/pkg/service/activitylog/activitylog_test.go b/services/activitylog/pkg/service/activitylog/activitylog_test.go index c968a935ec..65f4da1240 100644 --- a/services/activitylog/pkg/service/activitylog/activitylog_test.go +++ b/services/activitylog/pkg/service/activitylog/activitylog_test.go @@ -14,7 +14,6 @@ import ( . "github.com/onsi/gomega" "github.com/opencloud-eu/opencloud/services/activitylog/pkg/command" "github.com/opencloud-eu/opencloud/services/activitylog/pkg/config" - "github.com/opencloud-eu/opencloud/services/activitylog/pkg/data" "github.com/opencloud-eu/opencloud/services/activitylog/pkg/service/activitylog" ) @@ -97,7 +96,7 @@ var _ = Describe("ActivitylogService", func() { Name string Tree map[string]*provider.ResourceInfo Activities map[string]string - Expected map[string][]data.RawActivity + Expected map[string][]activitylog.RawActivity } testCases := []testCase{ @@ -111,7 +110,7 @@ var _ = Describe("ActivitylogService", func() { Activities: map[string]string{ "activity": "base", }, - Expected: map[string][]data.RawActivity{ + Expected: map[string][]activitylog.RawActivity{ "base": activitites("activity", 0), "parent": activitites("activity", 1), "spaceid": activitites("activity", 2), @@ -128,7 +127,7 @@ var _ = Describe("ActivitylogService", func() { "activity1": "base", "activity2": "base", }, - Expected: map[string][]data.RawActivity{ + Expected: map[string][]activitylog.RawActivity{ "base": activitites("activity1", 0, "activity2", 0), "parent": activitites("activity1", 1, "activity2", 1), "spaceid": activitites("activity1", 2, "activity2", 2), @@ -260,9 +259,9 @@ var _ = Describe("ActivitylogService", func() { }) }) -func activitites(acts ...any) []data.RawActivity { - var activities []data.RawActivity - act := data.RawActivity{} +func activitites(acts ...any) []activitylog.RawActivity { + var activities []activitylog.RawActivity + act := activitylog.RawActivity{} for _, a := range acts { switch v := a.(type) { case string: diff --git a/services/activitylog/pkg/service/activitylog/debouncer.go b/services/activitylog/pkg/service/activitylog/debouncer.go index 5c60923462..bbb4613b2f 100644 --- a/services/activitylog/pkg/service/activitylog/debouncer.go +++ b/services/activitylog/pkg/service/activitylog/debouncer.go @@ -3,14 +3,12 @@ package activitylog import ( "sync" "time" - - "github.com/opencloud-eu/opencloud/services/activitylog/pkg/data" ) // Debouncer is used to debounce writes to the activity log store. type Debouncer struct { after time.Duration - f func(id string, ra []data.RawActivity) error + f func(id string, ra []RawActivity) error pending sync.Map inProgress sync.Map @@ -18,12 +16,12 @@ type Debouncer struct { } type queueItem struct { - activities []data.RawActivity + activities []RawActivity timer *time.Timer } // NewDebouncer returns a new Debouncer instance. -func NewDebouncer(d time.Duration, f func(id string, ra []data.RawActivity) error) *Debouncer { +func NewDebouncer(d time.Duration, f func(id string, ra []RawActivity) error) *Debouncer { return &Debouncer{ after: d, f: f, @@ -33,9 +31,9 @@ func NewDebouncer(d time.Duration, f func(id string, ra []data.RawActivity) erro } // Debounce restarts the debounce timer for the given space. -func (d *Debouncer) Debounce(id string, ra data.RawActivity) { +func (d *Debouncer) Debounce(id string, ra RawActivity) { if d.after == 0 { - d.f(id, []data.RawActivity{ra}) + d.f(id, []RawActivity{ra}) return } @@ -43,7 +41,7 @@ func (d *Debouncer) Debounce(id string, ra data.RawActivity) { defer d.mutex.Unlock() item := &queueItem{ - activities: []data.RawActivity{ra}, + activities: []RawActivity{ra}, } if i, ok := d.pending.Load(id); ok { diff --git a/services/activitylog/pkg/service/activitylog/debouncer_test.go b/services/activitylog/pkg/service/activitylog/debouncer_test.go index 5a44d33d6c..cbf178a836 100644 --- a/services/activitylog/pkg/service/activitylog/debouncer_test.go +++ b/services/activitylog/pkg/service/activitylog/debouncer_test.go @@ -4,24 +4,23 @@ import ( "sync" "time" - "github.com/opencloud-eu/opencloud/services/activitylog/pkg/data" - "github.com/opencloud-eu/opencloud/services/activitylog/pkg/service/activitylog" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "github.com/opencloud-eu/opencloud/services/activitylog/pkg/service/activitylog" ) var _ = Describe("Debouncer", func() { var ( - mu sync.Mutex - callbacks []data.RawActivity - newCallback func(id string, ra []data.RawActivity) error + mu sync.Mutex + callbacks []activitylog.RawActivity + newCallback func(id string, ra []activitylog.RawActivity) error ) BeforeEach(func() { mu.Lock() callbacks = nil mu.Unlock() - newCallback = func(id string, ra []data.RawActivity) error { + newCallback = func(id string, ra []activitylog.RawActivity) error { mu.Lock() defer mu.Unlock() callbacks = append(callbacks, ra...) @@ -32,15 +31,15 @@ var _ = Describe("Debouncer", func() { Context("with zero duration", func() { It("calls the callback immediately", func() { d := activitylog.NewDebouncer(0, newCallback) - d.Debounce("space1", data.RawActivity{EventID: "activity1"}) + d.Debounce("space1", activitylog.RawActivity{EventID: "activity1"}) Expect(callbacks).To(HaveLen(1)) Expect(callbacks[0].EventID).To(Equal("activity1")) }) It("calls the callback immediately for each event", func() { d := activitylog.NewDebouncer(0, newCallback) - d.Debounce("space1", data.RawActivity{EventID: "activity1"}) - d.Debounce("space2", data.RawActivity{EventID: "activity2"}) + d.Debounce("space1", activitylog.RawActivity{EventID: "activity1"}) + d.Debounce("space2", activitylog.RawActivity{EventID: "activity2"}) Expect(callbacks).To(HaveLen(2)) }) }) @@ -48,9 +47,9 @@ var _ = Describe("Debouncer", func() { Context("with non-zero duration", func() { It("batches activities with the same id", func() { d := activitylog.NewDebouncer(10*time.Millisecond, newCallback) - d.Debounce("space1", data.RawActivity{EventID: "activity1"}) - d.Debounce("space1", data.RawActivity{EventID: "activity2"}) - d.Debounce("space1", data.RawActivity{EventID: "activity3"}) + d.Debounce("space1", activitylog.RawActivity{EventID: "activity1"}) + d.Debounce("space1", activitylog.RawActivity{EventID: "activity2"}) + d.Debounce("space1", activitylog.RawActivity{EventID: "activity3"}) Eventually(func() int { mu.Lock() @@ -61,8 +60,8 @@ var _ = Describe("Debouncer", func() { It("handles different ids independently", func() { d := activitylog.NewDebouncer(10*time.Millisecond, newCallback) - d.Debounce("space1", data.RawActivity{EventID: "activity1"}) - d.Debounce("space2", data.RawActivity{EventID: "activity2"}) + d.Debounce("space1", activitylog.RawActivity{EventID: "activity1"}) + d.Debounce("space2", activitylog.RawActivity{EventID: "activity2"}) Eventually(func() int { mu.Lock() @@ -73,11 +72,11 @@ var _ = Describe("Debouncer", func() { It("batches activities that arrive within the debounce window", func() { d := activitylog.NewDebouncer(100*time.Millisecond, newCallback) - d.Debounce("space1", data.RawActivity{EventID: "activity1"}) + d.Debounce("space1", activitylog.RawActivity{EventID: "activity1"}) time.Sleep(20 * time.Millisecond) - d.Debounce("space1", data.RawActivity{EventID: "activity2"}) + d.Debounce("space1", activitylog.RawActivity{EventID: "activity2"}) time.Sleep(20 * time.Millisecond) - d.Debounce("space1", data.RawActivity{EventID: "activity3"}) + d.Debounce("space1", activitylog.RawActivity{EventID: "activity3"}) Eventually(func() int { mu.Lock() @@ -88,9 +87,9 @@ var _ = Describe("Debouncer", func() { It("processes new batch after previous completes", func() { d := activitylog.NewDebouncer(5*time.Millisecond, newCallback) - d.Debounce("space1", data.RawActivity{EventID: "activity1"}) + d.Debounce("space1", activitylog.RawActivity{EventID: "activity1"}) time.Sleep(20 * time.Millisecond) // let first batch complete - d.Debounce("space1", data.RawActivity{EventID: "activity2"}) + d.Debounce("space1", activitylog.RawActivity{EventID: "activity2"}) Eventually(func() int { mu.Lock() @@ -100,7 +99,7 @@ var _ = Describe("Debouncer", func() { }) It("skips duplicate write when timer fires during in-progress callback", func() { - slowCallback := func(id string, ra []data.RawActivity) error { + slowCallback := func(id string, ra []activitylog.RawActivity) error { time.Sleep(50 * time.Millisecond) // simulate slow write mu.Lock() defer mu.Unlock() @@ -109,7 +108,7 @@ var _ = Describe("Debouncer", func() { } d := activitylog.NewDebouncer(10*time.Millisecond, slowCallback) - d.Debounce("space1", data.RawActivity{EventID: "activity1"}) + d.Debounce("space1", activitylog.RawActivity{EventID: "activity1"}) time.Sleep(20 * time.Millisecond) // timer fires while callback is running Eventually(func() int { diff --git a/services/activitylog/pkg/service/activitylog/migrations.go b/services/activitylog/pkg/service/activitylog/migrations.go index 1b7e6d5c15..dca612e6b6 100644 --- a/services/activitylog/pkg/service/activitylog/migrations.go +++ b/services/activitylog/pkg/service/activitylog/migrations.go @@ -7,7 +7,6 @@ import ( "log" "github.com/nats-io/nats.go" - "github.com/opencloud-eu/opencloud/services/activitylog/pkg/data" "github.com/vmihailenco/msgpack/v5" ) @@ -84,7 +83,7 @@ func (a *ActivityLog) migrateToV1(_ context.Context, kv nats.KeyValue) error { } // Unmarshal value into a list of strings - var activities []data.RawActivity + var activities []RawActivity if err := msgpack.Unmarshal(val.Data, &activities); err != nil { if err := json.Unmarshal(val.Data, &activities); err != nil { // This key's value is not a JSON array of strings. Skip it. diff --git a/services/activitylog/pkg/service/events/service.go b/services/activitylog/pkg/service/events/service.go index 34cacf42d0..3f61c65527 100644 --- a/services/activitylog/pkg/service/events/service.go +++ b/services/activitylog/pkg/service/events/service.go @@ -12,7 +12,6 @@ import ( provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1" "github.com/opencloud-eu/opencloud/pkg/log" "github.com/opencloud-eu/opencloud/services/activitylog/pkg/config" - "github.com/opencloud-eu/opencloud/services/activitylog/pkg/data" "github.com/opencloud-eu/opencloud/services/activitylog/pkg/service/activitylog" "github.com/opencloud-eu/reva/v2/pkg/events" "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" @@ -221,7 +220,7 @@ func (a *ActivitylogService) AddActivityTrashed(ctx context.Context, resourceID } // store activity on trashed item - if err := a.al.StoreActivity(storagespace.FormatResourceID(resourceID), []data.RawActivity{ + if err := a.al.StoreActivity(storagespace.FormatResourceID(resourceID), []activitylog.RawActivity{ { EventID: eventID, Depth: 0, @@ -254,7 +253,7 @@ func (a *ActivitylogService) AddSpaceActivity(ctx context.Context, spaceID *prov return fmt.Errorf("could not parse space id: %w", err) } rid.OpaqueId = rid.GetSpaceId() - err = a.al.StoreActivity(storagespace.FormatResourceID(&rid), []data.RawActivity{ + err = a.al.StoreActivity(storagespace.FormatResourceID(&rid), []activitylog.RawActivity{ { EventID: eventID, Depth: 0, diff --git a/services/activitylog/pkg/service/http/service.go b/services/activitylog/pkg/service/http/service.go index e86c74c148..e552225391 100644 --- a/services/activitylog/pkg/service/http/service.go +++ b/services/activitylog/pkg/service/http/service.go @@ -19,8 +19,6 @@ import ( "github.com/opencloud-eu/opencloud/pkg/log" ehmsg "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/messages/eventhistory/v0" ehsvc "github.com/opencloud-eu/opencloud/protogen/gen/opencloud/services/eventhistory/v0" - "github.com/opencloud-eu/opencloud/services/activitylog/pkg/apierrors" - "github.com/opencloud-eu/opencloud/services/activitylog/pkg/data" "github.com/opencloud-eu/opencloud/services/activitylog/pkg/service/activitylog" "github.com/opencloud-eu/reva/v2/pkg/events" "github.com/opencloud-eu/reva/v2/pkg/rgrpc/todo/pool" @@ -63,6 +61,11 @@ type svc struct { registeredEvents map[string]events.Unmarshaller } +var ( + ErrBadRequest = errors.New("bad request") + ErrForbidden = errors.New("forbidden") +) + func (s *svc) GetItemActivities(ctx context.Context, query, loc string, t l10n.Translator) ([]libregraph.Activity, error) { gwc, err := s.gws.Next() if err != nil { @@ -72,17 +75,17 @@ func (s *svc) GetItemActivities(ctx context.Context, query, loc string, t l10n.T rid, limit, rawActivityAccepted, activityAccepted, sort, err := s.getFilters(query) if err != nil { s.log.Info().Str("query", query).Err(err).Msg("error getting filters") - return nil, apierrors.ErrBadRequest + return nil, ErrBadRequest } info, err := utils.GetResourceByID(ctx, rid, gwc) if err != nil { - return nil, apierrors.ErrForbidden + return nil, ErrForbidden } // you need ListGrants to see activities if !info.GetPermissionSet().GetListGrants() { - return nil, apierrors.ErrForbidden + return nil, ErrForbidden } raw, err := s.al.Activities(rid) @@ -264,13 +267,13 @@ func (s *svc) unwrapEvent(e *ehmsg.Event) any { return einterface } -func (s *svc) getFilters(query string) (*provider.ResourceId, int, func(data.RawActivity) bool, func(*ehmsg.Event) bool, func([]*ehmsg.Event), error) { +func (s *svc) getFilters(query string) (*provider.ResourceId, int, func(activitylog.RawActivity) bool, func(*ehmsg.Event) bool, func([]*ehmsg.Event), error) { qast, err := kql.Builder{}.Build(query) if err != nil { return nil, 0, nil, nil, nil, err } - prefilters := make([]func(data.RawActivity) bool, 0) + prefilters := make([]func(activitylog.RawActivity) bool, 0) postfilters := make([]func(*ehmsg.Event) bool, 0) sortby := func(_ []*ehmsg.Event) {} @@ -295,7 +298,7 @@ func (s *svc) getFilters(query string) (*provider.ResourceId, int, func(data.Raw break } - prefilters = append(prefilters, func(a data.RawActivity) bool { + prefilters = append(prefilters, func(a activitylog.RawActivity) bool { return a.Depth <= depth }) case "limit": @@ -318,11 +321,11 @@ func (s *svc) getFilters(query string) (*provider.ResourceId, int, func(data.Raw case *ast.DateTimeNode: switch v.Operator.Value { case "<", "<=": - prefilters = append(prefilters, func(a data.RawActivity) bool { + prefilters = append(prefilters, func(a activitylog.RawActivity) bool { return a.Timestamp.Before(v.Value) }) case ">", ">=": - prefilters = append(prefilters, func(a data.RawActivity) bool { + prefilters = append(prefilters, func(a activitylog.RawActivity) bool { return a.Timestamp.After(v.Value) }) } @@ -341,7 +344,7 @@ func (s *svc) getFilters(query string) (*provider.ResourceId, int, func(data.Raw // space root requested - fix format rid.OpaqueId = rid.GetSpaceId() } - pref := func(a data.RawActivity) bool { + pref := func(a activitylog.RawActivity) bool { for _, f := range prefilters { if !f(a) { return false From b2fb6b8bc9097f22c537af0fb0284db192d89149 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn=20Friedrich=20Dreyer?= Date: Fri, 14 Aug 2026 13:38:52 +0200 Subject: [PATCH 11/11] simplify ConnectNatsKV MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jörn Friedrich Dreyer --- services/activitylog/pkg/command/server.go | 31 +++++++--------------- 1 file changed, 9 insertions(+), 22 deletions(-) diff --git a/services/activitylog/pkg/command/server.go b/services/activitylog/pkg/command/server.go index b417b9ddde..1010b8d9da 100644 --- a/services/activitylog/pkg/command/server.go +++ b/services/activitylog/pkg/command/server.go @@ -2,8 +2,8 @@ package command import ( "context" - "crypto/tls" "fmt" + "strings" "github.com/nats-io/nats.go" "github.com/olekukonko/errors" @@ -12,6 +12,7 @@ import ( "github.com/opencloud-eu/opencloud/pkg/config/configlog" "github.com/opencloud-eu/opencloud/pkg/generators" "github.com/opencloud-eu/opencloud/pkg/log" + natspkg "github.com/opencloud-eu/opencloud/pkg/nats" "github.com/opencloud-eu/opencloud/pkg/registry" "github.com/opencloud-eu/opencloud/pkg/runner" ogrpc "github.com/opencloud-eu/opencloud/pkg/service/grpc" @@ -185,11 +186,11 @@ func Server(cfg *config.Config) *cobra.Command { return err } - gr.Add(runner.New(cfg.Service.Name+".svc", func() error { - return eventSvc.Run() - }, func() { - eventSvc.Close() - })) + gr.Add(runner.New(cfg.Service.Name+".svc", func() error { + return eventSvc.Run() + }, func() { + eventSvc.Close() + })) } else { logger.Info().Msg("event listening disabled, not starting event service") } @@ -223,22 +224,8 @@ func Server(cfg *config.Config) *cobra.Command { func ConnectNatsKV(cfg config.Store) (nats.KeyValue, error) { // Connect to NATS servers - natsOptions := nats.Options{ - Servers: cfg.Nodes, - } - if cfg.EnableTLS { - if cfg.TLSRootCACertificate != "" { - // when root ca is configured use it. an insecure flag is ignored. - nats.RootCAs(cfg.TLSRootCACertificate)(&natsOptions) - } else { - // enable tls and use insecure flag - nats.Secure(&tls.Config{MinVersion: tls.VersionTLS12, InsecureSkipVerify: cfg.TLSInsecure})(&natsOptions) - } - } - if cfg.AuthUsername != "" && cfg.AuthPassword != "" { - nats.UserInfo(cfg.AuthUsername, cfg.AuthPassword)(&natsOptions) - } - conn, err := natsOptions.Connect() + secureOption := natspkg.Secure(cfg.EnableTLS, cfg.TLSInsecure, cfg.TLSRootCACertificate) + conn, err := nats.Connect(strings.Join(cfg.Nodes, ","), secureOption, nats.UserInfo(cfg.AuthUsername, cfg.AuthPassword)) if err != nil { return nil, err }