Page Routes With Page and Line Class Refactor

[thehabes]

Closes #459

Summary

Refactors page and line routes to use the Page and Line class methods instead of inline logic, and hardens RERUM interactions with proper error handling and hydration tracking.

Page Class (classes/Page/Page.js)

• New #hydrated flag — Tracks whether the Page instance has been synced with RERUM, preventing redundant network calls on consecutive asJSON() invocations.
• New #itemsResolved flag — Prevents #loadAnnotationPageDataFromRerum() from overwriting already-resolved annotation items with raw RERUM references.
• New #loadAnnotationPageDataFromRerum() — Fetches and syncs all AnnotationPage properties from RERUM (target, items, creator, label, partOf, prev, next). Replaces inline RERUM fetching in route handlers.
• New #loadAnnotationPageItemsFromRerum() — Resolves all annotation references in items by instantiating Line objects and calling asJSON(true). Replaces the standalone resolveReferences() utility.
• New resolvePageItems() — Public method exposing #loadAnnotationPageItemsFromRerum() for the /resolved endpoint.
• New asJSON(isLD) — Returns the Page as a W3C AnnotationPage (JSON-LD with @context/type when isLD=true, plain object otherwise). Hydrates from RERUM on first call if needed.
• RERUM items are now minimal references — #savePageToRerum() strips items to {id, type} only when writing to RERUM. The TPEN database (MongoDB project record) continues to store items with target intact.
• Improved error handling in #savePageToRerum() — RERUM fetch failures now produce structured errors with status: 502 and descriptive messages instead of generic throws. Version conflicts (409) produce proper error objects with code: 'VERSION_CONFLICT'.
• @context updated — Changed from http://www.w3.org/ns/anno.jsonld to http://iiif.io/api/presentation/3/context.json for IIIF Presentation API v3 conformance.

Line Class (classes/Line/Line.js)

• New #hydrated flag — Replaces the body === undefined check in asJSON() and asTextBlob() with a proper hydration guard. Once hydrated (via load or save), subsequent calls skip RERUM fetches.
• Improved error handling in #saveLineToRerum() — RERUM responses are now parsed with status-aware error messages (502 for RERUM failures). Non-existent lines (no id/@id in response) trigger a create fallback instead of silent failure.
• Improved error handling in #loadAnnotationDataFromRerum() — Added .catch() for network errors and garbled-data detection, consistent with the Page class pattern.
• asJSON(isLD) includes creator — When isLD=true, the creator is included in the JSON-LD output if present.

Page Routes (page/index.js)

• GET /:pageId — Replaced inline AnnotationPage construction and conditional RERUM fetching with page.asJSON(true). The Page class handles hydration internally.
• PUT /:pageId — Response now uses page.asJSON(true) instead of returning the raw Page instance.
• GET /:pageId/resolved — Replaced resolveReferences() utility call with pageData.resolvePageItems() then pageData.asJSON(true).
• Removed resolveReferences import — No longer needed; resolution logic lives in the Page class.
• Removed redundant null checks — findPageById() now throws on not-found, so post-call null checks are unnecessary.

Line Routes (line/index.js)

• Explicit status codes — All responses now use explicit .status(200) or .status(201) instead of relying on Express defaults.
• Variable rename — jsonObj → lineJson for clarity.
• Removed redundant null check — POST handler no longer checks if (!page) return after findPageById() since it throws on not-found.

Utilities (utilities/shared.js)

• findPageById() simplified — Removed the rerum parameter and direct RERUM fetching. Always looks up the page in the project's layer data and returns a Page class instance. RERUM hydration is now the Page class's responsibility via asJSON().
• Removed resolveReference() and resolveReferences() — Annotation resolution is now handled by Page.#loadAnnotationPageItemsFromRerum() using the Line class.
• updatePageAndProject() error passthrough — RERUM errors (status: 502) are now re-thrown instead of being swallowed into a generic 500.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

[cubap]

What a big lift! I have lots of remarks. Some preferences, some stronger opinions.
I did not directly test functionality because I could see that was generally intact. My comments are all code-sniff related.

[cubap]

This block never sets the default at the top, right? It is only 64 or 66.

[thehabes]

What we are saying here is that

let rerumErrorMessage = resp.status code from RERUM or a default 500 since it could not be obtained: The URI of the thing that errored - {anticipate resp.text() for error text}. Here we are presuming this is going to be an understandable error from RERUM but we won't really know until we try to resp.text()

try {
To do resp.text(), which checks if it is a real error response from rerum. If it fails make up the text.
}
catch (err) {
Could not even attempt resp.text(), so this is really bad. The connection may not have occurred at all. The rerumErrorMessage is undefined, we need to start from scratch on an error. We may not even have err.status.
}

const err = that good rerumErrorMessage we built but if it is undefined ?? we need to be real generic and say "500 from RERUM" because we cannot gleam a status code or error message at all..

The error status to send forward to the client is 502, with the descriptive or generic RERUM error message code: uri - text.

[cubap]

That's how I read it.

[cubap]

Then down here the message is either used or the default that was just cleared is reinstated with more text at the end.

[thehabes]

Yes, because we cannot trust what we already had. We did not get a good error response from RERUM - perhaps no status code, definitely no error text. This happens on like "Connection Refused" (firewall), for example. So we need to recreate the text as something very generic-y. Here's your 502, and know that generically "500: https://devstore.rerum.io/v1/12345 - RERUM didn't work" even though I didn't get far enough to actually get that 500 from RERUM so I'm making it up (if resp.text() failed, you do not have a formatted RERUM error something else happened. You might have resp.status, you might not).

