26 q2#631
Conversation
- create and use a map in XQuery - store a detailed string in HTML meta - create and use a Map in JS Cf. #580
- in api.xq, add header Accept to a parameter in the request map Cf. #579
…search - adjust search.xqm to use IDs for forms (→ eventListener) - search.xsl must support XML as returned from new API - search.js handles onSubmit of FTS search form Cf. #579
- use consistent wording - always use `result` when returning lists - streamline the definition and use
Cf. #579 Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <copilot@github.com>
Used for paging project-based search and for creating KWIC for a file. This way, KWIC does not require a new search if the session is still active. Cf. #579 Co-authored-by: Copilot <copilot@github.com>
Results are now grouped by their computed path (below tei:text). This means we can add info about structural elements to the HTML view. This can also be used to filter the results. This might be more efficient than creating fields beforehand. Ideally, we can drop most of the specialised idexes. Cf. #579 #566
In general, only tei:text is indexed, with head getting an additional index, and note handled separately Cf. #566
Co-authored-by: Copilot <copilot@github.com>
Includes improved creation of JSON output and additional error handling and documentation Co-authored-by: Copilot <copilot@github.com>
include changes to wdbDocument.loadContent this is now smarter and accepts an optional selector indicating what to extract from the response Cf. #579
as we modify the request to add a header, auth needs to be set explicitly or will not be executed, meanin all operations requiring auth will fail
Projects created inherit owner and rights from parent; by default, this should be world readable
The path in found in $model?filePathInfo?mainProject and this does not need repeating. Also, the ID of the project is what’s actually expected here
removed stale code and added an ID in HTML
It erroneously assumed `resources` to reside in the project collection even for sub-projects. - All calls to applySpecificXsl had to be changed to use the pathInfo map; this included calls to r2:returnResponse which itself calls wdb:applySpecificXls - `start.xqm` had to be adjusted as it expects an element but now gets the response map
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 176411c30b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| :) | ||
| declare function r2p:optionsResource ( $request as map(*) ) as item() { | ||
| let $project := try { wdbFiles:getFullPath($request?parameters?ed) } catch * { $err:code } | ||
| , $t := util:log("info", "OPTIONS request for project with ID " || $request?parameters?ed || ". Project found: " || ($resource instance of map(*))) |
There was a problem hiding this comment.
The new logging expression references $resource, which is never declared in this function scope. In XQuery, an undeclared variable is a static error, so this can prevent projects.xqm from compiling/loading at runtime and break the whole REST2 projects module rather than just OPTIONS handling.
Useful? React with 👍 / 👎.
| </parameters> | ||
|
|
||
| return try { | ||
| let $content := wdbProc:process($input, $model?process//meta:step, $params) |
There was a problem hiding this comment.
getContent now calls wdbProc:process with $model?process//meta:step, but existing metadata definitions in this repo use <meta:command> directly (no <meta:step> wrapper), and wdbProc:process requires at least one step. That means the call receives an empty sequence and falls into the catch path, returning 500 for normal view rendering requests that previously worked.
Useful? React with 👍 / 👎.
| let $type := if ( exists($request?header?accept) ) | ||
| then $request?header?Accept |
There was a problem hiding this comment.
This branch checks $request?header (singular), but the request map uses headers (e.g., set in api:addHeader and used elsewhere in REST2). As a result, Accept is never read here and the code always falls back to text/html, causing incorrect content negotiation for /resources/{id}/views/{view}.
Useful? React with 👍 / 👎.
|
@codex check whether the recent changes address the problems and report other findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bcb658d277
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| else if ( $request?headers?Accept != ('application/xml', 'application/json') ) | ||
| then r2:response(406, 'text/plain', 'Listings are available as application/xml or application/json', $r2:allOrigins) |
There was a problem hiding this comment.
r2p:listProjectResources now uses != against a two-item sequence, which in XQuery is an existential comparison; for Accept: application/xml (or application/json) the expression still evaluates true because one pair is unequal. That makes this endpoint return 406 even for supported media types, so normal resource listings fail unless the header is absent.
Useful? React with 👍 / 👎.
| (: else if ( $meta//meta:file[@path = $request?body?path || '/' || $request?body?file?name | ||
| and @xml:id = $request?parameters?id | ||
| and @uuid = $uuid | ||
| ] ) then | ||
| r2:response(204, 'text/plain', '', $r2:allOrigins) | ||
| r2:response(204, 'text/plain', '', $r2:allOrigins) :) |
There was a problem hiding this comment.
The previous fast-path for “same path + same id + same hash” was commented out, so a retry of an identical PUT /projects/{ed}/resources/{id} now falls through to the UUID conflict check and returns 409. This breaks idempotent PUT semantics and causes legitimate client retries (e.g., after transient network failures) to fail as conflicts.
Useful? React with 👍 / 👎.
| <set-header name="Access-Control-Allow-Methods" value="{ request:get-method() }" />, | ||
| <set-header name="Access-Control-Allow-Headers" value="authorization" />, |
There was a problem hiding this comment.
The new API-v2 OPTIONS workaround sets Access-Control-Allow-Methods to request:get-method() (i.e. OPTIONS) and Access-Control-Allow-Headers to only authorization. For browser preflight on POST/PUT (and common content-type headers), this response does not authorize the requested method/headers, so cross-origin calls are blocked before reaching the API.
Useful? React with 👍 / 👎.
against live system at ULB-DA
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f3bb637c69
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| declare %private function r2r:allViews ( $resource as map(*) ) as element()* { | ||
| let $metaFile := $resource?meta | ||
| , $parent := $metaFile/meta:projectMD/meta:struct/meta:import[1] | ||
| , $parentMeta := doc($resource?projectPath || '/' || $parent/@path) |
There was a problem hiding this comment.
Load of $parentMeta is unconditional here, so projects that have no <meta:import> still evaluate doc($resource?projectPath || '/' || $parent/@path). When $parent is empty this resolves to an invalid document path and can raise a runtime error, causing resource view listing/selection to fail even though local meta:process entries exist. Move the doc() call inside the if (exists($parent)) branch.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The function has change in the last two commits: it is supposed to traverse upwards until it reaches the top-level wdbmeta.xml.
Background: a project is not required to import a parent; in large collections, this would clutter the navigation entirely.
Closes #628
As the API definition always starts with `/api/v2/`, Swagger’s base URL must not contain it. To keep things uniform, we always add `api/V2` in rest-common. Cf. #631 (comment)
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dfffb1283e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ( | ||
| ( $local:isget and $exist:resource = ('v2.json', 'apiv2.html') ) | ||
| ) then | ||
| if ( $local:isget and $exist:resource = ('v2.json', 'apiv2.html') ) then |
There was a problem hiding this comment.
This route guard still whitelists apiv2.html, but the commit renamed the documentation page to v2.html. Requests for /rest2/v2.html now miss this static forward and fall through to the generic HTML branch, which routes through the app view pipeline instead of serving the docs file directly, so the API documentation page becomes inaccessible.
Useful? React with 👍 / 👎.
| if ( $info?body?xml//tei:titleStmt/tei:title[@type eq 'num'] ) | ||
| then attribute order { normalize-space($info?body?xml//tei:titleStmt/tei:title[@type eq 'num']) } | ||
| then attribute order { normalize-space($info?numberingTitle) } |
There was a problem hiding this comment.
enterMetaForXml now receives precomputed metadata in $info (numberingTitle, mainTitle), but this condition still reads $info?body?xml. Since $info?body is no longer populated, the condition is always false and the generated <view> never gets an order attribute, which drops ordering metadata for numbered titles during resource creation/update.
Useful? React with 👍 / 👎.
| $resource?error | ||
| else | ||
| let $upload := r2r:parseUpload($request) | ||
| , $t := util:log("warn", "Error storing file under " || $upload?relativePath || ": A file with ID " || $request?parameters?id || " is present in a different location: " || $resource?entry/@path) |
There was a problem hiding this comment.
This warning is evaluated for every PUT request before any conflict checks run, so even successful updates emit an “Error storing file…” warning. That creates persistent false-positive error noise in logs and makes actual storage conflicts harder to distinguish in operations.
Useful? React with 👍 / 👎.
1 participant
No description provided.