Repository: crawler-commons/crawler-commons
Pull request #220 (2018-12-03 11:48:11)

In case of the use of a different locale, price tag can be formatted …
…with ',' instead of '.' leading to a NPE.

NPE is generated because parseFloat returns a Float object that can be set null in case of NumberFormatException, but the VideoPrice accepts only float.
To bypass this issue and avoid reccuring errors, I've moved the VideoPrice price field to a Float object instead accepting null in case of.

It is far from ideal, and parseFloat would enjoy being able to parse different locale formatting. Anyway, in a first quick fix, this allows the rest of the file to be parsed,
whereas the previous error had all the file to fail while parsing.


Patch:
diff --git a/src/main/java/crawlercommons/sitemaps/extension/VideoAttributes.java b/src/main/java/crawlercommons/sitemaps/extension/VideoAttributes.java
index 2f6e810..45a61db 100644
--- a/src/main/java/crawlercommons/sitemaps/extension/VideoAttributes.java
+++ b/src/main/java/crawlercommons/sitemaps/extension/VideoAttributes.java
@@ -424,17 +424,17 @@ public class VideoAttributes extends ExtensionMetadata {
         /**
          * Video price
          */
-        private float price;
+        private Float price;
 
-        public VideoPrice(final String currency, final float price) {
+        public VideoPrice(final String currency, final Float price) {
             this(currency, price, VideoPriceType.own);
         }
 
-        public VideoPrice(final String currency, final float price, final VideoPriceType type) {
+        public VideoPrice(final String currency, final Float price, final VideoPriceType type) {
             this(currency, price, type, null);
         }
 
-        public VideoPrice(final String currency, final float price, final VideoPriceType type, final VideoPriceResolution resolution) {
+        public VideoPrice(final String currency, final Float price, final VideoPriceType type, final VideoPriceResolution resolution) {
             this.currency = currency;
             this.price = price;
             this.type = type;
@@ -456,7 +456,7 @@ public class VideoAttributes extends ExtensionMetadata {
             }
             VideoPrice that = (VideoPrice) other;
             return Objects.equals(currency, that.currency) //
-                            && price == that.price //
+                            && Objects.equals(price, that.price) //
                             && type == that.type //
                             && Objects.equals(resolution, that.resolution);
         }
@@ -473,11 +473,11 @@ public class VideoAttributes extends ExtensionMetadata {
             return resolution;
         }
 
-        public float getPrice() {
+        public Float getPrice() {
             return price;
         }
 
-        public void setPrice(float price) {
+        public void setPrice(Float price) {
             this.price = price;
         }
 
diff --git a/src/main/java/crawlercommons/sitemaps/sax/extension/VideoHandler.java b/src/main/java/crawlercommons/sitemaps/sax/extension/VideoHandler.java
index 7405f98..89c3348 100644
--- a/src/main/java/crawlercommons/sitemaps/sax/extension/VideoHandler.java
+++ b/src/main/java/crawlercommons/sitemaps/sax/extension/VideoHandler.java
@@ -148,7 +148,7 @@ public class VideoHandler extends ExtensionHandler {
         } else if ("gallery_loc".equals(localName)) {
             currAttr.setGalleryLoc(getURLValue(value));
         } else if ("price".equals(localName)) {
-            float fvalue = getFloatValue(value);
+            Float fvalue = getFloatValue(value);
             String currency = null;
             VideoPriceType type = VideoPriceType.own;
             VideoPriceResolution resolution = null;


Test patch:
diff --git a/src/test/java/crawlercommons/sitemaps/SiteMapParserExtensionTest.java b/src/test/java/crawlercommons/sitemaps/SiteMapParserExtensionTest.java
index b09ac66..baf4b8a 100644
--- a/src/test/java/crawlercommons/sitemaps/SiteMapParserExtensionTest.java
+++ b/src/test/java/crawlercommons/sitemaps/SiteMapParserExtensionTest.java
@@ -257,4 +257,46 @@ public class SiteMapParserExtensionTest {
         assertEquals(74, sm.getSiteMapUrls().size());
     }
 
+
+    @Test
+    public void testVideosSitemapWithError() throws UnknownFormatException, IOException {
+        SiteMapParser parser = new SiteMapParser();
+        parser.enableExtension(Extension.VIDEO);
+
+        String contentType = "text/xml";
+        byte[] content = SiteMapParserTest.getResourceAsBytes("src/test/resources/sitemaps/extension/sitemap-videos-error.xml");
+
+        URL url = new URL("http://www.example.com/sitemap-video.xml");
+        AbstractSiteMap asm = parser.parseSiteMap(contentType, content, url);
+        assertEquals(false, asm.isIndex());
+        assertEquals(true, asm instanceof SiteMap);
+        SiteMap sm = (SiteMap) asm;
+        assertEquals(1, sm.getSiteMapUrls().size());
+        VideoAttributes expectedVideoAttributes = new VideoAttributes(new URL("http://www.example.com/thumbs/123.jpg"), "Grilling steaks for summer",
+                "Alkis shows you how to get perfectly done steaks every time", new URL("http://www.example.com/video123.flv"), new URL("http://www.example.com/videoplayer.swf?video=123"));
+        expectedVideoAttributes.setDuration(600);
+        ZonedDateTime dt = ZonedDateTime.parse("2009-11-05T19:20:30+08:00");
+        expectedVideoAttributes.setExpirationDate(dt);
+        dt = ZonedDateTime.parse("2007-11-05T19:20:30+08:00");
+        expectedVideoAttributes.setPublicationDate(dt);
+        expectedVideoAttributes.setRating(4.2f);
+        expectedVideoAttributes.setViewCount(12345);
+        expectedVideoAttributes.setFamilyFriendly(true);
+        expectedVideoAttributes.setTags(new String[] { "sample_tag1", "sample_tag2" });
+        expectedVideoAttributes.setAllowedCountries(new String[] { "IE", "GB", "US", "CA" });
+        expectedVideoAttributes.setGalleryLoc(new URL("http://cooking.example.com"));
+        expectedVideoAttributes.setGalleryTitle("Cooking Videos");
+        expectedVideoAttributes.setPrices(new VideoAttributes.VideoPrice[] { new VideoAttributes.VideoPrice("EUR", null, VideoAttributes.VideoPriceType.own) });
+        expectedVideoAttributes.setRequiresSubscription(true);
+        expectedVideoAttributes.setUploader("GrillyMcGrillerson");
+        expectedVideoAttributes.setUploaderInfo(new URL("http://www.example.com/users/grillymcgrillerson"));
+        expectedVideoAttributes.setLive(false);
+
+        for (SiteMapURL su : sm.getSiteMapUrls()) {
+            assertNotNull(su.getAttributesForExtension(Extension.VIDEO));
+            VideoAttributes attr = (VideoAttributes) su.getAttributesForExtension(Extension.VIDEO)[0];
+            assertEquals(expectedVideoAttributes, attr);
+        }
+    }
+
 }
diff --git a/src/test/resources/sitemaps/extension/sitemap-videos-error.xml b/src/test/resources/sitemaps/extension/sitemap-videos-error.xml
new file mode 100644
index 0000000..d08ee1c
--- /dev/null
+++ b/src/test/resources/sitemaps/extension/sitemap-videos-error.xml
@@ -0,0 +1,33 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9"
+        xmlns:video="http://www.google.com/schemas/sitemap-video/1.1" >
+    <!-- example from https://support.google.com/webmasters/answer/80471 -->
+    <url>
+        <loc>http://www.example.com/videos/some_video_landing_page.html</loc>
+        <video:video>
+            <video:thumbnail_loc>http://www.example.com/thumbs/123.jpg</video:thumbnail_loc>
+            <video:title>Grilling steaks for summer</video:title>
+            <video:description>Alkis shows you how to get perfectly done steaks every
+                time</video:description>
+            <video:content_loc>http://www.example.com/video123.flv</video:content_loc>
+            <video:player_loc allow_embed="yes" autoplay="ap=1">
+                http://www.example.com/videoplayer.swf?video=123</video:player_loc>
+            <video:duration>600</video:duration>
+            <video:expiration_date>2009-11-05T19:20:30+08:00</video:expiration_date>
+            <video:rating>4.2</video:rating>
+            <video:view_count>12345</video:view_count>
+            <video:publication_date>2007-11-05T19:20:30+08:00</video:publication_date>
+            <video:family_friendly>yes</video:family_friendly>
+            <video:tag>sample_tag1</video:tag>
+            <video:tag>sample_tag2</video:tag>
+            <video:restriction relationship="allow">IE GB US CA</video:restriction>
+            <video:gallery_loc title="Cooking Videos">http://cooking.example.com</video:gallery_loc>
+            <video:price currency="EUR">1,99</video:price>
+            <video:requires_subscription>yes</video:requires_subscription>
+            <video:uploader
+                    info="http://www.example.com/users/grillymcgrillerson">GrillyMcGrillerson
+            </video:uploader>
+            <video:live>no</video:live>
+        </video:video>
+    </url>
+</urlset>
\ No newline at end of file