Copilot commented on code in PR #2927:
URL: https://github.com/apache/tika/pull/2927#discussion_r3527894195
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-image-module/src/main/java/org/apache/tika/parser/image/ImageMetadataExtractor.java:
##########
@@ -306,6 +310,53 @@ public void handle(Directory directory, Metadata metadata)
throws MetadataExcept
}
}
+ /**
+ * Copies the XMP properties parsed by Metadata Extractor into the
metadata,
+ * keyed by their {@code prefix:name} path. The other handlers copy a
+ * directory's tags, but XMP keeps its properties in a separate map
+ * ({@link XmpDirectory#getXmpProperties()}), so without this they are
lost.
+ * A property is skipped when its key is already set or matches a known
Tika
+ * field, so normalized values from other handlers are not overwritten.
+ */
+ static class XmpHandler implements DirectoryHandler {
+
+ static {
+ // XMPCore's namespace registry is process-global and keeps the
first
+ // prefix it sees for a URI. Pin canonical prefixes so keys stay
stable
+ // (files use both Camera and GCamera for the Google photo
namespace).
+ // https://developer.android.com/media/platform/motion-photo-format
+ try {
+ XMPMetaFactory.getSchemaRegistry()
+
.registerNamespace("http://ns.google.com/photos/1.0/camera/";, "Camera");
+ XMPMetaFactory.getSchemaRegistry()
+
.registerNamespace("http://ns.google.com/photos/1.0/container/";, "Container");
+ XMPMetaFactory.getSchemaRegistry()
+
.registerNamespace("http://ns.google.com/photos/1.0/container/item/";, "Item");
+ } catch (XMPException e) {
+ // non-fatal: only affects prefix stability
+ }
Review Comment:
The `XMPException` is swallowed silently. If namespace registration fails
(e.g., due to a prior conflicting registration in the process-global registry),
prefix stability becomes non-deterministic with no visibility for debugging.
Consider at least logging this at debug/warn level (or rethrowing as a
`MetadataException` if deterministic keys are required), so operators can
understand why keys may vary.
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-image-module/src/main/java/org/apache/tika/parser/image/ImageMetadataExtractor.java:
##########
@@ -306,6 +310,53 @@ public void handle(Directory directory, Metadata metadata)
throws MetadataExcept
}
}
+ /**
+ * Copies the XMP properties parsed by Metadata Extractor into the
metadata,
+ * keyed by their {@code prefix:name} path. The other handlers copy a
+ * directory's tags, but XMP keeps its properties in a separate map
+ * ({@link XmpDirectory#getXmpProperties()}), so without this they are
lost.
+ * A property is skipped when its key is already set or matches a known
Tika
+ * field, so normalized values from other handlers are not overwritten.
+ */
+ static class XmpHandler implements DirectoryHandler {
+
+ static {
+ // XMPCore's namespace registry is process-global and keeps the
first
+ // prefix it sees for a URI. Pin canonical prefixes so keys stay
stable
+ // (files use both Camera and GCamera for the Google photo
namespace).
+ // https://developer.android.com/media/platform/motion-photo-format
+ try {
+ XMPMetaFactory.getSchemaRegistry()
+
.registerNamespace("http://ns.google.com/photos/1.0/camera/";, "Camera");
+ XMPMetaFactory.getSchemaRegistry()
+
.registerNamespace("http://ns.google.com/photos/1.0/container/";, "Container");
+ XMPMetaFactory.getSchemaRegistry()
+
.registerNamespace("http://ns.google.com/photos/1.0/container/item/";, "Item");
+ } catch (XMPException e) {
+ // non-fatal: only affects prefix stability
+ }
+ }
+
+ public boolean supports(Class<? extends Directory> directoryType) {
+ return XmpDirectory.class.isAssignableFrom(directoryType);
+ }
+
+ public void handle(Directory directory, Metadata metadata) throws
MetadataException {
+ Map<String, String> properties = ((XmpDirectory)
directory).getXmpProperties();
Review Comment:
Add `@Override` annotations to `supports` and `handle` to ensure
compile-time checking that the `DirectoryHandler` contract is being implemented
correctly (helps prevent accidental signature drift).
##########
tika-parsers/tika-parsers-standard/tika-parsers-standard-modules/tika-parser-image-module/src/test/java/org/apache/tika/parser/image/MotionPhotoXmpTest.java:
##########
@@ -0,0 +1,68 @@
+/*
+ * 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.image;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+import org.junit.jupiter.api.Test;
+import org.xml.sax.helpers.DefaultHandler;
+
+import org.apache.tika.TikaTest;
+import org.apache.tika.io.TikaInputStream;
+import org.apache.tika.metadata.Metadata;
+import org.apache.tika.parser.ParseContext;
+
+/**
+ * Google Motion Photos keep their metadata in a vendor XMP namespace. Two
+ * variants share that namespace: the current Motion Photo format and the
legacy
+ * MicroVideo format. Both are covered here.
+ */
+public class MotionPhotoXmpTest extends TikaTest {
+
+ /** XMP from a vendor namespace (Google Motion Photo) is exposed, not
dropped. */
+ @Test
+ public void testMotionPhotoXmpIsExposed() throws Exception {
+ Metadata metadata = new Metadata();
+ metadata.set(Metadata.CONTENT_TYPE, "image/jpeg");
+ try (TikaInputStream tis =
+
getResourceAsStream("/test-documents/testJPEG_MotionPhoto.jpg")) {
+ new JpegParser().parse(tis, new DefaultHandler(), metadata, new
ParseContext());
+ }
+
+ assertEquals("1", metadata.get("Camera:MotionPhoto"));
+ assertEquals("1", metadata.get("Camera:MotionPhotoVersion"));
+ assertEquals("500000",
metadata.get("Camera:MotionPhotoPresentationTimestampUs"));
+ // The embedded video item (its byte length lets a client range-fetch
the
+ // video without downloading the whole file) is exposed too.
+ assertEquals("MotionPhoto",
metadata.get("Container:Directory[2]/Item:Semantic"));
+ assertEquals("122562",
metadata.get("Container:Directory[2]/Item:Length"));
+ }
+
+ /** Keys use the canonical prefix even when the file declares another
(GCamera). */
+ @Test
+ public void testCanonicalPrefixIsStable() throws Exception {
+ Metadata metadata = new Metadata();
+ metadata.set(Metadata.CONTENT_TYPE, "image/jpeg");
+ try (TikaInputStream tis =
+
getResourceAsStream("/test-documents/testJPEG_MicroVideo.jpg")) {
+ new JpegParser().parse(tis, new DefaultHandler(), metadata, new
ParseContext());
+ }
+ assertEquals("1", metadata.get("Camera:MicroVideo"));
+ assertEquals("4182318", metadata.get("Camera:MicroVideoOffset"));
+ assertEquals(null, metadata.get("GCamera:MicroVideoOffset"));
Review Comment:
Prefer `assertNull(metadata.get(\"GCamera:MicroVideoOffset\"))` (and a
static import) instead of `assertEquals(null, ...)` for clearer intent and
better failure messages in JUnit.
--
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]
