From 333cae9e7297d72fbcbe5225922fc85f8b913ad1 Mon Sep 17 00:00:00 2001 From: Christoph Weitkamp Date: Wed, 21 Oct 2020 17:53:46 +0200 Subject: [PATCH] [feed] Minor improvements for Feed Binding (#8824) Signed-off-by: Christoph Weitkamp --- bundles/org.openhab.binding.feed/README.md | 20 ++- .../feed/internal/FeedBindingConstants.java | 6 +- .../feed/internal/FeedHandlerFactory.java | 8 +- .../feed/internal/handler/FeedHandler.java | 115 +++++++++--------- .../resources/OH-INF/thing/thing-types.xml | 21 ++-- .../binding/feed/test/FeedHandlerTest.java | 2 +- .../openhab/binding/feed/test/SlowTests.java | 6 +- 7 files changed, 91 insertions(+), 87 deletions(-) diff --git a/bundles/org.openhab.binding.feed/README.md b/bundles/org.openhab.binding.feed/README.md index 982d52d0e..cec19d547 100644 --- a/bundles/org.openhab.binding.feed/README.md +++ b/bundles/org.openhab.binding.feed/README.md @@ -4,9 +4,7 @@ This binding allows you to integrate feeds in the openHAB environment. The Feed binding downloads the content, tracks for changes, and displays information like feed author, feed title and description, number of entries, last update date. It can be used in combination with openHAB rules to trigger events on feed change. -It uses the [ROME library](https://rometools.github.io/rome/index.html) for parsing -and supports a wide range of popular feed formats - RSS 2.00, RSS 1.00, RSS 0.94, RSS 0.93, RSS 0.92, RSS 0.91 UserLand, -RSS 0.91 Netscape, RSS 0.90, Atom 1.0, Atom 0.3. +It uses the [ROME library](https://rometools.github.io/rome/index.html) for parsing and supports a wide range of popular feed formats - RSS 2.00, RSS 1.00, RSS 0.94, RSS 0.93, RSS 0.92, RSS 0.91 UserLand, RSS 0.91 Netscape, RSS 0.90, Atom 1.0, Atom 0.3. ## Supported Things @@ -24,11 +22,11 @@ No binding configuration required. Required configuration: -- **URL** - the URL of the feed (e.g ). The binding uses this URL to download data +- **URL** - the URL of the feed (e.g ). The binding uses this URL to download data. Optional configuration: -- **refresh** - a refresh interval defines after how many minutes the binding will check, if new content is available. Default value is 20 minutes +- **refresh** - a refresh interval defines after how many minutes the binding will check, if new content is available. Default value is 20 minutes. ## Channels @@ -39,18 +37,18 @@ The binding supports following channels | latest-title | String | Contains the title of the last feed entry. | | latest-description | String | Contains the description of last feed entry. | | latest-date | DateTime | Contains the published date of the last feed entry. | -| author | String | The name of the feed author, if author is present | -| title | String | The title of the feed | -| description | String | Description of the feed | -| last-update | DateTime | The last update date of the feed | -| number-of-entries | Number | Number of entries in the feed | +| author | String | The name of the feed author, if author is present. | +| title | String | The title of the feed. | +| description | String | Description of the feed. | +| last-update | DateTime | The last update date of the feed. | +| number-of-entries | Number | Number of entries in the feed. | ## Example Things: ```java -feed:feed:bbc [ URL="http://feeds.bbci.co.uk/news/video_and_audio/news_front_page/rss.xml?edition=uk"] +feed:feed:bbc [ URL="http://feeds.bbci.co.uk/news/video_and_audio/news_front_page/rss.xml?edition=uk"] feed:feed:techCrunch [ URL="http://feeds.feedburner.com/TechCrunch/", refresh=60] ``` diff --git a/bundles/org.openhab.binding.feed/src/main/java/org/openhab/binding/feed/internal/FeedBindingConstants.java b/bundles/org.openhab.binding.feed/src/main/java/org/openhab/binding/feed/internal/FeedBindingConstants.java index cdc78866a..366f131a7 100644 --- a/bundles/org.openhab.binding.feed/src/main/java/org/openhab/binding/feed/internal/FeedBindingConstants.java +++ b/bundles/org.openhab.binding.feed/src/main/java/org/openhab/binding/feed/internal/FeedBindingConstants.java @@ -12,8 +12,6 @@ */ package org.openhab.binding.feed.internal; -import java.math.BigDecimal; - import org.eclipse.jdt.annotation.NonNullByDefault; import org.openhab.core.thing.ThingTypeUID; @@ -86,11 +84,11 @@ public class FeedBindingConstants { /** * The default auto refresh time in minutes. */ - public static final BigDecimal DEFAULT_REFRESH_TIME = new BigDecimal(20); + public static final long DEFAULT_REFRESH_TIME = 20; /** * The minimum refresh time in milliseconds. Any REFRESH command send to a Thing, before this time has expired, will - * not trigger an attempt to dowload new data form the server. + * not trigger an attempt to download new data from the server. **/ public static final int MINIMUM_REFRESH_TIME = 3000; } diff --git a/bundles/org.openhab.binding.feed/src/main/java/org/openhab/binding/feed/internal/FeedHandlerFactory.java b/bundles/org.openhab.binding.feed/src/main/java/org/openhab/binding/feed/internal/FeedHandlerFactory.java index b090fb95b..c39546c97 100644 --- a/bundles/org.openhab.binding.feed/src/main/java/org/openhab/binding/feed/internal/FeedHandlerFactory.java +++ b/bundles/org.openhab.binding.feed/src/main/java/org/openhab/binding/feed/internal/FeedHandlerFactory.java @@ -14,9 +14,10 @@ package org.openhab.binding.feed.internal; import static org.openhab.binding.feed.internal.FeedBindingConstants.FEED_THING_TYPE_UID; -import java.util.Collections; import java.util.Set; +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; import org.openhab.binding.feed.internal.handler.FeedHandler; import org.openhab.core.thing.Thing; import org.openhab.core.thing.ThingTypeUID; @@ -32,9 +33,10 @@ import org.osgi.service.component.annotations.Component; * @author Svilen Valkanov - Initial contribution */ @Component(service = ThingHandlerFactory.class, configurationPid = "binding.feed") +@NonNullByDefault public class FeedHandlerFactory extends BaseThingHandlerFactory { - private static final Set SUPPORTED_THING_TYPES_UIDS = Collections.singleton(FEED_THING_TYPE_UID); + private static final Set SUPPORTED_THING_TYPES_UIDS = Set.of(FEED_THING_TYPE_UID); @Override public boolean supportsThingType(ThingTypeUID thingTypeUID) { @@ -42,7 +44,7 @@ public class FeedHandlerFactory extends BaseThingHandlerFactory { } @Override - protected ThingHandler createHandler(Thing thing) { + protected @Nullable ThingHandler createHandler(Thing thing) { ThingTypeUID thingTypeUID = thing.getThingTypeUID(); if (thingTypeUID.equals(FEED_THING_TYPE_UID)) { diff --git a/bundles/org.openhab.binding.feed/src/main/java/org/openhab/binding/feed/internal/handler/FeedHandler.java b/bundles/org.openhab.binding.feed/src/main/java/org/openhab/binding/feed/internal/handler/FeedHandler.java index 9d8289651..874654c09 100644 --- a/bundles/org.openhab.binding.feed/src/main/java/org/openhab/binding/feed/internal/handler/FeedHandler.java +++ b/bundles/org.openhab.binding.feed/src/main/java/org/openhab/binding/feed/internal/handler/FeedHandler.java @@ -29,11 +29,12 @@ import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; import java.util.zip.GZIPInputStream; +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; import org.openhab.core.config.core.Configuration; import org.openhab.core.library.types.DateTimeType; import org.openhab.core.library.types.DecimalType; import org.openhab.core.library.types.StringType; -import org.openhab.core.thing.Channel; import org.openhab.core.thing.ChannelUID; import org.openhab.core.thing.Thing; import org.openhab.core.thing.ThingStatus; @@ -57,73 +58,80 @@ import com.rometools.rome.io.SyndFeedInput; * * @author Svilen Valkanov - Initial contribution */ +@NonNullByDefault public class FeedHandler extends BaseThingHandler { - private Logger logger = LoggerFactory.getLogger(FeedHandler.class); + private final Logger logger = LoggerFactory.getLogger(FeedHandler.class); - private String urlString; - private BigDecimal refreshTime; - private ScheduledFuture refreshTask; - private SyndFeed currentFeedState; + private @Nullable URL url; + private long refreshTime; + private @Nullable ScheduledFuture refreshTask; + private @Nullable SyndFeed currentFeedState; private long lastRefreshTime; public FeedHandler(Thing thing) { super(thing); - currentFeedState = null; } @Override public void initialize() { - checkConfiguration(); - updateStatus(ThingStatus.UNKNOWN); - startAutomaticRefresh(); + if (checkConfiguration()) { + updateStatus(ThingStatus.UNKNOWN); + startAutomaticRefresh(); + } } /** * This method checks if the provided configuration is valid. * When invalid parameter is found, default value is assigned. */ - private void checkConfiguration() { + private boolean checkConfiguration() { logger.debug("Start reading Feed Thing configuration."); Configuration configuration = getConfig(); // It is not necessary to check if the URL is valid, this will be done in fetchFeedData() method - urlString = (String) configuration.get(URL); - + String urlString = (String) configuration.get(URL); try { - refreshTime = (BigDecimal) configuration.get(REFRESH_TIME); - if (refreshTime.intValue() <= 0) { + url = new URL(urlString); + } catch (MalformedURLException e) { + logger.warn("Url '{}' is not valid: ", urlString, e); + updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.OFFLINE.CONFIGURATION_ERROR, e.getMessage()); + return false; + } + + BigDecimal localRefreshTime = null; + try { + localRefreshTime = (BigDecimal) configuration.get(REFRESH_TIME); + if (localRefreshTime.intValue() <= 0) { throw new IllegalArgumentException("Refresh time must be positive number!"); } + refreshTime = localRefreshTime.longValue(); } catch (Exception e) { - logger.warn("Refresh time [{}] is not valid. Falling back to default value: {}. {}", refreshTime, + logger.warn("Refresh time [{}] is not valid. Falling back to default value: {}. {}", localRefreshTime, DEFAULT_REFRESH_TIME, e.getMessage()); refreshTime = DEFAULT_REFRESH_TIME; } + return true; } private void startAutomaticRefresh() { - refreshTask = scheduler.scheduleWithFixedDelay(this::refreshFeedState, 0, refreshTime.intValue(), - TimeUnit.MINUTES); - logger.debug("Start automatic refresh at {} minutes", refreshTime.intValue()); + refreshTask = scheduler.scheduleWithFixedDelay(this::refreshFeedState, 0, refreshTime, TimeUnit.MINUTES); + logger.debug("Start automatic refresh at {} minutes!", refreshTime); } private void refreshFeedState() { - SyndFeed feed = fetchFeedData(urlString); + SyndFeed feed = fetchFeedData(); boolean feedUpdated = updateFeedIfChanged(feed); - if (feedUpdated) { - List channels = getThing().getChannels(); - for (Channel channel : channels) { - publishChannelIfLinked(channel.getUID()); - } + getThing().getChannels().forEach(channel -> publishChannelIfLinked(channel.getUID())); } } private void publishChannelIfLinked(ChannelUID channelUID) { String channelID = channelUID.getId(); - if (currentFeedState == null) { + SyndFeed feedState = currentFeedState; + if (feedState == null) { // This will happen if the binding could not download data from the server logger.trace("Cannot update channel with ID {}; no data has been downloaded from the server!", channelID); return; @@ -135,7 +143,7 @@ public class FeedHandler extends BaseThingHandler { } State state = null; - SyndEntry latestEntry = getLatestEntry(currentFeedState); + SyndEntry latestEntry = getLatestEntry(feedState); switch (channelID) { case CHANNEL_LATEST_TITLE: @@ -166,19 +174,19 @@ public class FeedHandler extends BaseThingHandler { } break; case CHANNEL_AUTHOR: - String author = currentFeedState.getAuthor(); + String author = feedState.getAuthor(); state = new StringType(getValueSafely(author)); break; case CHANNEL_DESCRIPTION: - String channelDescription = currentFeedState.getDescription(); + String channelDescription = feedState.getDescription(); state = new StringType(getValueSafely(channelDescription)); break; case CHANNEL_TITLE: - String channelTitle = currentFeedState.getTitle(); + String channelTitle = feedState.getTitle(); state = new StringType(getValueSafely(channelTitle)); break; case CHANNEL_NUMBER_OF_ENTRIES: - int numberOfEntries = currentFeedState.getEntries().size(); + int numberOfEntries = feedState.getEntries().size(); state = new DecimalType(numberOfEntries); break; default: @@ -200,7 +208,7 @@ public class FeedHandler extends BaseThingHandler { * @return true if new content is available on the server since the last update or false * otherwise */ - private synchronized boolean updateFeedIfChanged(SyndFeed newFeedState) { + private synchronized boolean updateFeedIfChanged(@Nullable SyndFeed newFeedState) { // SyndFeed class has implementation of equals () if (newFeedState != null && !newFeedState.equals(currentFeedState)) { currentFeedState = newFeedState; @@ -218,16 +226,18 @@ public class FeedHandler extends BaseThingHandler { * {@link ThingStatusDetail#CONFIGURATION_ERROR} or * {@link ThingStatusDetail#COMMUNICATION_ERROR} and adequate message. * - * @param urlString URL of the Feed * @return {@link SyndFeed} instance with the feed data, if the connection attempt was successful and * null otherwise */ - private SyndFeed fetchFeedData(String urlString) { - SyndFeed feed = null; - try { - URL url = new URL(urlString); + private @Nullable SyndFeed fetchFeedData() { + URL localUrl = url; + if (localUrl == null) { + logger.trace("Url '{}' is not valid: ", localUrl); + return null; + } - URLConnection connection = url.openConnection(); + try { + URLConnection connection = localUrl.openConnection(); connection.setRequestProperty("Accept-Encoding", "gzip"); BufferedReader in = null; @@ -238,50 +248,45 @@ public class FeedHandler extends BaseThingHandler { } SyndFeedInput input = new SyndFeedInput(); - feed = input.build(in); + SyndFeed feed = input.build(in); in.close(); if (this.thing.getStatus() != ThingStatus.ONLINE) { updateStatus(ThingStatus.ONLINE); } - } catch (MalformedURLException e) { - logger.warn("Url '{}' is not valid: ", urlString, e); - updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.OFFLINE.CONFIGURATION_ERROR, e.getMessage()); - return null; + + return feed; } catch (IOException e) { - logger.warn("Error accessing feed: {}", urlString, e); + logger.warn("Error accessing feed: {}", localUrl, e); updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.OFFLINE.COMMUNICATION_ERROR, e.getMessage()); return null; } catch (IllegalArgumentException e) { - logger.warn("Feed URL is null ", e); + logger.warn("Feed URL is null: {} ", localUrl, e); updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.OFFLINE.CONFIGURATION_ERROR, e.getMessage()); return null; } catch (FeedException e) { - logger.warn("Feed content is not valid: {} ", urlString, e); + logger.warn("Feed content is not valid: {} ", localUrl, e); updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.OFFLINE.CONFIGURATION_ERROR, e.getMessage()); return null; } - - return feed; } /** * Returns the most recent entry or null, if no entries are found. */ - private SyndEntry getLatestEntry(SyndFeed feed) { + private @Nullable SyndEntry getLatestEntry(SyndFeed feed) { List allEntries = feed.getEntries(); - SyndEntry lastEntry = null; if (!allEntries.isEmpty()) { /* * The entries are stored in the SyndFeed object in the following order - * the newest entry has index 0. The order is determined from the time the entry was posted, not the * published time of the entry. */ - lastEntry = allEntries.get(0); + return allEntries.get(0); } else { logger.debug("No entries found"); } - return lastEntry; + return null; } @Override @@ -289,7 +294,7 @@ public class FeedHandler extends BaseThingHandler { if (command instanceof RefreshType) { // safeguard for multiple REFRESH commands for different channels in a row if (isMinimumRefreshTimeExceeded()) { - SyndFeed feed = fetchFeedData(urlString); + SyndFeed feed = fetchFeedData(); updateFeedIfChanged(feed); } publishChannelIfLinked(channelUID); @@ -317,7 +322,7 @@ public class FeedHandler extends BaseThingHandler { return true; } - public String getValueSafely(String value) { - return value == null ? new String() : value; + public String getValueSafely(@Nullable String value) { + return value == null ? "" : value; } } diff --git a/bundles/org.openhab.binding.feed/src/main/resources/OH-INF/thing/thing-types.xml b/bundles/org.openhab.binding.feed/src/main/resources/OH-INF/thing/thing-types.xml index dd6275891..258347274 100644 --- a/bundles/org.openhab.binding.feed/src/main/resources/OH-INF/thing/thing-types.xml +++ b/bundles/org.openhab.binding.feed/src/main/resources/OH-INF/thing/thing-types.xml @@ -6,8 +6,9 @@ + short + description and other information like images, comments and etc. Entry in this binding is abstraction for RSS item element + and Atom entry element. --> @@ -28,18 +29,16 @@ - The URL of the feed + The URL of the feed. - - Refresh time interval in minutes. 20 - @@ -67,35 +66,35 @@ String - The name of the feed author, if author is present + The name of the feed author, if author is present. String - The title of the feed + The title of the feed. String - Description of the feed + Description of the feed. DateTime - The last update date of the feed + The last update date of the feed. Number - Number of entries in the feed + Number of entries in the feed. diff --git a/itests/org.openhab.binding.feed.tests/src/main/java/org/openhab/binding/feed/test/FeedHandlerTest.java b/itests/org.openhab.binding.feed.tests/src/main/java/org/openhab/binding/feed/test/FeedHandlerTest.java index a34c38b97..fea0dba78 100644 --- a/itests/org.openhab.binding.feed.tests/src/main/java/org/openhab/binding/feed/test/FeedHandlerTest.java +++ b/itests/org.openhab.binding.feed.tests/src/main/java/org/openhab/binding/feed/test/FeedHandlerTest.java @@ -93,7 +93,7 @@ public class FeedHandlerTest extends JavaOSGiTest { /** * It is updated from mocked {@link StateChangeListener#stateUpdated() } */ - private StringType currentItemState = null; + private StringType currentItemState; // Required services for the test private ManagedThingProvider managedThingProvider; diff --git a/itests/org.openhab.binding.feed.tests/src/main/java/org/openhab/binding/feed/test/SlowTests.java b/itests/org.openhab.binding.feed.tests/src/main/java/org/openhab/binding/feed/test/SlowTests.java index 4aca7103d..4e858916d 100644 --- a/itests/org.openhab.binding.feed.tests/src/main/java/org/openhab/binding/feed/test/SlowTests.java +++ b/itests/org.openhab.binding.feed.tests/src/main/java/org/openhab/binding/feed/test/SlowTests.java @@ -12,11 +12,13 @@ */ package org.openhab.binding.feed.test; +import org.eclipse.jdt.annotation.NonNullByDefault; + /** * This interface is used to mark tests that take too much time * - * @author Svilen Valkanov + * @author Svilen Valkanov - Initial contribution */ +@NonNullByDefault public interface SlowTests { - }