From 0ba962bda1a2112af608fec18e4c99bb81dfb481 Mon Sep 17 00:00:00 2001 From: Pascal Bleser Date: Tue, 29 Jul 2025 15:49:38 +0200 Subject: [PATCH] groupware: refactoring the API mechanisms --- pkg/jmap/jmap_http.go | 10 ++ services/groupware/pkg/config/config.go | 23 ++-- .../pkg/config/defaults/defaultconfig.go | 17 +-- services/groupware/pkg/groupware/groupware.go | 54 +++------ ...upware_api_error.go => groupware_error.go} | 0 .../pkg/groupware/groupware_lowlevel.go | 110 +++++++----------- .../pkg/groupware/groupware_session.go | 66 +++++++++++ 7 files changed, 158 insertions(+), 122 deletions(-) rename services/groupware/pkg/groupware/{groupware_api_error.go => groupware_error.go} (100%) create mode 100644 services/groupware/pkg/groupware/groupware_session.go diff --git a/pkg/jmap/jmap_http.go b/pkg/jmap/jmap_http.go index cb8d4faa8f..34c24a02c2 100644 --- a/pkg/jmap/jmap_http.go +++ b/pkg/jmap/jmap_http.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "net/http" + "net/http/httputil" "net/url" "github.com/opencloud-eu/opencloud/pkg/log" @@ -127,6 +128,15 @@ func (h *HttpJmapApiClient) Command(ctx context.Context, logger *log.Logger, ses req.Header.Add("User-Agent", h.userAgent) h.auth(session.Username, logger, req) + { + if logger.Trace().Enabled() { + safereq := req.Clone(ctx) + safereq.Header.Set("Authorization", "***") + bytes, _ := httputil.DumpRequest(safereq, false) + logger.Info().Msgf("sending command: %s", string(bytes)) + } + } + res, err := h.client.Do(req) if err != nil { logger.Error().Err(err).Msgf("failed to perform POST %v", jmapUrl) diff --git a/services/groupware/pkg/config/config.go b/services/groupware/pkg/config/config.go index 149bbd96c5..0c66c7e172 100644 --- a/services/groupware/pkg/config/config.go +++ b/services/groupware/pkg/config/config.go @@ -31,13 +31,18 @@ type MailMasterAuth struct { Password string `yaml:"password" env:"GROUPWARE_JMAP_MASTER_PASSWORD"` } -type Mail struct { - Master MailMasterAuth `yaml:"master"` - BaseUrl string `yaml:"base_url" env:"GROUPWARE_JMAP_BASE_URL"` - Timeout time.Duration `yaml:"timeout" env:"GROUPWARE_JMAP_TIMEOUT"` - DefaultEmailLimit int `yaml:"default_email_limit" env:"GROUPWARE_DEFAULT_EMAIL_LIMIT"` - MaxBodyValueBytes int `yaml:"max_body_value_bytes" env:"GROUPWARE_MAX_BODY_VALUE_BYTES"` - ResponseHeaderTimeout time.Duration `yaml:"response_header_timeout" env:"GROUPWARE_RESPONSE_HEADER_TIMEOUT"` - SessionCacheTtl time.Duration `yaml:"session_cache_ttl" env:"GROUPWARE_SESSION_CACHE_TTL"` - SessionFailureCacheTtl time.Duration `yaml:"session_failure_cache_ttl" env:"GROUPWARE_SESSION_FAILURE_CACHE_TTL"` +type MailSessionCache struct { + MaxCapacity int `yaml:"max_capacity" env:"GROUPWARE_SESSION_CACHE_MAX_CAPACITY"` + Ttl time.Duration `yaml:"ttl" env:"GROUPWARE_SESSION_CACHE_TTL"` + FailureTtl time.Duration `yaml:"failure_ttl" env:"GROUPWARE_SESSION_FAILURE_CACHE_TTL"` +} + +type Mail struct { + Master MailMasterAuth `yaml:"master"` + BaseUrl string `yaml:"base_url" env:"GROUPWARE_JMAP_BASE_URL"` + Timeout time.Duration `yaml:"timeout" env:"GROUPWARE_JMAP_TIMEOUT"` + DefaultEmailLimit int `yaml:"default_email_limit" env:"GROUPWARE_DEFAULT_EMAIL_LIMIT"` + MaxBodyValueBytes int `yaml:"max_body_value_bytes" env:"GROUPWARE_MAX_BODY_VALUE_BYTES"` + ResponseHeaderTimeout time.Duration `yaml:"response_header_timeout" env:"GROUPWARE_RESPONSE_HEADER_TIMEOUT"` + SessionCache MailSessionCache `yaml:"session_cache"` } diff --git a/services/groupware/pkg/config/defaults/defaultconfig.go b/services/groupware/pkg/config/defaults/defaultconfig.go index 1d63e4d22b..d92f8f13f3 100644 --- a/services/groupware/pkg/config/defaults/defaultconfig.go +++ b/services/groupware/pkg/config/defaults/defaultconfig.go @@ -29,13 +29,16 @@ func DefaultConfig() *config.Config { Username: "master", Password: "admin", }, - BaseUrl: "https://stalwart.opencloud.test", - Timeout: 30 * time.Second, - DefaultEmailLimit: -1, - MaxBodyValueBytes: -1, - ResponseHeaderTimeout: 10 * time.Second, - SessionCacheTtl: 30 * time.Second, - SessionFailureCacheTtl: 15 * time.Second, + BaseUrl: "https://stalwart.opencloud.test", + Timeout: 30 * time.Second, + DefaultEmailLimit: -1, + MaxBodyValueBytes: -1, + ResponseHeaderTimeout: 10 * time.Second, + SessionCache: config.MailSessionCache{ + Ttl: 30 * time.Second, + FailureTtl: 15 * time.Second, + MaxCapacity: 10_000, + }, }, HTTP: config.HTTP{ Addr: "127.0.0.1:9276", diff --git a/services/groupware/pkg/groupware/groupware.go b/services/groupware/pkg/groupware/groupware.go index 4a2f729f5a..808a1d78f9 100644 --- a/services/groupware/pkg/groupware/groupware.go +++ b/services/groupware/pkg/groupware/groupware.go @@ -1,12 +1,10 @@ package groupware import ( - "context" "net/http" "strconv" "github.com/go-chi/chi/v5" - "github.com/go-chi/render" "github.com/opencloud-eu/opencloud/pkg/jmap" "github.com/opencloud-eu/opencloud/pkg/log" @@ -30,34 +28,21 @@ func (IndexResponse) Render(w http.ResponseWriter, r *http.Request) error { } func (g Groupware) Index(w http.ResponseWriter, r *http.Request) { - ctx := r.Context() - logger := g.logger.SubloggerWithRequestID(ctx) - - session, ok, err := g.session(r, ctx, &logger) - if err != nil { - w.WriteHeader(http.StatusInternalServerError) - return - } - if ok { - //logger = session.DecorateLogger(logger) - _ = render.Render(w, r, IndexResponse{AccountId: session.AccountId}) - } else { - w.WriteHeader(http.StatusInternalServerError) - return - } - + g.respond(w, r, func(req Request) (any, string, *ApiError) { + return IndexResponse{AccountId: req.session.AccountId}, "", nil + }) } func (g Groupware) GetIdentity(w http.ResponseWriter, r *http.Request) { - g.respond(w, r, func(r *http.Request, ctx context.Context, logger *log.Logger, session *jmap.Session) (any, string, *ApiError) { - res, err := g.jmap.GetIdentity(session, ctx, logger) + g.respond(w, r, func(req Request) (any, string, *ApiError) { + res, err := g.jmap.GetIdentity(req.session, req.ctx, req.logger) return res, res.State, apiErrorFromJmap(err) }) } func (g Groupware) GetVacation(w http.ResponseWriter, r *http.Request) { - g.respond(w, r, func(r *http.Request, ctx context.Context, logger *log.Logger, session *jmap.Session) (any, string, *ApiError) { - res, err := g.jmap.GetVacationResponse(session, ctx, logger) + g.respond(w, r, func(req Request) (any, string, *ApiError) { + res, err := g.jmap.GetVacationResponse(req.session, req.ctx, req.logger) return res, res.State, apiErrorFromJmap(err) }) } @@ -69,8 +54,8 @@ func (g Groupware) GetMailboxById(w http.ResponseWriter, r *http.Request) { return } - g.respond(w, r, func(r *http.Request, ctx context.Context, logger *log.Logger, session *jmap.Session) (any, string, *ApiError) { - res, err := g.jmap.GetMailbox(session, ctx, logger, []string{mailboxId}) + g.respond(w, r, func(req Request) (any, string, *ApiError) { + res, err := g.jmap.GetMailbox(req.session, req.ctx, req.logger, []string{mailboxId}) if err != nil { return res, "", apiErrorFromJmap(err) } @@ -109,29 +94,28 @@ func (g Groupware) GetMailboxes(w http.ResponseWriter, r *http.Request) { hasCriteria = true } - if hasCriteria { - g.respond(w, r, func(r *http.Request, ctx context.Context, logger *log.Logger, session *jmap.Session) (any, string, *ApiError) { - mailboxes, err := g.jmap.SearchMailboxes(session, ctx, logger, filter) + g.respond(w, r, func(req Request) (any, string, *ApiError) { + if hasCriteria { + mailboxes, err := g.jmap.SearchMailboxes(req.session, req.ctx, req.logger, filter) if err != nil { return nil, "", apiErrorFromJmap(err) } return mailboxes.Mailboxes, mailboxes.State, nil - }) - } else { - g.respond(w, r, func(r *http.Request, ctx context.Context, logger *log.Logger, session *jmap.Session) (any, string, *ApiError) { - mailboxes, err := g.jmap.GetAllMailboxes(session, ctx, logger) + } else { + mailboxes, err := g.jmap.GetAllMailboxes(req.session, req.ctx, req.logger) if err != nil { return nil, "", apiErrorFromJmap(err) } return mailboxes.List, mailboxes.State, nil - }) - } + } + }) } func (g Groupware) GetMessages(w http.ResponseWriter, r *http.Request) { mailboxId := chi.URLParam(r, "mailbox") - g.respond(w, r, func(r *http.Request, ctx context.Context, logger *log.Logger, session *jmap.Session) (any, string, *ApiError) { + g.respond(w, r, func(req Request) (any, string, *ApiError) { page, ok, _ := ParseNumericParam(r, "page", -1) + logger := req.logger if ok { logger = &log.Logger{Logger: logger.With().Int("page", page).Logger()} } @@ -146,7 +130,7 @@ func (g Groupware) GetMessages(w http.ResponseWriter, r *http.Request) { limit = g.defaultEmailLimit } - emails, err := g.jmap.GetEmails(session, ctx, logger, mailboxId, offset, limit, true, g.maxBodyValueBytes) + emails, err := g.jmap.GetEmails(req.session, req.ctx, logger, mailboxId, offset, limit, true, g.maxBodyValueBytes) if err != nil { return nil, "", apiErrorFromJmap(err) } diff --git a/services/groupware/pkg/groupware/groupware_api_error.go b/services/groupware/pkg/groupware/groupware_error.go similarity index 100% rename from services/groupware/pkg/groupware/groupware_api_error.go rename to services/groupware/pkg/groupware/groupware_error.go diff --git a/services/groupware/pkg/groupware/groupware_lowlevel.go b/services/groupware/pkg/groupware/groupware_lowlevel.go index 82d1d9c1f6..10d66b26bf 100644 --- a/services/groupware/pkg/groupware/groupware_lowlevel.go +++ b/services/groupware/pkg/groupware/groupware_lowlevel.go @@ -6,7 +6,6 @@ import ( "fmt" "net/http" "net/url" - "time" "github.com/go-chi/chi/v5" "github.com/go-chi/render" @@ -23,63 +22,6 @@ const ( logQuery = "query" ) -type cachedSession interface { - Success() bool - Get() jmap.Session - Error() error -} - -type succeededSession struct { - session jmap.Session -} - -func (s succeededSession) Success() bool { - return true -} -func (s succeededSession) Get() jmap.Session { - return s.session -} -func (s succeededSession) Error() error { - return nil -} - -var _ cachedSession = succeededSession{} - -type failedSession struct { - err error -} - -func (s failedSession) Success() bool { - return false -} -func (s failedSession) Get() jmap.Session { - panic("this should never be called") -} -func (s failedSession) Error() error { - return s.err -} - -var _ cachedSession = failedSession{} - -type sessionCacheLoader struct { - logger *log.Logger - jmapClient jmap.Client - errorTtl time.Duration -} - -func (l *sessionCacheLoader) Load(c *ttlcache.Cache[string, cachedSession], username string) *ttlcache.Item[string, cachedSession] { - session, err := l.jmapClient.FetchSession(username, l.logger) - if err != nil { - l.logger.Warn().Str("username", username).Err(err).Msgf("failed to create session for '%v'", username) - return c.Set(username, failedSession{err: err}, l.errorTtl) - } else { - l.logger.Debug().Str("username", username).Msgf("successfully created session for '%v'", username) - return c.Set(username, succeededSession{session: session}, 0) // 0 = use the TTL configured on the Cache - } -} - -var _ ttlcache.Loader[string, cachedSession] = &sessionCacheLoader{} - type Groupware struct { mux *chi.Mux logger *log.Logger @@ -127,8 +69,9 @@ func NewGroupware(config *config.Config, logger *log.Logger, mux *chi.Mux) (*Gro defaultEmailLimit := max(config.Mail.DefaultEmailLimit, 0) maxBodyValueBytes := max(config.Mail.MaxBodyValueBytes, 0) responseHeaderTimeout := max(config.Mail.ResponseHeaderTimeout, 0) - sessionCacheTtl := max(config.Mail.SessionCacheTtl, 0) - sessionFailureCacheTtl := max(config.Mail.SessionFailureCacheTtl, 0) + sessionCacheMaxCapacity := uint64(max(config.Mail.SessionCache.MaxCapacity, 0)) + sessionCacheTtl := max(config.Mail.SessionCache.Ttl, 0) + sessionFailureCacheTtl := max(config.Mail.SessionCache.FailureTtl, 0) tr := http.DefaultTransport.(*http.Transport).Clone() tr.ResponseHeaderTimeout = responseHeaderTimeout @@ -157,9 +100,8 @@ func NewGroupware(config *config.Config, logger *log.Logger, mux *chi.Mux) (*Gro } sessionCache = ttlcache.New( - ttlcache.WithTTL[string, cachedSession]( - sessionCacheTtl, - ), + ttlcache.WithCapacity[string, cachedSession](sessionCacheMaxCapacity), + ttlcache.WithTTL[string, cachedSession](sessionCacheTtl), ttlcache.WithDisableTouchOnHit[string, cachedSession](), ttlcache.WithLoader(sessionLoader), ) @@ -206,27 +148,45 @@ func (g Groupware) session(req *http.Request, ctx context.Context, logger *log.L return jmap.Session{}, false, nil } -func (g Groupware) respond(w http.ResponseWriter, r *http.Request, - handler func(r *http.Request, ctx context.Context, logger *log.Logger, session *jmap.Session) (any, string, *ApiError)) { +// using a wrapper class for requests, to group multiple parameters, really to avoid crowding the +// API of handlers but also to make it easier to expand it in the future without having to modify +// the parameter list of every single handler function +type Request struct { + r *http.Request + ctx context.Context + logger *log.Logger + session *jmap.Session +} + +func (g Groupware) respond(w http.ResponseWriter, r *http.Request, handler func(r Request) (any, string, *ApiError)) { ctx := r.Context() logger := g.logger.SubloggerWithRequestID(ctx) session, ok, err := g.session(r, ctx, &logger) if err != nil { logger.Error().Err(err).Interface(logQuery, r.URL.Query()).Msg("failed to determine JMAP session") - w.WriteHeader(http.StatusInternalServerError) + render.Status(r, http.StatusInternalServerError) return } if !ok { // no session = authentication failed logger.Warn().Err(err).Interface(logQuery, r.URL.Query()).Msg("could not authenticate") - w.WriteHeader(http.StatusForbidden) + render.Status(r, http.StatusForbidden) return } logger = session.DecorateLogger(logger) - response, state, apierr := handler(r, ctx, &logger, &session) + req := Request{ + r: r, + ctx: ctx, + logger: &logger, + session: &session, + } + + response, state, apierr := handler(req) if apierr != nil { logger.Warn().Interface("error", apierr).Msgf("API error: %v", apierr) + w.Header().Add("Content-Type", ContentTypeJsonApi) + render.Status(r, apierr.NumStatus) render.Render(w, r, errorResponses(*apierr)) return } @@ -242,8 +202,8 @@ func (g Groupware) respond(w http.ResponseWriter, r *http.Request, } } -func (g Groupware) withSession(w http.ResponseWriter, r *http.Request, - handler func(r *http.Request, ctx context.Context, logger *log.Logger, session *jmap.Session) (any, string, error)) (any, string, error) { +/* +func (g Groupware) withSession(w http.ResponseWriter, r *http.Request, handler func(r Request) (any, string, error)) (any, string, error) { ctx := r.Context() logger := g.logger.SubloggerWithRequestID(ctx) session, ok, err := g.session(r, ctx, &logger) @@ -258,9 +218,17 @@ func (g Groupware) withSession(w http.ResponseWriter, r *http.Request, } logger = session.DecorateLogger(logger) - response, state, err := handler(r, ctx, &logger, &session) + req := Request{ + r: r, + ctx: ctx, + logger: &logger, + session: &session, + } + + response, state, err := handler(req) if err != nil { logger.Error().Err(err).Interface(logQuery, r.URL.Query()).Msg(err.Error()) } return response, state, err } +*/ diff --git a/services/groupware/pkg/groupware/groupware_session.go b/services/groupware/pkg/groupware/groupware_session.go new file mode 100644 index 0000000000..4ee2ecf683 --- /dev/null +++ b/services/groupware/pkg/groupware/groupware_session.go @@ -0,0 +1,66 @@ +package groupware + +import ( + "time" + + "github.com/jellydator/ttlcache/v3" + "github.com/opencloud-eu/opencloud/pkg/jmap" + "github.com/opencloud-eu/opencloud/pkg/log" +) + +type cachedSession interface { + Success() bool + Get() jmap.Session + Error() error +} + +type succeededSession struct { + session jmap.Session +} + +func (s succeededSession) Success() bool { + return true +} +func (s succeededSession) Get() jmap.Session { + return s.session +} +func (s succeededSession) Error() error { + return nil +} + +var _ cachedSession = succeededSession{} + +type failedSession struct { + err error +} + +func (s failedSession) Success() bool { + return false +} +func (s failedSession) Get() jmap.Session { + panic("this should never be called") +} +func (s failedSession) Error() error { + return s.err +} + +var _ cachedSession = failedSession{} + +type sessionCacheLoader struct { + logger *log.Logger + jmapClient jmap.Client + errorTtl time.Duration +} + +func (l *sessionCacheLoader) Load(c *ttlcache.Cache[string, cachedSession], username string) *ttlcache.Item[string, cachedSession] { + session, err := l.jmapClient.FetchSession(username, l.logger) + if err != nil { + l.logger.Warn().Str("username", username).Err(err).Msgf("failed to create session for '%v'", username) + return c.Set(username, failedSession{err: err}, l.errorTtl) + } else { + l.logger.Debug().Str("username", username).Msgf("successfully created session for '%v'", username) + return c.Set(username, succeededSession{session: session}, 0) // 0 = use the TTL configured on the Cache + } +} + +var _ ttlcache.Loader[string, cachedSession] = &sessionCacheLoader{}