From e802cfa8eb4e2a70d6625ce913390d3c170450bd Mon Sep 17 00:00:00 2001 From: Aurelien Violette Date: Mon, 3 Dec 2018 12:44:15 +0100 Subject: [PATCH 1/2] 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. --- .../sitemaps/extension/VideoAttributes.java | 10 ++--- .../sitemaps/sax/extension/VideoHandler.java | 2 +- .../sitemaps/SiteMapParserExtensionTest.java | 42 +++++++++++++++++++ .../extension/sitemap-videos-error.xml | 33 +++++++++++++++ 4 files changed, 81 insertions(+), 6 deletions(-) create mode 100644 src/test/resources/sitemaps/extension/sitemap-videos-error.xml diff --git a/src/main/java/crawlercommons/sitemaps/extension/VideoAttributes.java b/src/main/java/crawlercommons/sitemaps/extension/VideoAttributes.java index 2f6e810..ca1fa02 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; @@ -473,7 +473,7 @@ public class VideoAttributes extends ExtensionMetadata { return resolution; } - public float getPrice() { + public Float getPrice() { return 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; 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 @@ + + + + + http://www.example.com/videos/some_video_landing_page.html + + http://www.example.com/thumbs/123.jpg + Grilling steaks for summer + Alkis shows you how to get perfectly done steaks every + time + http://www.example.com/video123.flv + + http://www.example.com/videoplayer.swf?video=123 + 600 + 2009-11-05T19:20:30+08:00 + 4.2 + 12345 + 2007-11-05T19:20:30+08:00 + yes + sample_tag1 + sample_tag2 + IE GB US CA + http://cooking.example.com + 1,99 + yes + GrillyMcGrillerson + + no + + + \ No newline at end of file From 3c12c715c8b00f18efdc8fb378f762ae708669fb Mon Sep 17 00:00:00 2001 From: Aurelien Violette Date: Mon, 3 Dec 2018 12:55:30 +0100 Subject: [PATCH 2/2] Fix object comparison on testing. --- .../crawlercommons/sitemaps/extension/VideoAttributes.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/crawlercommons/sitemaps/extension/VideoAttributes.java b/src/main/java/crawlercommons/sitemaps/extension/VideoAttributes.java index ca1fa02..45a61db 100644 --- a/src/main/java/crawlercommons/sitemaps/extension/VideoAttributes.java +++ b/src/main/java/crawlercommons/sitemaps/extension/VideoAttributes.java @@ -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); } @@ -477,7 +477,7 @@ public class VideoAttributes extends ExtensionMetadata { return price; } - public void setPrice(float price) { + public void setPrice(Float price) { this.price = price; }