Skip to content

Commit

Permalink
Fix discovery exception (openhab#17669)
Browse files Browse the repository at this point in the history
Fixes openhab#17668

Signed-off-by: Jacob Laursen <jacob-github@vindvejr.dk>
  • Loading branch information
jlaur authored Oct 30, 2024
1 parent 488832d commit 276254d
Show file tree
Hide file tree
Showing 10 changed files with 125 additions and 67 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,11 @@

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.eclipse.jetty.client.HttpClient;
import org.openhab.binding.fmiweather.internal.client.Client;
import org.openhab.binding.fmiweather.internal.client.Data;
import org.openhab.binding.fmiweather.internal.client.FMIRequest;
import org.openhab.binding.fmiweather.internal.client.FMIResponse;
import org.openhab.binding.fmiweather.internal.client.Request;
import org.openhab.binding.fmiweather.internal.client.exception.FMIResponseException;
import org.openhab.binding.fmiweather.internal.client.exception.FMIUnexpectedResponseException;
import org.openhab.core.library.types.DateTimeType;
Expand Down Expand Up @@ -68,6 +69,7 @@ public abstract class AbstractWeatherHandler extends BaseThingHandler {

protected static final int TIMEOUT_MILLIS = 10_000;
private final Logger logger = LoggerFactory.getLogger(AbstractWeatherHandler.class);
private final HttpClient httpClient;

protected volatile @NonNullByDefault({}) Client client;
protected final AtomicReference<@Nullable ScheduledFuture<?>> futureRef = new AtomicReference<>();
Expand All @@ -77,8 +79,9 @@ public abstract class AbstractWeatherHandler extends BaseThingHandler {
private volatile long lastRefreshMillis = 0;
private final AtomicReference<@Nullable ScheduledFuture<?>> updateChannelsFutureRef = new AtomicReference<>();

public AbstractWeatherHandler(Thing thing) {
public AbstractWeatherHandler(final Thing thing, final HttpClient httpClient) {
super(thing);
this.httpClient = httpClient;
}

@Override
Expand Down Expand Up @@ -111,7 +114,7 @@ private boolean isSameFuture(@Nullable ScheduledFuture<?> future1, @Nullable Sch

@Override
public void initialize() {
client = new Client();
client = new Client(httpClient);
updateStatus(ThingStatus.UNKNOWN);
rescheduleUpdate(0, false);
}
Expand All @@ -136,7 +139,7 @@ private ScheduledFuture<?> submitUpdateChannelsThrottled() {

protected abstract void updateChannels();

protected abstract Request getRequest();
protected abstract FMIRequest getRequest();

protected void update(int retry) {
if (retry < RETRIES) {
Expand All @@ -145,6 +148,9 @@ protected void update(int retry) {
} catch (FMIResponseException e) {
handleError(e, retry);
return;
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
return;
}
} else {
logger.trace("Query failed. Retries exhausted, not trying again until next poll.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,13 @@

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.eclipse.jetty.client.HttpClient;
import org.openhab.binding.fmiweather.internal.client.Data;
import org.openhab.binding.fmiweather.internal.client.FMIRequest;
import org.openhab.binding.fmiweather.internal.client.FMIResponse;
import org.openhab.binding.fmiweather.internal.client.ForecastRequest;
import org.openhab.binding.fmiweather.internal.client.LatLon;
import org.openhab.binding.fmiweather.internal.client.Location;
import org.openhab.binding.fmiweather.internal.client.Request;
import org.openhab.binding.fmiweather.internal.client.exception.FMIUnexpectedResponseException;
import org.openhab.binding.fmiweather.internal.config.ForecastConfiguration;
import org.openhab.core.thing.Channel;
Expand Down Expand Up @@ -85,8 +86,8 @@ private static void addMapping(String channelId, String requestField, @Nullable
private @NonNullByDefault({}) LatLon location;
private String query = "";

public ForecastWeatherHandler(Thing thing) {
super(thing);
public ForecastWeatherHandler(final Thing thing, final HttpClient httpClient) {
super(thing, httpClient);
// Override poll interval to slower value
pollIntervalSeconds = (int) TimeUnit.MINUTES.toSeconds(QUERY_RESOLUTION_MINUTES);
}
Expand Down Expand Up @@ -119,7 +120,7 @@ public void dispose() {
}

@Override
protected Request getRequest() {
protected FMIRequest getRequest() {
long now = Instant.now().getEpochSecond();
return new ForecastRequest(location, query, floorToEvenMinutes(now, QUERY_RESOLUTION_MINUTES),
ceilToEvenMinutes(now + TimeUnit.HOURS.toSeconds(FORECAST_HORIZON_HOURS), QUERY_RESOLUTION_MINUTES),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,17 @@

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.eclipse.jetty.client.HttpClient;
import org.openhab.core.io.net.http.HttpClientFactory;
import org.openhab.core.thing.Thing;
import org.openhab.core.thing.ThingTypeUID;
import org.openhab.core.thing.binding.BaseThingHandlerFactory;
import org.openhab.core.thing.binding.ThingHandler;
import org.openhab.core.thing.binding.ThingHandlerFactory;
import org.osgi.service.component.ComponentContext;
import org.osgi.service.component.annotations.Activate;
import org.osgi.service.component.annotations.Component;
import org.osgi.service.component.annotations.Reference;

/**
* The {@link HandlerFactory} is responsible for creating things and thing
Expand All @@ -38,6 +43,14 @@ public class HandlerFactory extends BaseThingHandlerFactory {
private static final Set<ThingTypeUID> SUPPORTED_THING_TYPES_UIDS = Set.of(THING_TYPE_OBSERVATION,
THING_TYPE_FORECAST);

private final HttpClient httpClient;

@Activate
public HandlerFactory(final @Reference HttpClientFactory httpClientFactory, ComponentContext componentContext) {
super.activate(componentContext);
this.httpClient = httpClientFactory.getCommonHttpClient();
}

@Override
public boolean supportsThingType(ThingTypeUID thingTypeUID) {
return SUPPORTED_THING_TYPES_UIDS.contains(thingTypeUID);
Expand All @@ -48,9 +61,9 @@ public boolean supportsThingType(ThingTypeUID thingTypeUID) {
ThingTypeUID thingTypeUID = thing.getThingTypeUID();

if (THING_TYPE_OBSERVATION.equals(thingTypeUID)) {
return new ObservationWeatherHandler(thing);
return new ObservationWeatherHandler(thing, httpClient);
} else if (THING_TYPE_FORECAST.equals(thingTypeUID)) {
return new ForecastWeatherHandler(thing);
return new ForecastWeatherHandler(thing, httpClient);
}

return null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,13 @@

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.eclipse.jetty.client.HttpClient;
import org.openhab.binding.fmiweather.internal.client.Data;
import org.openhab.binding.fmiweather.internal.client.FMIRequest;
import org.openhab.binding.fmiweather.internal.client.FMIResponse;
import org.openhab.binding.fmiweather.internal.client.FMISID;
import org.openhab.binding.fmiweather.internal.client.Location;
import org.openhab.binding.fmiweather.internal.client.ObservationRequest;
import org.openhab.binding.fmiweather.internal.client.Request;
import org.openhab.binding.fmiweather.internal.client.exception.FMIUnexpectedResponseException;
import org.openhab.core.library.unit.MetricPrefix;
import org.openhab.core.thing.Channel;
Expand Down Expand Up @@ -107,8 +108,8 @@ private static void addMapping(String channelId, String requestField, @Nullable

private @NonNullByDefault({}) String fmisid;

public ObservationWeatherHandler(Thing thing) {
super(thing);
public ObservationWeatherHandler(final Thing thing, final HttpClient httpClient) {
super(thing, httpClient);
pollIntervalSeconds = POLL_INTERVAL_SECONDS;
}

Expand All @@ -124,7 +125,7 @@ public void initialize() {
}

@Override
protected Request getRequest() {
protected FMIRequest getRequest() {
long now = Instant.now().getEpochSecond();
return new ObservationRequest(new FMISID(fmisid),
floorToEvenMinutes(now - OBSERVATION_LOOK_BACK_SECONDS, STEP_MINUTES),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,9 @@
import java.util.Iterator;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.stream.IntStream;

import javax.xml.namespace.NamespaceContext;
Expand All @@ -33,11 +36,15 @@

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.api.ContentResponse;
import org.eclipse.jetty.client.api.Request;
import org.eclipse.jetty.http.HttpMethod;
import org.eclipse.jetty.http.HttpStatus;
import org.openhab.binding.fmiweather.internal.client.FMIResponse.Builder;
import org.openhab.binding.fmiweather.internal.client.exception.FMIExceptionReportException;
import org.openhab.binding.fmiweather.internal.client.exception.FMIIOException;
import org.openhab.binding.fmiweather.internal.client.exception.FMIUnexpectedResponseException;
import org.openhab.core.io.net.http.HttpUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.w3c.dom.Document;
Expand Down Expand Up @@ -79,6 +86,7 @@ public class Client {
"ef", "http://inspire.ec.europa.eu/schemas/ef/4.0");

private final Logger logger = LoggerFactory.getLogger(Client.class);
private final HttpClient httpClient;

private static final NamespaceContext NAMESPACE_CONTEXT = new NamespaceContext() {
@Override
Expand All @@ -101,7 +109,9 @@ public class Client {
private DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactory.newInstance();
private DocumentBuilder documentBuilder;

public Client() {
public Client(final HttpClient httpClient) {
this.httpClient = httpClient;

documentBuilderFactory.setNamespaceAware(true);
try {
// see https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html
Expand All @@ -126,20 +136,30 @@ public Client() {
* @throws FMIUnexpectedResponseException on all unexpected content errors
* @throws FMIExceptionReportException on explicit error responses from the server
*/
public FMIResponse query(Request request, int timeoutMillis)
throws FMIExceptionReportException, FMIUnexpectedResponseException, FMIIOException {
public FMIResponse query(FMIRequest fmiRequest, int timeoutMillis)
throws FMIExceptionReportException, FMIUnexpectedResponseException, FMIIOException, InterruptedException {
try {
String url = request.toUrl();
String url = fmiRequest.toUrl();
logger.trace("GET request for {}", url);
String responseText = HttpUtil.executeUrl("GET", url, timeoutMillis);
if (responseText == null) {
throw new FMIIOException(String.format("HTTP error with %s", request.toUrl()));
Request request = httpClient.newRequest(url).timeout(timeoutMillis, TimeUnit.MILLISECONDS)
.method(HttpMethod.GET);

ContentResponse response = request.send();

int status = response.getStatus();
if (!HttpStatus.isSuccess(status)) {
throw new FMIIOException("The request failed with HTTP error " + status);
}

String responseContent = response.getContentAsString();
if (responseContent == null) {
throw new FMIIOException(String.format("HTTP error with %s", url));
}
logger.trace("Response content: '{}'", responseText);
FMIResponse response = parseMultiPointCoverageXml(responseText);
logger.debug("Request {} translated to url {}. Response: {}", request, url, response);
return response;
} catch (IOException e) {
logger.trace("Response content: '{}'", responseContent);
FMIResponse fmiResponse = parseMultiPointCoverageXml(responseContent);
logger.debug("Request {} translated to url {}. Response: {}", fmiRequest, url, response);
return fmiResponse;
} catch (IOException | TimeoutException | ExecutionException e) {
throw new FMIIOException(e);
} catch (SAXException | XPathExpressionException e) {
throw new FMIUnexpectedResponseException(e);
Expand All @@ -156,14 +176,24 @@ public FMIResponse query(Request request, int timeoutMillis)
* @throws FMIExceptionReportException on explicit error responses from the server
*/
public Set<Location> queryWeatherStations(int timeoutMillis)
throws FMIIOException, FMIUnexpectedResponseException, FMIExceptionReportException {
throws FMIIOException, FMIUnexpectedResponseException, FMIExceptionReportException, InterruptedException {
try {
String response = HttpUtil.executeUrl("GET", WEATHER_STATIONS_URL, timeoutMillis);
if (response == null) {
Request request = httpClient.newRequest(WEATHER_STATIONS_URL).timeout(timeoutMillis, TimeUnit.MILLISECONDS)
.method(HttpMethod.GET);

ContentResponse response = request.send();

int status = response.getStatus();
if (!HttpStatus.isSuccess(status)) {
throw new FMIIOException("The request failed with HTTP error " + status);
}

String responseContent = response.getContentAsString();
if (responseContent == null) {
throw new FMIIOException(String.format("HTTP error with %s", WEATHER_STATIONS_URL));
}
return parseStations(response);
} catch (IOException e) {
return parseStations(responseContent);
} catch (IOException | TimeoutException | ExecutionException e) {
throw new FMIIOException(e);
} catch (XPathExpressionException | SAXException e) {
throw new FMIUnexpectedResponseException(e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
*
*/
@NonNullByDefault
public class Request {
public class FMIRequest {

public static final String FMI_WFS_URL = "https://opendata.fmi.fi/wfs";

Expand All @@ -40,13 +40,13 @@ public class Request {
private static final ZoneId UTC = ZoneId.of("Z");
private static final DateTimeFormatter FORMATTER = DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss'Z'");

public Request(String storedQueryId, QueryParameter location, long startEpoch, long endEpoch,
public FMIRequest(String storedQueryId, QueryParameter location, long startEpoch, long endEpoch,
long timestepMinutes) {
this(storedQueryId, location, startEpoch, endEpoch, timestepMinutes, new String[0]);
}

public Request(String storedQueryId, QueryParameter location, long startEpoch, long endEpoch, long timestepMinutes,
String[] parameters) {
public FMIRequest(String storedQueryId, QueryParameter location, long startEpoch, long endEpoch,
long timestepMinutes, String[] parameters) {
this.storedQueryId = storedQueryId;
this.location = location;
this.startEpoch = startEpoch;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
*
*/
@NonNullByDefault
public class ForecastRequest extends Request {
public class ForecastRequest extends FMIRequest {

public static final String STORED_QUERY_ID_HARMONIE = "fmi::forecast::harmonie::surface::point::multipointcoverage";
public static final String STORED_QUERY_ID_EDITED = "fmi::forecast::edited::weather::scandinavia::point::multipointcoverage";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
*
*/
@NonNullByDefault
public class ObservationRequest extends Request {
public class ObservationRequest extends FMIRequest {

public static final String STORED_QUERY_ID = "fmi::observations::weather::multipointcoverage";

Expand Down
Loading

0 comments on commit 276254d

Please sign in to comment.