Open
Conversation
no implementation at this point
Based on the iiif-presentation-tests
It's only create for now sadly
It's used to return pure IIIF if no show-extra headers were provided. Includes some simple tests to verify.
…be used with Hierarchical endpoints It doesn't attach the show-extra headers, but otherwise behaves the same way a `GetPrivateRequest` would
As the validation of supplied user data is moved to a request handler, we need a way to return 403 from it.
| ], | ||
| Rights = "https://creativecommons.org/licenses/by/4.0/" | ||
| // outside IIIF | ||
| , |
|
|
||
| httpClient = fixture.httpClient; | ||
| // parent = dbContext.Collections | ||
| // .First(x => x.CustomerId == Customer && x.Hierarchy!.Any(h => h.Slug == string.Empty)).Id; |
Collaborator
There was a problem hiding this comment.
commented out code
| private readonly IAmazonS3 amazonS3; | ||
| private readonly HttpClient httpClient; | ||
|
|
||
| public const int Customer = 1; |
Collaborator
There was a problem hiding this comment.
maybe not worth it here, but I think we do this a lot - might be worth creating a constant that works like RootCollection.Id
|
|
||
| // POST into the parent public hierarchical collection | ||
| slug = UniqueSlug(); | ||
| Add(HttpMethod.Post.ToString(),$"{Grandparent}/{Parent}", null, null, slug,null, slug); |
|
|
||
| // POST to flat collections endpoint, use FLAT parent | ||
| var slug = UniqueSlug(); | ||
| Add(HttpMethod.Post.ToString(),Collections,null,Collections+ParentId, slug,null, slug); |
| else | ||
| { | ||
| // 11.2. Flat | ||
| var parsedParentSlugResult = await parentSlugParser.Parse(collection, request.CustomerId, |
Collaborator
There was a problem hiding this comment.
this is the first thing done on both sides of this if statement - can it be moved out - in fact, I think the only difference in this if is the MismatchedParent check
|
|
||
| // finally check the slug against prohibited list | ||
| // source: from PresentationValidator | ||
| if(SpecConstants.ProhibitedSlugs.Contains(resourceSlug)) |
|
|
||
|
|
||
| // 12. Create/update the db collection object | ||
| // source: pre-existing logic in UpsertCollection |
Collaborator
There was a problem hiding this comment.
is this source comment required?
| // If we want just plain IIIF output, we'll clean (i.e. rewrite the standard props) and return as-is. | ||
| if (!request.IsShowExtras) | ||
| { | ||
| // Note: skipping "IsHierarchical" check because it has to be |
Collaborator
There was a problem hiding this comment.
explanation for why it has to be skipped
| // This will load string value of `type` property on the top level of the JSON, if present | ||
| var type = FastJsonPropertyRead.FindAtLevel(rawRequestBody, "type"); | ||
|
|
||
| return type switch |
Collaborator
There was a problem hiding this comment.
can this be shared with above
Member
Quick note that "implements" isn't a linking word, so this isn't linked to the issue. Can use close(s|d), fix(es|ed), resolve(s|d) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements #464
Notes
This PR includes:
As per the source ticket this includes an integration test that performs all allowed flat and hierarchical POST/PUTs for Manifests and Collections, providing the required data in a variety of ways.
To make the test pass, i.e. to implement that desired behaviour, the biggest change is to Collection handling.
Controllers
For hierarchical POST/PUTs the type of the item being upserted is not specified by the path. As both POST and PUT might be creating new resource, we cannot rely on hierarchy-by-slug resolution to check the type. As per the ticket, we have to inspect the
typeproperty.This is implemented via new utility
FastJsonPropertyReadclass,FindAtLevel(string json, string targetPropertyName, int level = 1)method.This uses
Utf8JsonReaderto quickly read through the provided JSON until a selected property (here:type) is found at a specified nesting level (here:1, top level).Collections
Before: In hierarchical paths, only Collection POST implemented. URL path handling as if it was PUT. Only "pure" IIIF collections supported.
Now:
POST /path/to/parentof either "pure" or "presentation" Collection supported.PUT /path/to/resourcealso supported for both types of Collections.Necessary information (slug + parent) can be provided in many ways, which has been expressed in the
CollectionWriteRequestHandler. The handling method has been extensively annotated to explain what is being done and why.The general process is very similar in all scenarios, but at nearly every step small details vary, which makes finding an optimal way of implementing the desired logic very hard. The approach I took avoids code duplication and attempts to allow future changes/maintenance to be done easily via being able to follow the logic step-by-step, aided by the generous comments.
Essentially all logic previously spread between
CollectionController,StorageController,CreateCollection(Handler),UpsertCollection(Handler)andPostHierarhicalCollection(Handler)has been unified, primarily to exemplify the actual desired flow and logic of the concept of "creating or changing a collection", regardless of which of many, many possible ways of doing it was chosen.Show-Extra handling
As the hierarchical endpoints provide "standard", IIIF storage capabilities, i.e. one that does not require knowledge of the extended properties added by
Presentation*entities on top of the base IIIF models, making POST/PUTs does not require providing the custom header.Likewise, it is expected that the response will be plain IIIF, without the extended properties. However, "under the hood", we work with
PresentationCollection, as we will add various properties likeSlugorParent. To allow future-proof way of obtaining the base classV3.CollectionfromPresentationCollection(and same for manifests) there's a new classPresentationIIIFCleanerThis class uses a static constructor to perform one-per-application-lifetime reflection of the relevant models and creates a method that will first create an empty instance of the base model (e.g.
V3.Collectionand then rewrite all properties it can handle from the sourcePresentation*derived class.Because the list of properties to "rewrite" is loaded on each startup, any new properties added to the base model will automatically be included, reliving developers from remembering to and adding manually a property rewrite.
Deletion
As it was at one point useful in testing, I have implemented the hierarchical DELETE as well, which currently just determines type of resource and fires one of already-existing requests for the specified resource. No change to the implementation of those requests was done, just the endpoint.
Manifests
Before: Only flat paths. Assuming
ManifestWriteServiceis "correct", hence leaving as-is.Now:
POST /path/to/parentandPUT/path/to/parent/manifest_slugimplemented viatypeproperty inspection inStorageController.The amount of combinations is rather large with both the method (POST/PUT) and the "style" (flat/hierarchical) affecting some details of the overall process.
This logic has been captured in
DispatchManifestRequestwhich supersedesCreateManifest(flat POST) andUpsertManifest(flat PUT), and also handles hierarchical versions. In the end it determines values necessary for successful handling (parent, slug), setting them on the body if necessary, and also performs checks to catch up mismatch between path-derived and body values.As with the
CollectionWriteRequest, it is richly annotated with comments to explain logic/assumptions/decisions in detail sufficient for both review and future changes.As the validation logic from controllers would have to be cloned (or pulled up class tree too high up), it has been moved to this new
DispatchManifestRequest.