Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure s.json thumb sizes are emitted in sizes from named query manifests and single-asset manifests #631

Closed
tomcrane opened this issue Sep 20, 2023 · 5 comments · Fixed by #810
Assignees
Labels
C-W delivery-channels jira marks issue for crossposting to JIRA

Comments

@tomcrane
Copy link
Contributor

tomcrane commented Sep 20, 2023

...so that clients can see what sized thumbs were created in bulk, for a named query, as well as individually.

Clients including the DDS.

At the moment protagonist deduces these sizes, so is potentially prone to rounding errors.
It won't be doing any deducing in future, just reading the sizes that cantaloupe made.

Requirement outlined above resulted in RFC being written (#767)

Update after RFC

RFC documents how metadata that can be read in bulk will be stored in AssetApplicationMetadata.

This ticket is blocked by #787 - once that is done we will have the AssetApplicationMetadata table available. This ticket is then to update classes involved in NQ + single-asset manifest generation (likely IIIFManifestBuilder and/or IIIFCanvasFactory) to read the thumbnail sizes generated, rather than deducing them.

However, when first introduced, and possibly for some time after, this new table won't be populated with "ThumbSizes" metadata value for every asset. So until that happens we need a method of attempting to read the actual values but falling back to calculating if there is no "ThumbSizes" metadata.

Initially upon release all new-format (ie using IIIF Size Param) thumb sizes will be based on confined sizes (!w,h) so we will be able to calculate them using existing methods in AssetX (updating method to only calculate confined thumb sizes).

Implementation points:

  • We don't want to .Include() in the base NQ IQueryable generator - need a hook to include relevant data only depending on projection.
  • It doesn't feel correct for Orchestrator to load assets with PolicyData. It will load ImageDeliveryChannel with PolicyId - this can be used to load + cache PolicyData if required (use same local caching mechanism as is currently happening with ThumbsPolicy).
  • As well as "sizes the sizes read here need to populate the "thumbnail" property on manifest

Acceptance Criteria

  • Named query and single-item manifests output sizes property read from database
  • Where no thumbs sizes found in database calculate from image dimensions + policyData
  • Logic to fallback to calculating should be controllable by a config flag - this should be a date to control fallback based on last image ingest date.
@tomcrane tomcrane added the C-W label Sep 20, 2023
@donaldgray
Copy link
Member

IIIFManifestBuilder is central class used for generating both NQ + single-asset manifests.

@tomcrane
Copy link
Contributor Author

GetThumbnailSizesForImage should read S3 sizes.json.

@donaldgray
Copy link
Member

private async Task<ImageSizeDetails> GetThumbnailSizesForImage(Asset image)
is the method call to work out sizes per canvas (ie image).

We will need to take performance implications into account, particularly for larger NQs

@tomcrane
Copy link
Contributor Author

Also need to make sure that "secret" thumbnails are also present in the sizes list.

@JackLewis-digirati
Copy link
Contributor

This ticket will also need to cover handling skipped tests in NamedQueryTests and additional tests in ManifestTests to cover the retrieval of thumbnail sizes from the database (as opposed to s.json that it uses before this ticket)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-W delivery-channels jira marks issue for crossposting to JIRA
Projects
Status: Done
4 participants