Copilot commented on code in PR #3095: URL: https://github.com/apache/tika/pull/3095#discussion_r3891597255
########## tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/WMFParser.java: ########## @@ -32,25 +32,56 @@ import org.xml.sax.SAXException; import org.apache.tika.annotation.TikaComponent; +import org.apache.tika.config.ConfigDeserializer; +import org.apache.tika.config.JsonConfig; +import org.apache.tika.config.ParseContextConfig; import org.apache.tika.exception.TikaException; import org.apache.tika.io.TikaInputStream; import org.apache.tika.metadata.Metadata; +import org.apache.tika.metadata.TikaCoreProperties; import org.apache.tika.mime.MediaType; import org.apache.tika.parser.ParseContext; import org.apache.tika.parser.Parser; +import org.apache.tika.parser.RenderingParser; +import org.apache.tika.renderer.Renderer; import org.apache.tika.sax.XHTMLContentHandler; /** * This parser offers a very rough capability to extract text if there * is text stored in the WMF files. */ Review Comment: There are two consecutive class-level Javadoc blocks; only the second one is used by Javadoc tooling and the first becomes redundant noise. ########## tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/MetafileRendering.java: ########## @@ -0,0 +1,135 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.tika.parser.microsoft; + +import java.io.IOException; + +import org.xml.sax.SAXException; + +import org.apache.tika.exception.TikaException; +import org.apache.tika.extractor.EmbeddedDocumentExtractor; +import org.apache.tika.extractor.EmbeddedDocumentUtil; +import org.apache.tika.io.TikaInputStream; +import org.apache.tika.metadata.HttpHeaders; +import org.apache.tika.metadata.Metadata; +import org.apache.tika.metadata.TikaCoreProperties; +import org.apache.tika.mime.MediaType; +import org.apache.tika.parser.ParseContext; +import org.apache.tika.renderer.RenderResult; +import org.apache.tika.renderer.RenderResults; +import org.apache.tika.renderer.Renderer; +import org.apache.tika.renderer.microsoft.POIMetafileRenderer; +import org.apache.tika.sax.EmbeddedContentHandler; +import org.apache.tika.sax.XHTMLContentHandler; + +/** + * Shared by {@link EMFParser} and {@link WMFParser}: renders a parsed + * metafile through a {@link Renderer} and emits the result as an embedded + * document, the way the PDF parser emits page renderings. The rendering of + * a {@link TikaCoreProperties.EmbeddedResourceType#THUMBNAIL} is itself a + * THUMBNAIL (it is the same picture in a form a client can display); any + * other rendering is a {@link TikaCoreProperties.EmbeddedResourceType#RENDERING}. + */ +final class MetafileRendering { + + private MetafileRendering() { + } + + /** + * @param injected the renderer set on the parser, or null + * @param picture the parsed {@code HemfPicture} or {@code HwmfPicture} + */ + static void render(Renderer injected, MetafileParserConfig config, MediaType type, + Object picture, XHTMLContentHandler xhtml, Metadata metadata, + ParseContext context) throws IOException, SAXException { + //like the PDF parser: the injected renderer if it handles the type, + //the default one otherwise + Renderer renderer = injected != null && injected.getSupportedTypes(context).contains(type) + ? injected : defaultRenderer(config); + Metadata renderMetadata = Metadata.newInstance(context); + renderMetadata.set(TikaCoreProperties.TYPE, type.toString()); + EmbeddedDocumentExtractor extractor = + EmbeddedDocumentUtil.getEmbeddedDocumentExtractor(context); + try (TikaInputStream pictureStream = TikaInputStream.get(new byte[0]); + RenderResults results = render(renderer, pictureStream, picture, renderMetadata, + context)) { + if (results == null) { + return; + } Review Comment: If renderer.render(...) throws, MetafileRendering.render(...) returns with results == null and no exception is recorded on the parent metadata, so rendering failures can be silently swallowed. ########## tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/renderer/microsoft/POIMetafileRenderer.java: ########## @@ -0,0 +1,259 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.tika.renderer.microsoft; + +import java.awt.Color; +import java.awt.Graphics2D; +import java.awt.RenderingHints; +import java.awt.geom.Dimension2D; +import java.awt.geom.Rectangle2D; +import java.awt.image.BufferedImage; +import java.io.IOException; +import java.io.OutputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; +import javax.imageio.ImageIO; + +import org.apache.poi.hemf.usermodel.HemfPicture; +import org.apache.poi.hwmf.record.HwmfFill; +import org.apache.poi.hwmf.record.HwmfRecord; +import org.apache.poi.hwmf.usermodel.HwmfPicture; + +import org.apache.tika.annotation.TikaComponent; +import org.apache.tika.exception.TikaException; +import org.apache.tika.io.TemporaryResources; +import org.apache.tika.io.TikaInputStream; +import org.apache.tika.metadata.HttpHeaders; +import org.apache.tika.metadata.Metadata; +import org.apache.tika.metadata.Rendering; +import org.apache.tika.metadata.TikaCoreProperties; +import org.apache.tika.mime.MediaType; +import org.apache.tika.parser.ParseContext; +import org.apache.tika.renderer.RenderRequest; +import org.apache.tika.renderer.RenderResult; +import org.apache.tika.renderer.RenderResults; +import org.apache.tika.renderer.Renderer; +import org.apache.tika.renderer.RenderingTracker; + +/** + * Renders EMF and WMF images to a raster image through POI's HEMF and HWMF, + * the way {@code PDFBoxRenderer} renders PDF pages. The rendering has the + * configured width, its height follows the image's aspect ratio, and it is + * drawn on a white canvas. Metafiles have no pages, so the render requests + * are ignored and a single result is returned. + * <p> + * The WMF thumbnails that Word stores in the SummaryInformation of a .doc + * consist of a window extent and a single {@code dibStretchBlt} record, for + * which POI cannot compute bounds; those are rendered from the record's + * bitmap directly. + */ +@TikaComponent(name = "poi-metafile-renderer") +public class POIMetafileRenderer implements Renderer { + + public static final String RENDERED_BY = "poi-metafile-renderer"; + + public static final MediaType EMF = MediaType.image("emf"); + public static final MediaType WMF = MediaType.image("wmf"); + + private static final Set<MediaType> SUPPORTED_TYPES = + Collections.unmodifiableSet(new HashSet<>(Arrays.asList(EMF, WMF))); + + private static final int MAX_WIDTH = 10000; + + private int width = 800; + private String imageFormatName = "png"; + + @Override + public Set<MediaType> getSupportedTypes(ParseContext context) { + return SUPPORTED_TYPES; + } + + /** + * Renders the metafile in the stream, or the {@link HemfPicture} or + * {@link HwmfPicture} set as the stream's open container. The metadata's + * {@link TikaCoreProperties#TYPE} tells EMF from WMF when a stream is + * parsed; it defaults to EMF. + */ + @Override + public RenderResults render(TikaInputStream tis, Metadata metadata, ParseContext parseContext, + RenderRequest... requests) throws IOException, TikaException { + Object picture = tis.getOpenContainer(); + if (!(picture instanceof HemfPicture) && !(picture instanceof HwmfPicture)) { + picture = WMF.toString().equals(metadata.get(TikaCoreProperties.TYPE)) + ? new HwmfPicture(tis) : new HemfPicture(tis); + } + RenderingTracker tracker = parseContext.get(RenderingTracker.class); + if (tracker == null) { + tracker = new RenderingTracker(); + parseContext.set(RenderingTracker.class, tracker); + } + int id = tracker.getNextId(); + Metadata renderingMetadata = Metadata.newInstance(parseContext); + renderingMetadata.set(TikaCoreProperties.EMBEDDED_RESOURCE_TYPE, + TikaCoreProperties.EmbeddedResourceType.RENDERING.name()); + RenderResults results = new RenderResults(new TemporaryResources()); + try { + long start = System.currentTimeMillis(); + BufferedImage image = picture instanceof HemfPicture + ? draw((HemfPicture) picture) : draw((HwmfPicture) picture); + Path tmpFile = write(image, id); + renderingMetadata.set(Rendering.RENDERED_MS, System.currentTimeMillis() - start); + renderingMetadata.add(Rendering.RENDERED_BY, RENDERED_BY); + renderingMetadata.set(HttpHeaders.CONTENT_TYPE, "image/" + imageFormatName); + results.add(new RenderResult(RenderResult.STATUS.SUCCESS, id, tmpFile, + renderingMetadata)); + } catch (SecurityException e) { + throw e; + } catch (Exception e) { + results.add(new RenderResult(RenderResult.STATUS.EXCEPTION, id, null, + renderingMetadata)); + } Review Comment: On rendering exceptions, the renderer currently emits an EXCEPTION RenderResult but does not record the underlying exception details into the result metadata. This makes downstream diagnostics much harder (contrast with PDFBoxRenderer which records exceptions). This issue also appears on line 256 of the same file. ########## CHANGES.txt: ########## @@ -1,5 +1,16 @@ Release 4.1.0 - unreleased + * Raster previews for the vector thumbnails of Office documents: the new + poi-metafile-renderer draws EMF and WMF images through POI (a PNG of + a configurable width; Word's bitmap-in-WMF thumbnails from the bitmap + directly), EMFParser and WMFParser are RenderingParsers that emit the + rendering as a RENDERING embedded document with "emf-parser" / + "wmf-parser": {"renderImage": true, "renderWidth": 800}, off by + default and restrictable to e.g. THUMBNAIL embedded documents with + "renderOnlyEmbeddedResourceTypes", and OfficeParser emits the SummaryInformation thumbnail of the + OLE2 formats (a WMF) as a THUMBNAIL embedded document, as the OOXML + parsers do with the docProps thumbnail (TIKA-4855). Review Comment: This release note line is significantly longer than surrounding wrapped lines, which makes CHANGES.txt harder to read in terminals. Please wrap it consistently with the surrounding formatting. ########## tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-microsoft-module/src/main/java/org/apache/tika/parser/microsoft/EMFParser.java: ########## @@ -62,12 +68,39 @@ * We'd have to do something like what PDFBox or XPS do to sort the * runs and then put the cow back together from the hamburger...lol... */ +/** + * Extracts the text of an EMF image and its embedded WMF and multi-format Review Comment: The class now has two consecutive Javadoc blocks (one ending just above this new one). Only one should remain; please merge/remove the older block so the class has a single coherent Javadoc. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