[cubap]

This is accommodating that "Create" is no longer in the tinyDriver and that it is called 'save'. I think this should just be changed to toggle save/update and avoid this weird trick.

[thehabes]

Not quite. Consider how Page.js does this

if (this.#tinyAction === 'create') {
    const saved = await databaseTiny.save(pageAsAnnotationPage)
        .catch(err => {
            console.error(err, pageAsAnnotationPage)
            throw new Error(`Failed to save Page to RERUM: ${err.message}`)
        })
    this.#tinyAction = 'update'
    this.#hydrated = true
    return this
}

It calls databaseTiny.save manually.

Now consider how Line.js was doing it

const newURI = await databaseTiny[action](updatedLine).then(res => res.id)
.catch(err => {
    throw new Error(`Failed to update Line in RERUM: ${err.message}`)
})

When databaseTiny[action] has "create" for action, databaseTiny.create is undefined and throws an error, because it is databaseTiny.save not databaseTiny.create. In this line, it determines if it is trying to "create" or "update" and I didn't want to refactor around it, I just made sure it called the right databaseTiny function when it means to.

Also note that databaseTiny error throughput !== RERUM error throughput. I did not adjust the databaseTiny error throughput in this PR.

[cubap]

This illustrates my point. The action is save/update and the #tinyAction is create/update. The whole existence of "create" seems extra. Those could both be simplified easily.

[cubap]

Is this whole catch block repeated nearly exactly? It all looks very familiar and it might be something worth extracting, since we want to have it be consistent.

[thehabes]

Yes, it is repeated nearly exactly multiple times. The files already each had independent handling but it was irregular. Now that it is regular, you are noticing it.

Here is the regularization, which is not extracted as a utility or helper pattern. It was all instantiated in place of where the error were already being processed individually.

let data = await fetch(rerumURI).then(async (resp) => {
    if (resp.ok) return resp.json()
    Process RERUM error resp.text() and build rerumErrorMessage (but catch when resp.text() fails)
    throw TPEN Services 502 with message body "rerumErrorStatusCode: uri - rerumErrorMessage", make up "rerumErrorMessage" if you have to because resp.text() failed.  
})
.catch(err => {
    Was it that 502?  Ok, throw the err forward we are done
    Wasn't a 502?  Oh boy the connection to RERUM did not occur at all.  Gotta be generic
    Build that generic RERUM error message.
    throw TPEN Services 502 with message body "500: uri - genericErrorMessage"
})
if (!(data.id || data["@id"])) {
    Well we got a 200 or 201 from RERUM the but the item in the body does not have an id or @id.
    Build that generic RERUM error message.
   throw TPEN Services 502 with message body "500: uri - genericErrorMessage"
}

We can extract this and make it prettier, but this is what makes the error throughput work from RERUM through a Class and out as a TPEN Services response. I can show you with a demo, but this was those days I was getting in there in the back end and messing with the URIs to make them fail these ways to make sure we caught all the kinds of errors that could occur when attempting RERUM connections and communications.

To do much more than this is probably going down the #425 road

[cubap]

This doesn't have to be in scope. I am just nervous about creating another situation we might update what seems to be off and miss the corners we haven't seen broken yet.

[cubap]

This is already a promise we can .catch() which might be a lot easier to read.

[thehabes]

Lines.asJSON(true) does not return a promise. It returns an object.

#loadAnnotationDataFromRerum() either throws an error (at the top of Line.asJSON()), or asJSON() proceeds forward with a JSON object to give back.

So it needs the try {} catch(){} pattern.

[cubap]

What are we awaiting?

[cubap]

I think if you reordered this logic 2-1-3, it could be a lot less if/elseif/else-y

[thehabes]

eh probably. we can change it, but then we have to test it and verify the change. could be worth it.

[cubap]

What's the reason for this? The handler seems useful...

[thehabes]

You must hand forward the Express response stream res to handleVersionConflict. This line of code errored out trying to use null. We also do not have the Express response stream here, so we were caught in a pickle. This now replicates what handleVersionConflict() would do if we were able to give it the Express Response stream.

[cubap]

Just throwing here (with a 409, which it has already) would land here:

TPEN-services/page/index.js

Line 240 in 0a5e152

return handleVersionConflict(res, error)

and that doesn't need much data to reproduce this error:

TPEN-services/utilities/shared.js

Lines 327 to 338 in 23711c9

	export const handleVersionConflict = (res, error) => {
	return res.status(409).json({
	error: error.message,
	currentVersion: error.currentVersion,
	code: 'VERSION_CONFLICT',
	details: 'The document was modified by another process.',
	// Include additional context if available
	...(error.pageId && { pageId: error.pageId }),
	...(error.layerId && { layerId: error.layerId }),
	...(error.lineId && { lineId: error.lineId })
	})
	}

so if the error back from Tiny (which I am not certain we have right) has err.currentVersion then we're golden.

https://github.com/CenterForDigitalHumanities/TinyPen/blob/ed4535eada10325d007f0d5a6deacbde82029b55/routes/overwrite.js#L46-L50

You just modify the err.message if you want and rethrow it. It is correct that you cannot go directly into handleVersionConflict() with it.