Dynamically generate Allow and Accept-Patch/Put/Post headers #1241
Conversation
There was a problem hiding this comment.
Excellent work!
I think that AllowAcceptMetadataWriter makes several assumptions about how the other classes work. I would leave a TODO to simplify that; I think we should collect more metadata going forward (see below).
The only thing that put my off is RedirectHttpError being treated as a special case; why can't we have consistent instanceof behavior?
If an error gets thrown the identifier of the target resource does not reach the metadata writer so we can't make
I wonder if the target resource should be part of the Error constructor. While it makes sense (it's always an Error for a certain resource), it probably is not feasible.
In case of errors we also don't know if the target resource exists with similar issues as above.
Maybe in general we should be storing much more in metadata of what we "learn" during processing.
| object: NamedNode | BlankNode | Literal | string | null = null, | ||
| graph: MetadataGraph | null = null, | ||
| ): boolean { | ||
| return this.store.countQuads(this.id, predicate, object, graph) > 0; |
There was a problem hiding this comment.
Yeah let's make an N3.js issue for that.
There was a problem hiding this comment.
N3.js has a has function actually because it implements the DatasetCore interface. But that one requires an exact triple without wildcards.
There was a problem hiding this comment.
There was a problem hiding this comment.
It's based on the interface defined here: https://rdf.js.org/dataset-spec/#datasetcore-interface . Quad is defined as being a NamedNode/BlankNode/Variable/Quad http://rdf.js.org/data-model-spec/#quad-interface. So only way to have wildcards there would be to use Variables I guess?
There was a problem hiding this comment.
You could give secret nulls, but yeah 😅
I'm currently working on that PR to have that functionality (fine for now).
Forgot to mention this in my main comment, but one issue I had after this is that error metadata is in this weird zone where we now have a |
📁 Related issues
Closes #577, #572
✍️ Description
Minor issues still remaining:
Allow: POST). Would require a small change in some interfaces to also pass that along should there be a need for that in the future.Accept-PutandAccept-Postheaders will still say*/*even though we only accept RDF there. Could be fixed by setting the values of those in the config that defines theDataAccessorthat is used, but this might complicate it a bit for people wanting to create configs for new data accessors, or that want to disable those headers for some reason so I left it on*/*for everything for now. But is fixable if really needed.This PR also introduces a change to how error classes are generated to make sure they all have the same static functions which makes it easier to add new static functions or new error classes. We could do
export const NotFoundHttpError = generateHttpErrorClass(404, 'NotFoundHttpError');, but that way Components.js is unable to deduce the constructor parameters, so each class still has to define its constructor to get around that.