Migrate the search index in place instead of a full Tika re-crawl when the shared
search.SchemaRevision is bumped.
bleve (single instance): rebuild from the existing documents into a temp index and
atomically swap, with crash recovery; auto-migrate on startup (opt-out via
SEARCH_ENGINE_BLEVE_AUTO_MIGRATE).
OpenSearch (scaled): versioned indices <base>-v<rev> (no alias), filled from the
newest older index via a create-only scroll reindex - idempotent, never overwrites,
safe to run live. 'opencloud search index migrate' fills the current-revision index;
'opencloud search index prune' drops older ones. Per-document fixups are gated on the
source schema version.
Trash preserved, geopoint siblings synthesized, all fields round-tripped.
- the additive warnings advertise --all-spaces --force-rescan; a plain
walk skips unchanged documents and never backfills the new fields
- Apply checks index existence first again, so a pre-provisioned index
needs no create privilege and odd create-error shapes (string error
bodies, cluster blocks) cannot fail a healthy startup; Create on 404
keeps the typed already-exists swallow as the creation-race backstop
- number_of_replicas drift is not breaking, it is runtime-tunable and
needs no rebuild
- bleve returns the classification alongside post-persist errors and
the server warns before the error check, so the one-time additive
warning is not lost when close or reopen fails
- a golden fixture pins the marshaled bleve mapping so a dependency
bump that changes marshaling fails in CI instead of refusing every
installation in the field
Addresses the two Copilot review comments on the PR: the additive
opensearch log now matches the bleve warning (level and re-index hint),
and the refuse message spells out how to delete the index per engine
(DELETE /<name> vs removing the bleve directory).
Both engines now diff the stored/live index schema against the schema
generated from code when the service starts. A shared recursive
classifier in the mapping package is the single oracle:
- equal: start normally.
- additive (new fields without any indexed data): applied in place.
OpenSearch gets a PUT _mapping with the full code properties, bleve
persists the code mapping into the index (SetInternal + reopen) so
the new fields are properly typed immediately and later startups
classify equal. A startup warning lists the new fields because
documents indexed before the upgrade lack them until re-indexed.
- breaking (changed definitions or analyzers, removed or renamed
fields, or new fields that already contain data of unknown form):
refuse to start with an error describing the rebuild procedure
(delete the index, start, run "opencloud search index --all-spaces")
and the OC_EXCLUDE_RUN_SERVICES=search escape hatch.
PUT _mapping is deliberately only the apply mechanism, never the
judge: its merge semantics cannot see removals or renames and it
accepts in-place updatable param changes with an ack. bleve
additionally checks idx.Fields() so previously dynamically indexed
data (which leaves no schema trace in bleve) is caught, matching by
exact name and by path prefix.
While at it: the OpenSearch startup check runs with a real,
minute-bounded context instead of context.TODO(), bleve indexes are
opened with a 5s bolt_timeout so a second process fails fast instead
of hanging on the file lock, and the reversed errors.Is arguments in
bleve.NewIndex were fixed.
https://github.com/opencloud-eu/opencloud/issues/3092
The Mtime field is mapped as an OpenSearch `date`, which rejects an
empty value with `mapper_parsing_exception: cannot parse empty date`.
The folder and root fixtures had no Mtime, so serializing them to
`"Mtime": ""` made TestEngine_Purge/purge_resource_trees fail when the
document was indexed. Give both a valid RFC3339 Mtime, matching the
file fixture.
Add a TypeGeopoint field type. The libregraph Location facet is kept as an
object (retrieval / numeric queries) and a sibling <name>_geopoint field
carries the {lat,lon} form for geo-distance / bbox / polygon queries,
uniform across bleve and OpenSearch via the shared mapping. PrepareForIndex
splices the sibling in at write time.
Build the bleve and OpenSearch index mappings from the Go struct via
reflection (json tags + per-field overrides) instead of hand-rolled
mappings and hit deserializers. New mapping package: BleveBuildMapping,
OpenSearchBuildMapping, Deserialize[T], PrepareForIndex; field decoding is
fail-soft. Mtime is typed as a date so mtime ranges are chronological on
both backends. Route CS3 facet parsing through mapping.DeserializeStringMap.
The any-valued (bleve hit) and string-valued (CS3 metadata) deserializers
share one generic fillStruct walker with a per-value setLeaf callback.
Per review discussion: don't document accidental behavior. The contract is
simply "percent-encode each path segment, as MS Graph requires; encode ':'
as %3A" - OpenCloud allows ':' in names (OneDrive forbids it), so it's one
more character in the mandatory encode set, not a special case.
The parser is unchanged (split on ":/", decode once). This only rewrites the
docs (code comment, acceptance feature, PR description) to state the encode
contract, and drops the tests that relied on a raw, unencoded ':' in a file
name - keeping the "%3A" test that reflects the actual contract.
Review feedback: split the anchor/path and path/suffix on the structural
delimiter ":/" instead of a bare ":". Since the path and suffix always
start with "/", ":/" is the real delimiter, and a ":" *inside* a file or
directory name (which OpenCloud allows but MS Graph/OneDrive forbid) is
kept as part of the path instead of being mistaken for a separator.
A ":" sitting at a segment boundary (e.g. a name ending in ":") stays
ambiguous and must be percent-encoded as "%3A": the split works on the
literal ":/", so "%3A" is never a delimiter and decodes back to ":". This
is now documented in the code and the acceptance feature.
Tests: colon inside a name (with and without a suffix), the Stat path
carrying the colon, and the "%3A" boundary escape.
The colon-syntax parsing is plain string operations now, but a few
comments still referred to "the previous regex" / "captures". Update them
to match: parseColonPath extracts the itemID from the path (driveID comes
from the route param), and the shape checks stand on their own.
parseColonPath split the anchor asymmetrically (root trimmed the delimiter
colon as part of a literal "/root:" prefix, item cut on it), and the root
branch's comment talked about the driveID which isn't this function's
concern. Split once at the first colon into anchor + rest, then classify
the anchor (exactly "/root", or "/items/{id}" with a single-segment id).
Same behavior, easier to follow.
Also add explicit coverage for the rule that a suffix requires a second
colon: "/root:/Documents/children" (no second colon) is the path
"/Documents/children", not the path "/Documents" with a "/children"
suffix. Two tests pin it end-to-end - it must route to the bare item (not
/items/{id}/children) and Stat must receive the full path.
Review feedback: the two colon-syntax regexes were hard to read. Since the
middleware is scoped to /drives/{driveID}, the RoutePath it inspects is just
the sub-path (/root:/... or /items/{id}:/...), so a regex buys nothing.
Replace rootColonRe/itemColonRe (and the matchInto/extract helpers) with a
single parseColonPath that uses plain string operations
(HasPrefix/TrimPrefix/TrimSuffix/Cut), one commented step at a time.
Behavior is unchanged; the existing table tests (trailing colon, no suffix,
deep paths, multi-segment suffixes, item-anchored, encoded paths) still pass
and pin it.
Addresses review feedback: a sub-router middleware can re-route after all,
as long as it rewrites chi.RouteContext().RoutePath instead of r.URL.Path.
Once chi has descended into a sub-router its routeHTTP matches against
rctx.RoutePath and ignores r.URL.Path, which is why the earlier top-level
registration was thought to be required.
Move ResolveGraphPath off the top-level mux.Use and attach it to the
/drives/{driveID} sub-routers (v1.0 + v1beta1). It now reads driveID from
chi.URLParam and matches against RoutePath (the part below the drive), so
the regexes drop the version + drive prefix entirely.
RoutePath carries the percent-encoded wire form (Graph.ServeHTTP sets
RawPath), so the captured driveID/itemID/path are PathUnescape'd exactly
once - reproducing the decoded r.URL.Path a normal handler would see,
without the previous RawPath/EscapedPath workaround. r.URL.Path is now
left untouched; only chi's internal RoutePath is rewritten.
Tests are reworked to drive requests through a chi router mirroring the
production nesting (including the Graph.ServeHTTP RawPath behavior), so
chi's sub-router middleware ordering, RoutePath encoding and param
round-trip are all covered indirectly: a chi upgrade that changes any of
them fails these tests instead of silently breaking colon-path lookups.
Adds explicit coverage for percent-decoding (%20, %252F) and the `$`/`!`
sub-delimiter id round-trip.
Codacy flagged the colon-path middleware comment claiming both `$`
and `!` need percent-encoding for chi's tree match, while the
implementation only calls r.URL.RawPath = r.URL.EscapedPath() which
does not encode either character per the suggestion's reading.
In practice EscapedPath does encode `!` as `%21` (only `?` is the
hardcoded escape, `!` is escaped because Go's net/url treats it as
needing encoding outside specific contexts). It leaves `$` literal,
which chi handles fine. chi.URLParam returns the encoded segment
verbatim, and downstream OpenCloud handlers (parseIDParam,
GetDriveAndItemIDParam) already PathUnescape before parsing IDs, so
the round-trip works end-to-end. The acceptance tests on this
branch already exercise this with real `$`/`!`-containing IDs.
Add a focused unit test that mounts the middleware behind chi,
sends a colon URL, and asserts the actual contract:
- driveID (only `$`): chi.URLParam returns it literal
- itemID (with `!`): PathUnescape(chi.URLParam(...)) == original
Update the misleading comment so future readers (and reviewers) see
what the encoding actually does and which downstream contract it
relies on.
Cover the rewrite shapes the middleware handles end-to-end against a
real OpenCloud server: root-anchored, item-anchored, deep paths,
trailing colon, and the "/<path>:/<suffix>" sub-route form. Also
assert that NOT_FOUND and PERMISSION_DENIED both collapse to 404.
The /permissions sub-route is registered only at /v1beta1, and the
v1beta1 GetDriveItem handler is share-jail-only, so the v1beta1
mount of the middleware is exercised through the permissions
scenario, since there is no other v1beta1 endpoint that works for
regular personal-drive items.
Three more Copilot review nits on the colon-syntax middleware:
- CS3 Stat returning UNAUTHENTICATED now surfaces as HTTP 401 (was 500
via the default-case fallback). Distinct sentinel + handler branch.
- Item-anchored form (/drives/{driveID}/items/{itemID}:/...) now
validates that driveID's storage/space prefix matches the itemID's,
short-circuiting to 400 InvalidRequest on mismatch instead of doing
a CS3 Stat that would only fail at the handler layer.
- ParseID failures on the anchor id now surface as 400 (was 404). In
practice this branch is defensive — storagespace.ParseID is lenient
and only errors on empty input, which the regex already filters out
— but the right semantic for malformed client input is 400, not 404.
Tests: added two new cases (UNAUTHENTICATED → 401, drive/item id
mismatch → 400). The "malformed drive id" case Copilot suggested
isn't reachable in practice given ParseID's leniency, so it's not
covered.
Two follow-up Copilot review nits on the colon-syntax middleware:
- Don't blank r.URL.RawPath after the rewrite. Graph.ServeHTTP sets
RawPath = EscapedPath() as a workaround for chi's parameter-binding
quirks with `$`/`!` in IDs (see go-chi/chi#641). Clearing RawPath
for rewritten requests negates that workaround. Re-establish the
same invariant after the rewrite.
- Tests previously used log.NewLogger(), which mutates global zerolog
state. Switch to log.NopLogger() — order-independent, no global
side effects.
- Drop double-decoding of URL path components. r.URL.Path is already
decoded by net/http; calling url.PathUnescape again would let crafted
inputs like "%252F" become "/", changing path semantics. Path and
anchor-id strings now go straight to utils.MakeRelativePath /
storagespace.ParseID.
- Distinguish operational errors from "not found". Gateway selection
failure, RPC transport errors, and unexpected CS3 status codes now
surface as 500 (don't mask outages); only NOT_FOUND and PERMISSION_DENIED
collapse to 404 (no existence disclosure). Implemented via a sentinel
errPathNotFound + a 500 fall-through.
- Refactor the two regex-handling branches in rewriteColonPath to share a
resolution path via a small colonMatch struct + matchInto/extract helpers.
Removes the SonarCloud duplication finding without changing behavior.
- Update the docstring on ResolveGraphPath to reflect that the rewrite
preserves the requested API version (/{version}/...) rather than
hard-coding /v1beta1/...
- Test cleanup: register t.Cleanup to remove the per-subtest selector
entry from pool's global selectors map after the subtest, and update
the "unexpected status" case to expect 500 (matches the new error
semantics).
Adds a chi middleware that detects MS Graph colon-syntax URLs and rewrites
them to the canonical /items/{itemID}/... form before chi performs route
matching. Existing handlers, routes, and GetDriveAndItemIDParam stay
unchanged.
Two URL shapes are recognized at both /v1.0 and /v1beta1:
/drives/{driveID}/root:/<path>[:/<suffix>][:]
/drives/{driveID}/items/{itemID}:/<relativePath>[:/<suffix>][:]
Path resolution runs as the request user via CS3 Stat. Both NOT_FOUND and
PERMISSION_DENIED collapse to a 404 response so existence isn't disclosed
to unauthorized callers. URLs without colon syntax fast-path through with
a single substring check. The original URL is stashed in request context
under OriginalPathContextKey for downstream tracing/logging.
The middleware is registered as a top-level mux.Use so it runs before any
route matching: chi middleware on a sub-router runs after the prefix is
matched but cannot redirect to a different leaf route. Top-level
middleware lets URL rewriting actually re-route the request.
Tests cover regex matching across versions, all rewrite variants
(root/items anchored, with/without suffix, with/without trailing colon,
deep paths), NOT_FOUND -> 404, PERMISSION_DENIED -> 404 (security: no
existence disclosure), and original-URL preservation in request context.
#1877 reordered the api-test teardown to delete spaces before users, so the
state where a project space's manager has been deleted is no longer exercised.
This adds explicit coverage for it: the only manager of a project space is
deleted, and a space admin can still list and delete (disable + purge) the space.
Related: #1878
opencloud-eu/reva#655 changes the TUS create response for an invalid name
from 412 to 400. Update the invalid-name scenario to match.
Each example now carries its own expected status in an <http-status-code>
column. Names that fail ValidateName expect 400. The three folder/file rows
stay 412: filename() applies path.Base first, so "folder/file" becomes the
valid leaf "file" and never reaches the changed branch.
lowLevelCreationExtension.feature is unchanged; its missing-Upload-Length
case still returns 412.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add a skill that let's agents bump web to a specified version, commit
the changes and create a PR for it. The user invokes the skill via e.g.
"bump web to v7.0.0".
Dominik Schmidt