Skip to content

Commit

Permalink
BXC-4802 updating tests and code for zero byte JP2s (#1864)
Browse files Browse the repository at this point in the history
* BXC-4802 updating tests and code for zero byte JP2s

* BXC-4802 fix test

* BXC-4802 adjusting manifest logic for differences in listViewableFiles for canvases and manifests

* BXC-4802 update exception message

* BXC-4802 trying to fix tests

* BXC-4802 DRY up method

* BXC-4802 fix tests and remove unneeded method

* BXC-4802 fix tests

* BXC-4802 fix last test

* BXC-4802 add test for manifest service

* BXC-4802 add new test for canvas for non viewable files

* BXC-4802 accounting for filsize of 0

* BXC-4802 remove extent check

* BXC-4802 account for no filesize in jp2 datastreams

* BXC-4802 update invalid jp2 logic

* BXC-4802 update corpus and solr test
  • Loading branch information
sharonluong authored Jan 21, 2025
1 parent 1195ee5 commit 5067b42
Show file tree
Hide file tree
Showing 9 changed files with 263 additions and 59 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ public void deleteTree() throws Exception {
server.commit();

SolrDocumentList docListAfter = getDocumentList();
assertEquals(2, docListAfter.getNumFound());
assertEquals(4, docListAfter.getNumFound());

assertObjectsNotExist(corpus.pid2, corpus.pid4, corpus.pid6, corpus.pid5);
}
Expand All @@ -108,7 +108,7 @@ public void deleteNonexistent() throws Exception {

SolrDocumentList docListAfter = getDocumentList();

assertEquals(7, docListAfter.getNumFound());
assertEquals(9, docListAfter.getNumFound());
}

@Test
Expand All @@ -119,7 +119,7 @@ public void deleteSimple() throws Exception {
server.commit();

SolrDocumentList docListAfter = getDocumentList();
assertEquals(5, docListAfter.getNumFound(), "One object should have been removed");
assertEquals(7, docListAfter.getNumFound(), "One object should have been removed");

assertObjectsNotExist(corpus.pid6);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
* > obj4
* > obj5
* > obj6
* > obj7
* > obj3
*
* @author bbpennel
Expand All @@ -40,7 +41,9 @@ public class TestCorpus {
public PID pid4;
public PID pid5;
public PID pid6;
public PID pid7;
public PID pid6File;
public PID pid7File;
public PID nonExistentPid;

public TestCorpus() {
Expand All @@ -50,7 +53,9 @@ public TestCorpus() {
pid4 = makePid();
pid5 = makePid();
pid6 = makePid();
pid7 = makePid();
pid6File = makePid();
pid7File = makePid();
nonExistentPid = makePid();
}

Expand Down Expand Up @@ -79,7 +84,7 @@ public List<SolrInputDocument> populate() {
newDoc.addField("ancestorPath", makeAncestorPath(pid1));
newDoc.addField("resourceType", "Collection");
List<String> collectionDatastream = List.of(
DatastreamType.JP2_ACCESS_COPY.getId() + "|image/jp2|bunny.jp2|jp2|||" + pid2.getId() + "|1200x1200");
DatastreamType.JP2_ACCESS_COPY.getId() + "|image/jp2|bunny.jp2|jp2|766||" + pid2.getId() + "|1200x1200");
newDoc.addField(SearchFieldKey.DATASTREAM.getSolrField(), collectionDatastream);
docs.add(newDoc);

Expand Down Expand Up @@ -119,7 +124,7 @@ public List<SolrInputDocument> populate() {
newDoc.addField("resourceType", ResourceType.File.name());
List<String> imgDatastreams = Arrays.asList(
ORIGINAL_FILE.getId() + "|image/png|file.png|png|766|urn:sha1:checksum||1200x1200",
DatastreamType.JP2_ACCESS_COPY.getId() + "|image/jp2|bunny.jp2|jp2|||" + pid6File.getId() + "|1200x1200");
DatastreamType.JP2_ACCESS_COPY.getId() + "|image/jp2|bunny.jp2|jp2|766||" + pid6File.getId() + "|1200x1200");
newDoc.addField(SearchFieldKey.DATASTREAM.getSolrField(), imgDatastreams);
newDoc.addField(SearchFieldKey.FILE_FORMAT_CATEGORY.getSolrField(), ContentCategory.image.getDisplayName());
newDoc.addField(SearchFieldKey.FILE_FORMAT_TYPE.getSolrField(), "png");
Expand All @@ -137,12 +142,48 @@ public List<SolrInputDocument> populate() {
newDoc.addField("resourceType", ResourceType.Work.name());
List<String> workDatastreams = Arrays.asList(
ORIGINAL_FILE.getId() + "|image/png|file.png|png|766|urn:sha1:checksum|" + pid6File.getId() + "|1200x1200",
DatastreamType.JP2_ACCESS_COPY.getId() + "|image/jp2|bunny.jp2|jp2|||" + pid6File.getId() + "|1200x1200");
DatastreamType.JP2_ACCESS_COPY.getId() + "|image/jp2|bunny.jp2|jp2|766||" + pid6File.getId() + "|1200x1200");
newDoc.addField(SearchFieldKey.DATASTREAM.getSolrField(), workDatastreams);
newDoc.addField(SearchFieldKey.FILE_FORMAT_CATEGORY.getSolrField(), ContentCategory.image.getDisplayName());
newDoc.addField(SearchFieldKey.FILE_FORMAT_TYPE.getSolrField(), "png");
docs.add(newDoc);

newDoc = new SolrInputDocument();
newDoc.addField("title", "Work2");
newDoc.addField("id", pid7.getId());
newDoc.addField("rollup", pid7.getId());
newDoc.addField("roleGroup", "public admin");
newDoc.addField("readGroup", "public");
newDoc.addField("adminGroup", "admin");
newDoc.addField("ancestorIds", makeAncestorIds(pid1, pid3));
newDoc.addField("ancestorPath", makeAncestorPath(pid1, pid3));
newDoc.addField("resourceType", ResourceType.Work.name());
List<String> work2Datastreams = Arrays.asList(
ORIGINAL_FILE.getId() + "|image/png|file.png|png|766|urn:sha1:checksum|" + pid6File.getId() + "|1200x1200",
DatastreamType.JP2_ACCESS_COPY.getId() + "|image/jp2|bunny.jp2|jp2|766||" + pid6File.getId() + "|1200x1200");
newDoc.addField(SearchFieldKey.DATASTREAM.getSolrField(), work2Datastreams);
newDoc.addField(SearchFieldKey.FILE_FORMAT_CATEGORY.getSolrField(), ContentCategory.image.getDisplayName());
newDoc.addField(SearchFieldKey.FILE_FORMAT_TYPE.getSolrField(), "png");
docs.add(newDoc);

newDoc = new SolrInputDocument();
newDoc.addField("title", "File2");
newDoc.addField("id", pid7File.getId());
newDoc.addField("rollup", pid7.getId());
newDoc.addField("roleGroup", "public admin");
newDoc.addField("readGroup", "public");
newDoc.addField("adminGroup", "admin");
newDoc.addField("ancestorIds", makeAncestorIds(pid1, pid3, pid7));
newDoc.addField("ancestorPath", makeAncestorPath(pid1, pid3, pid7));
newDoc.addField("resourceType", ResourceType.File.name());
List<String> fileDatastreams = Arrays.asList(
ORIGINAL_FILE.getId() + "|image/png|file.png|png|766|urn:sha1:checksum||120x120",
DatastreamType.JP2_ACCESS_COPY.getId() + "|image/jp2|bunny.jp2|jp2|766||" + pid7File.getId() + "|120x120");
newDoc.addField(SearchFieldKey.DATASTREAM.getSolrField(), fileDatastreams);
newDoc.addField(SearchFieldKey.FILE_FORMAT_CATEGORY.getSolrField(), ContentCategory.image.getDisplayName());
newDoc.addField(SearchFieldKey.FILE_FORMAT_TYPE.getSolrField(), "png");
docs.add(newDoc);

newDoc = new SolrInputDocument();
newDoc.addField("title", "Second collection");
newDoc.addField("id", pid3.getId());
Expand All @@ -154,7 +195,7 @@ public List<SolrInputDocument> populate() {
newDoc.addField("ancestorPath", makeAncestorPath(pid1));
newDoc.addField("resourceType", "Collection");
List<String> collection2Datastream = List.of(
DatastreamType.JP2_ACCESS_COPY.getId() + "|image/jp2|bunny.jp2|jp2|||" + pid2.getId() + "|120x120");
DatastreamType.JP2_ACCESS_COPY.getId() + "|image/jp2|bunny.jp2|jp2|0||" + pid2.getId() + "|1200x1200");
newDoc.addField(SearchFieldKey.DATASTREAM.getSolrField(), collection2Datastream);
docs.add(newDoc);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,6 @@ public class DownloadImageService {
*/
public ResponseEntity<InputStreamResource> streamImage(ContentObjectRecord contentObjectRecord, String size, boolean attachment)
throws IOException {
if (contentObjectRecord.getDatastreamObject(DatastreamType.JP2_ACCESS_COPY.getId()) == null) {
return ResponseEntity.notFound().build();
}

var contentDispositionHeader = "inline;";
if (attachment) {
String filename = getDownloadFilename(contentObjectRecord, size);
Expand Down Expand Up @@ -82,6 +78,9 @@ public String getSize(ContentObjectRecord contentObjectRecord, String size) {
if (!Objects.equals(size, FULL_SIZE)) {
int integerSize = parseSize(size);
var longerSide = getLongestSide(contentObjectRecord);
if (longerSide == 0) {
return null;
}
// request is bigger than or equal to full size, so we will switch to full size
if (integerSize >= longerSide) {
return FULL_SIZE;
Expand Down Expand Up @@ -144,7 +143,8 @@ private String getExtent(ContentObjectRecord contentObjectRecord) {
}

/**
* If the shortest side of the content object is smaller than the size requested, return the placeholder
* If the shortest side of the content object is smaller than the size requested or if there is no
* valid JP2, then return the placeholder
* @param contentObjectRecord
* @param size
* @return true if service needs to return a placeholder image
Expand All @@ -153,6 +153,9 @@ private boolean needsPlaceholder(ContentObjectRecord contentObjectRecord, String
if (Objects.equals(FULL_SIZE, size)) {
return false;
}
if (isInvalidJP2Datastream(contentObjectRecord)) {
return true;
}
var shortestSide = getShortestSide(contentObjectRecord);
return shortestSide < Integer.parseInt(size);
}
Expand Down Expand Up @@ -190,6 +193,11 @@ private String getPlaceholderUrl(String size) {
return iiifBasePath + PLACEHOLDER_ID + "/full/" + formattedSize + "/0/default.png";
}

public boolean isInvalidJP2Datastream(ContentObjectRecord contentObjectRecord) {
var datastream = contentObjectRecord.getDatastreamObject(DatastreamType.JP2_ACCESS_COPY.getId());
return datastream == null || datastream.getFilesize() == 0;
}

public void setIiifBasePath(String iiifBasePath) {
this.iiifBasePath = iiifBasePath;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,13 @@ public class IiifV3ManifestService {
*/
public Manifest buildManifest(PID pid, AgentPrincipals agent) {
assertHasAccess(pid, agent);
var contentObjs = listViewableFiles(pid, agent.getPrincipals());
if (contentObjs.isEmpty()) {
ContentObjectRecord rootObj = solrSearchService.getObjectById(new SimpleIdRequest(pid, agent.getPrincipals()));
if (rootObj == null) {
throw new NotFoundException("No objects were found for inclusion in manifest for object " + pid.getId());
}
var contentObjs = listViewableFiles(rootObj, agent.getPrincipals());
log.debug("Constructing manifest for {} containing {} items", pid.getId(), contentObjs.size());
ContentObjectRecord rootObj = contentObjs.get(0);

var manifest = new Manifest(getManifestPath(rootObj), new Label(getTitle(rootObj)));
manifest.setMetadata(constructMetadataSection(rootObj));
addAttribution(manifest, rootObj);
Expand Down Expand Up @@ -155,9 +156,12 @@ private void addCanvasItems(Manifest manifest, List<ContentObjectRecord> content
*/
public Canvas buildCanvas(PID pid, AgentPrincipals agent) {
assertHasAccess(pid, agent);
var contentObjs = listViewableFiles(pid, agent.getPrincipals());
ContentObjectRecord rootObj = contentObjs.get(0);
return constructCanvasSection(rootObj);
ContentObjectRecord obj = solrSearchService.getObjectById(new SimpleIdRequest(pid, agent.getPrincipals()));
if (obj == null || !hasViewableContent(obj)) {
throw new NotFoundException("No objects were found for canvas for object " + pid.getId());
}

return constructCanvasSection(obj);
}

/**
Expand Down Expand Up @@ -319,9 +323,9 @@ private boolean hasViewableContent(ContentObjectRecord contentObj) {
}

var jp2Datastream = contentObj.getDatastreamObject(DatastreamType.JP2_ACCESS_COPY.getId());
var isValidDatastream = jp2Datastream != null;
var isValidDatastream = jp2Datastream != null && jp2Datastream.getFilesize() != 0;
var originalDatastream = contentObj.getDatastreamObject(DatastreamType.ORIGINAL_FILE.getId());
// check if original datastream mimetype is image or video
// check if original datastream mimetype is audio or video
if (!isValidDatastream && originalDatastream != null) {
var mimetype = originalDatastream.getMimetype();
isValidDatastream = isAudio(mimetype, contentObj) || isVideo(mimetype);
Expand All @@ -331,27 +335,21 @@ private boolean hasViewableContent(ContentObjectRecord contentObj) {
}

/**
* List viewable files for the specified object
* @param pid
* List viewable files for the specified object's manifest
* @param rootObj
* @param principals
* @return
*/
private List<ContentObjectRecord> listViewableFiles(PID pid, AccessGroupSet principals) {
ContentObjectRecord briefObj = solrSearchService.getObjectById(new SimpleIdRequest(pid, principals));
if (briefObj == null) {
return Collections.emptyList();
}
String resourceType = briefObj.getResourceType();
if (hasViewableContent(briefObj)) {
return Collections.singletonList(briefObj);
private List<ContentObjectRecord> listViewableFiles(ContentObjectRecord rootObj, AccessGroupSet principals) {
String resourceType = rootObj.getResourceType();
if (hasViewableContent(rootObj)) {
return Collections.singletonList(rootObj);
}
if (!ResourceType.Work.nameEquals(resourceType)) {
return Collections.emptyList();
}

var mdObjs = performQuery(briefObj, principals);
mdObjs.add(0, briefObj);
return mdObjs;
return performQuery(rootObj, principals);
}

private List<ContentObjectRecord> performQuery(ContentObjectRecord briefObj, AccessGroupSet principals) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,17 @@ public ResponseEntity<InputStreamResource> getImage(@PathVariable("pid") String
log.error("No content object found for {}", pidString);
return new ResponseEntity<>(HttpStatus.NOT_FOUND);
}

if (downloadImageService.isInvalidJP2Datastream(contentObjectRecord)) {
log.error("No valid JP2 datastream for {}", pidString);
return new ResponseEntity<>(HttpStatus.NOT_FOUND);
}

String validatedSize = downloadImageService.getSize(contentObjectRecord, size);
if (validatedSize == null) {
log.error("No validated size found for {}", pidString);
return new ResponseEntity<>(HttpStatus.NOT_FOUND);
}

if (Objects.equals(validatedSize, ImageServerUtil.FULL_SIZE)) {
aclService.assertHasAccess("Insufficient permissions to download full size copy for " + pidString,
Expand Down
Loading

0 comments on commit 5067b42

Please sign in to comment.