From 0dae4f25623c99cd79844d2c7d7b86ce55c7d2b5 Mon Sep 17 00:00:00 2001 From: Romain Deltour Date: Mon, 30 Dec 2024 17:58:52 +0100 Subject: [PATCH] refactor: tweak the OCFRersources closing code - Introduce an `OCFResources` interface (currently only implemented by `OCFZipResources`) to prepare for other kink of OCF resources like unpackaged EPUBs with no intermediary ZIP archive. - call the `close()` method in a `finally` block --- .../com/adobe/epubcheck/ocf/OCFChecker.java | 302 +++++++++--------- .../adobe/epubcheck/ocf/OCFCheckerState.java | 10 +- .../com/adobe/epubcheck/ocf/OCFResources.java | 9 + .../adobe/epubcheck/ocf/OCFZipResources.java | 2 +- 4 files changed, 170 insertions(+), 153 deletions(-) create mode 100644 src/main/java/com/adobe/epubcheck/ocf/OCFResources.java diff --git a/src/main/java/com/adobe/epubcheck/ocf/OCFChecker.java b/src/main/java/com/adobe/epubcheck/ocf/OCFChecker.java index ecdc40c02..32c00811d 100755 --- a/src/main/java/com/adobe/epubcheck/ocf/OCFChecker.java +++ b/src/main/java/com/adobe/epubcheck/ocf/OCFChecker.java @@ -74,177 +74,184 @@ public void check() // It will be augmented with detected validation version, profile, etc. OCFCheckerState state = new OCFCheckerState(context); - // Check the EPUB file (zip) - // ------------------------- - // - checkZipFile(); - - // Check the OCF Container file structure - // -------------------------------------- - // - if (!checkContainerStructure(state)) - { - return; - } - ; - OCFContainer container = state.getContainer(); - - // - // Check the mimetype file - // ------------------------------ - // - checkMimetypeFile(state); - - // - // Check the container.xml file - // ---------------------------- - // - if (!checkContainerFile(state)) + try { - return; - } - List packageDocs = state.getPackageDocuments(); + // Check the EPUB file (zip) + // ------------------------- + // + checkZipFile(); + + // Check the OCF Container file structure + // -------------------------------------- + // + if (!checkContainerStructure(state)) + { + return; + } + ; + OCFContainer container = state.getContainer(); + + // + // Check the mimetype file + // ------------------------------ + // + checkMimetypeFile(state); + + // + // Check the container.xml file + // ---------------------------- + // + if (!checkContainerFile(state)) + { + return; + } + List packageDocs = state.getPackageDocuments(); - // - // Check the declared package documents - // ------------------------------------ - // - if (!checkDeclaredPackageDocuments(state)) - { - return; - } + // + // Check the declared package documents + // ------------------------------------ + // + if (!checkDeclaredPackageDocuments(state)) + { + return; + } - // - // Override the context-provided version and profile - // by what is actually declared in the publication - // ------------------------------------------------- - EPUBVersion validationVersion = checkPublicationVersion(state); - state.setVersion(validationVersion); - state.setProfile(checkPublicationProfile(state, validationVersion)); - - // - // Check if there are multiple renditions - // -------------------------------------- - // EPUB 2.0 says there SHOULD be only one OPS rendition - if (validationVersion == EPUBVersion.VERSION_2 && packageDocs.size() > 1) - { - report.message(MessageId.PKG_013, OCFMetaFile.CONTAINER.asLocation(container)); - } - // EPUB 3.0 Multiple Renditions recommends the presence of a metadata file - if (validationVersion == EPUBVersion.VERSION_3 && packageDocs.size() > 1) - { - state.addProperty(EpubCheckVocab.VOCAB.get(EpubCheckVocab.PROPERTIES.MULTIPLE_RENDITION)); - if (!OCFMetaFile.METADATA.isPresent(container)) + // + // Override the context-provided version and profile + // by what is actually declared in the publication + // ------------------------------------------------- + EPUBVersion validationVersion = checkPublicationVersion(state); + state.setVersion(validationVersion); + state.setProfile(checkPublicationProfile(state, validationVersion)); + + // + // Check if there are multiple renditions + // -------------------------------------- + // EPUB 2.0 says there SHOULD be only one OPS rendition + if (validationVersion == EPUBVersion.VERSION_2 && packageDocs.size() > 1) { - report.message(MessageId.RSC_019, EPUBLocation.of(context)); + report.message(MessageId.PKG_013, OCFMetaFile.CONTAINER.asLocation(container)); } - if (state.getMappingDocument().isPresent()) + // EPUB 3.0 Multiple Renditions recommends the presence of a metadata file + if (validationVersion == EPUBVersion.VERSION_3 && packageDocs.size() > 1) { - new MappingDocumentChecker( - state.context().mimetype("application/xhtml+xml").url(state.getMappingDocument().get()) - .addProperty(EpubCheckVocab.VOCAB.get(EpubCheckVocab.PROPERTIES.RENDITION_MAPPING)) - .build()).check(); + state.addProperty(EpubCheckVocab.VOCAB.get(EpubCheckVocab.PROPERTIES.MULTIPLE_RENDITION)); + if (!OCFMetaFile.METADATA.isPresent(container)) + { + report.message(MessageId.RSC_019, EPUBLocation.of(context)); + } + if (state.getMappingDocument().isPresent()) + { + new MappingDocumentChecker( + state.context().mimetype("application/xhtml+xml") + .url(state.getMappingDocument().get()) + .addProperty( + EpubCheckVocab.VOCAB.get(EpubCheckVocab.PROPERTIES.RENDITION_MAPPING)) + .build()).check(); + } } - } - // - // Check other META-INF files - // ------------------------------ - // - checkEncryptionFile(state); - checkOtherMetaFiles(state); - container = state.getContainer(); - - // - // Check each Publication (i.e. Package Document) - // ---------------------------------------------- - // - List opfHandlers = new LinkedList(); - for (URL packageDoc : packageDocs) - { - ValidationContextBuilder opfContext = state.context().url(packageDoc) - .mimetype(MIMEType.PACKAGE_DOC.toString()).featureReport(new FeatureReport()); + // + // Check other META-INF files + // ------------------------------ + // + checkEncryptionFile(state); + checkOtherMetaFiles(state); + container = state.getContainer(); + + // + // Check each Publication (i.e. Package Document) + // ---------------------------------------------- + // + List opfHandlers = new LinkedList(); + for (URL packageDoc : packageDocs) + { + ValidationContextBuilder opfContext = state.context().url(packageDoc) + .mimetype(MIMEType.PACKAGE_DOC.toString()).featureReport(new FeatureReport()); - opfContext.container(container); - opfContext.pubTypes(state.getPublicationTypes(packageDoc)); + opfContext.container(container); + opfContext.pubTypes(state.getPublicationTypes(packageDoc)); - Checker opfChecker = CheckerFactory.newChecker(opfContext.build()); - assert opfChecker instanceof OPFChecker; - opfChecker.check(); - opfHandlers.add(((OPFChecker) opfChecker).getOPFHandler()); - } + Checker opfChecker = CheckerFactory.newChecker(opfContext.build()); + assert opfChecker instanceof OPFChecker; + opfChecker.check(); + opfHandlers.add(((OPFChecker) opfChecker).getOPFHandler()); + } - // - // Check container consistency with Package Documents content - // ---------------------------------------------------------- - // + // + // Check container consistency with Package Documents content + // ---------------------------------------------------------- + // - for (final URL resource : container.getResources()) - { - String path = resource.path().substring(1); - // if the entry is not in the whitelist (META-INF/* + mimetype) - // and not declared in (one of) the OPF document(s) - - // FIXME 2022 add a method to register known files in state, to fix #1115 - // (in this case, only the last mapping document is considered known) - if (!path.startsWith("META-INF/") && !path.startsWith("META-INF\\") - && !path.equals("mimetype") && !state.isDeclared(resource) - && !Iterables.tryFind(opfHandlers, new Predicate() - { - @Override - public boolean apply(OPFHandler opfHandler) - { - // found if declared as an OPF item - // or in an EPUB 3 link element - return opfHandler.getItemByURL(resource).isPresent() - || (validationVersion == EPUBVersion.VERSION_3 - && ((OPFHandler30) opfHandler).getLinkedResources().hasResource(resource)); - } - }).isPresent()) + for (final URL resource : container.getResources()) { - report.message(MessageId.OPF_003, EPUBLocation.of(context), path); - } + String path = resource.path().substring(1); + // if the entry is not in the whitelist (META-INF/* + mimetype) + // and not declared in (one of) the OPF document(s) + + // FIXME 2022 add a method to register known files in state, to fix + // #1115 + // (in this case, only the last mapping document is considered known) + if (!path.startsWith("META-INF/") && !path.startsWith("META-INF\\") + && !path.equals("mimetype") && !state.isDeclared(resource) + && !Iterables.tryFind(opfHandlers, new Predicate() + { + @Override + public boolean apply(OPFHandler opfHandler) + { + // found if declared as an OPF item + // or in an EPUB 3 link element + return opfHandler.getItemByURL(resource).isPresent() + || (validationVersion == EPUBVersion.VERSION_3 + && ((OPFHandler30) opfHandler).getLinkedResources().hasResource(resource)); + } + }).isPresent()) + { + report.message(MessageId.OPF_003, EPUBLocation.of(context), path); + } - // check obfuscated resource are Font Core Media Types - if (state.isObfuscated(resource)) - { - for (OPFHandler opf : opfHandlers) + // check obfuscated resource are Font Core Media Types + if (state.isObfuscated(resource)) { - // try to find the first Package Document where the entry is - // declared - Optional item = opf.getItemByURL(resource); - if (item.isPresent()) + for (OPFHandler opf : opfHandlers) { - // report if it is not a font core media type - if (!OPFChecker30.isBlessedFontType(item.get().getMimeType())) + // try to find the first Package Document where the entry is + // declared + Optional item = opf.getItemByURL(resource); + if (item.isPresent()) { - report.message(MessageId.PKG_026, state.getObfuscationLocation(resource), - item.get().getMimeType(), opf.getPath()); + // report if it is not a font core media type + if (!OPFChecker30.isBlessedFontType(item.get().getMimeType())) + { + report.message(MessageId.PKG_026, state.getObfuscationLocation(resource), + item.get().getMimeType(), opf.getPath()); + } + break; } - break; } } } - } - - // - // Check other file properties - // --------------------------- - // - checkFileExtension(state); - // Close zip file to free resource - if (state.getZipResources() != null) + // + // Check other file properties + // --------------------------- + // + checkFileExtension(state); + } finally { - try + // Close resources + if (state.getResources() != null) { - state.getZipResources().close(); - } - catch (Exception e) - { - // FIXME 2023 - Inability to close zip file should be handled + try + { + state.getResources().close(); + } catch (IOException e) + { + throw new RuntimeException(e); + } } } + } private boolean checkContainerFile(OCFCheckerState state) @@ -278,14 +285,15 @@ private boolean checkContainerStructure(OCFCheckerState state) // FIXME 2022 build resourcesProvider depending on MIME type // Get a container OCFZipResources resourcesProvider = new OCFZipResources(context.url); - // Store the OCFZipResources object so it can be closed later - state.setZipResources(resourcesProvider); // Set to store the normalized paths for duplicate checks final Set normalizedPaths = new HashSet<>(); // Lists to store the container entries for later empty directory check final List filePaths = new LinkedList<>(); final List directoryPaths = new LinkedList<>(); + // Store the OCFZipResources object so it can be closed later + state.setResources(resourcesProvider); + // Loop through the entries for (OCFResource resource : resourcesProvider) { diff --git a/src/main/java/com/adobe/epubcheck/ocf/OCFCheckerState.java b/src/main/java/com/adobe/epubcheck/ocf/OCFCheckerState.java index 4381752a3..c79999c04 100644 --- a/src/main/java/com/adobe/epubcheck/ocf/OCFCheckerState.java +++ b/src/main/java/com/adobe/epubcheck/ocf/OCFCheckerState.java @@ -41,7 +41,7 @@ class OCFCheckerState private final ImmutableList.Builder packageDocuments = ImmutableList.builder(); private URL mappingDocument; - private OCFZipResources zipResources = null; + private OCFResources resources = null; private final Map obfuscated = new HashMap<>(); private final Map> publicationTypes = new LinkedHashMap<>(); @@ -86,14 +86,14 @@ public void addResource(OCFResource resource) containerNeedsRebuild = true; } - public OCFZipResources getZipResources() + public OCFResources getResources() { - return zipResources; + return resources; } - public void setZipResources(OCFZipResources zipResources) + public void setResources(OCFResources resources) { - this.zipResources = zipResources; + this.resources = resources; } public void addRootfile(String mediaType, URL resource) diff --git a/src/main/java/com/adobe/epubcheck/ocf/OCFResources.java b/src/main/java/com/adobe/epubcheck/ocf/OCFResources.java new file mode 100644 index 000000000..8a7ef49a4 --- /dev/null +++ b/src/main/java/com/adobe/epubcheck/ocf/OCFResources.java @@ -0,0 +1,9 @@ +package com.adobe.epubcheck.ocf; + +import java.io.IOException; + +public interface OCFResources extends Iterable +{ + public void close() + throws IOException; +} diff --git a/src/main/java/com/adobe/epubcheck/ocf/OCFZipResources.java b/src/main/java/com/adobe/epubcheck/ocf/OCFZipResources.java index d7cbe2321..1d9ae69d0 100644 --- a/src/main/java/com/adobe/epubcheck/ocf/OCFZipResources.java +++ b/src/main/java/com/adobe/epubcheck/ocf/OCFZipResources.java @@ -22,7 +22,7 @@ import io.mola.galimatias.URL; -public class OCFZipResources implements Iterable +public class OCFZipResources implements OCFResources { private final ZipFile zip;