Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[tr064] make SOAP timeout configurable #59

Merged
merged 6 commits into from
Apr 5, 2021
Merged

[tr064] make SOAP timeout configurable #59

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
962cd7e
make SOAP timeout configurable
J-N-K Apr 3, 2021
468af09
Use Util method for phonebook retrieval
J-N-K Apr 4, 2021
288a954
use global constant for default value instead of setting it individually
J-N-K Apr 4, 2021
68cf770
also make phonebook timeout configurable
J-N-K Apr 5, 2021
8a7fa4a
use timeout value for all requests
J-N-K Apr 5, 2021
6345254
improve naming
J-N-K Apr 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions bundles/org.smarthomej.binding.tr064/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ If you only configured password authentication for your device, the `user` param
The second credential parameter is `password`, which is mandatory.
For security reasons it is highly recommended to set both, username and password.

Another optional and advanced configuration parameter is `timeout`.
This parameter applies to all requests to the device (SOAP requests, phonebook retrieval, call lists, ...).
It only needs to be changed from the default value of `5` seconds when the remote device is unexpectedly slow and does not respond within that time.

### `fritzbox`

The `fritzbox` devices can give additional informations in dedicated channels, controlled
Expand Down
29 changes: 16 additions & 13 deletions ...j.binding.tr064/src/main/java/org/smarthomej/binding/tr064/internal/Tr064RootHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@
*/
package org.smarthomej.binding.tr064.internal;

import static org.smarthomej.binding.tr064.internal.Tr064BindingConstants.THING_TYPE_FRITZBOX;
import static org.smarthomej.binding.tr064.internal.Tr064BindingConstants.THING_TYPE_GENERIC;
import static org.smarthomej.binding.tr064.internal.Tr064BindingConstants.*;

import java.net.URI;
import java.net.URISyntaxException;
Expand Down Expand Up @@ -75,12 +74,15 @@ public class Tr064RootHandler extends BaseBridgeHandler implements PhonebookProv
private final Logger logger = LoggerFactory.getLogger(Tr064RootHandler.class);
private final HttpClient httpClient;

private Tr064RootConfiguration config = new Tr064RootConfiguration();
private String deviceType = "";

private @Nullable SCPDUtil scpdUtil;
private SOAPConnector soapConnector;

// these are set when the config is available
private Tr064RootConfiguration config = new Tr064RootConfiguration();
private String endpointBaseURL = "";
private int timeout = Tr064RootConfiguration.DEFAULT_HTTP_TIMEOUT;

private String deviceType = "";

private final Map<ChannelUID, Tr064ChannelConfig> channels = new HashMap<>();
// caching is used to prevent excessive calls to the same action
Expand All @@ -96,7 +98,7 @@ public class Tr064RootHandler extends BaseBridgeHandler implements PhonebookProv
Tr064RootHandler(Bridge bridge, HttpClient httpClient) {
super(bridge);
this.httpClient = httpClient;
this.soapConnector = new SOAPConnector(httpClient, endpointBaseURL);
this.soapConnector = new SOAPConnector(httpClient, endpointBaseURL, timeout);
}

@Override
Expand Down Expand Up @@ -137,7 +139,8 @@ public void initialize() {
}

endpointBaseURL = "http://" + config.host + ":49000";
soapConnector = new SOAPConnector(httpClient, endpointBaseURL);
soapConnector = new SOAPConnector(httpClient, endpointBaseURL, timeout);
timeout = config.timeout;
updateStatus(ThingStatus.UNKNOWN);

connectFuture = scheduler.scheduleWithFixedDelay(this::internalInitialize, 0, RETRY_INTERVAL, TimeUnit.SECONDS);
Expand All @@ -148,7 +151,7 @@ public void initialize() {
*/
private void internalInitialize() {
try {
scpdUtil = new SCPDUtil(httpClient, endpointBaseURL);
scpdUtil = new SCPDUtil(httpClient, endpointBaseURL, timeout);
} catch (SCPDException e) {
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
"could not get device definitions from " + config.host);
Expand Down Expand Up @@ -228,11 +231,11 @@ private boolean establishSecureConnectionAndUpdateProperties() {
SOAPMessage soapResponse = soapConnector
.doSOAPRequest(new SOAPRequest(deviceService, "GetSecurityPort"));
if (!soapResponse.getSOAPBody().hasFault()) {
SOAPValueConverter soapValueConverter = new SOAPValueConverter(httpClient);
SOAPValueConverter soapValueConverter = new SOAPValueConverter(httpClient, timeout);
soapValueConverter.getStateFromSOAPValue(soapResponse, "NewSecurityPort", null)
.ifPresentOrElse(port -> {
endpointBaseURL = "https://" + config.host + ":" + port.toString();
soapConnector = new SOAPConnector(httpClient, endpointBaseURL);
soapConnector = new SOAPConnector(httpClient, endpointBaseURL, timeout);
logger.debug("endpointBaseURL is now '{}'", endpointBaseURL);
}, () -> logger.warn("Could not determine secure port, disabling https"));
} else {
Expand All @@ -253,7 +256,7 @@ private boolean establishSecureConnectionAndUpdateProperties() {
.orElseThrow(() -> new SCPDException("Action 'GetInfo' not found"));
SOAPMessage soapResponse1 = soapConnector
.doSOAPRequest(new SOAPRequest(deviceService, getInfoAction.getName()));
SOAPValueConverter soapValueConverter = new SOAPValueConverter(httpClient);
SOAPValueConverter soapValueConverter = new SOAPValueConverter(httpClient, timeout);
Map<String, String> properties = editProperties();
PROPERTY_ARGUMENTS.forEach(argumentName -> getInfoAction.getArgumentList().stream()
.filter(argument -> argument.getName().equals(argumentName)).findFirst()
Expand Down Expand Up @@ -331,7 +334,7 @@ private void installPolling() {
@SuppressWarnings("unchecked")
private Collection<Phonebook> processPhonebookList(SOAPMessage soapMessagePhonebookList,
SCPDServiceType scpdService) {
SOAPValueConverter soapValueConverter = new SOAPValueConverter(httpClient);
SOAPValueConverter soapValueConverter = new SOAPValueConverter(httpClient, timeout);
Optional<Stream<String>> phonebookStream = soapValueConverter
.getStateFromSOAPValue(soapMessagePhonebookList, "NewPhonebookList", null)
.map(phonebookList -> Arrays.stream(phonebookList.toString().split(",")));
Expand All @@ -343,7 +346,7 @@ private Collection<Phonebook> processPhonebookList(SOAPMessage soapMessagePhoneb
SOAPMessage soapMessageURL = soapConnector
.doSOAPRequest(new SOAPRequest(scpdService, "GetPhonebook", Map.of("NewPhonebookID", index)));
return soapValueConverter.getStateFromSOAPValue(soapMessageURL, "NewPhonebookURL", null)
.map(url -> (Phonebook) new Tr064PhonebookImpl(httpClient, url.toString()));
.map(url -> (Phonebook) new Tr064PhonebookImpl(httpClient, url.toString(), timeout));
} catch (Tr064CommunicationException e) {
logger.warn("Failed to get phonebook with index {}:", index, e);
}
Expand Down
3 changes: 3 additions & 0 deletions ...64/src/main/java/org/smarthomej/binding/tr064/internal/config/Tr064RootConfiguration.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,12 @@
*/
@NonNullByDefault
public class Tr064RootConfiguration extends Tr064BaseThingConfiguration {
public static final int DEFAULT_HTTP_TIMEOUT = 5; // in s

public String host = "";
public String user = "dslf-config";
public String password = "";
public int timeout = DEFAULT_HTTP_TIMEOUT;

/* following parameters only available in fritzbox thing */
public List<String> tamIndices = List.of();
Expand Down
57 changes: 17 additions & 40 deletions ...064/src/main/java/org/smarthomej/binding/tr064/internal/phonebook/Tr064PhonebookImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,31 +13,17 @@
*/
package org.smarthomej.binding.tr064.internal.phonebook;

import java.io.ByteArrayInputStream;
import java.io.InputStream;
import java.util.HashMap;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.stream.Collectors;

import javax.xml.bind.JAXBContext;
import javax.xml.bind.JAXBException;
import javax.xml.bind.Unmarshaller;
import javax.xml.stream.XMLInputFactory;
import javax.xml.stream.XMLStreamException;
import javax.xml.stream.XMLStreamReader;
import javax.xml.transform.stream.StreamSource;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.api.ContentResponse;
import org.eclipse.jetty.http.HttpMethod;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.smarthomej.binding.tr064.internal.dto.additions.PhonebooksType;
import org.smarthomej.binding.tr064.internal.util.Util;

/**
* The {@link Tr064PhonebookImpl} class implements a phonebook
Expand All @@ -52,41 +38,32 @@ public class Tr064PhonebookImpl implements Phonebook {

private final HttpClient httpClient;
private final String phonebookUrl;
private final int httpTimeout;

private String phonebookName = "";

public Tr064PhonebookImpl(HttpClient httpClient, String phonebookUrl) {
public Tr064PhonebookImpl(HttpClient httpClient, String phonebookUrl, int httpTimeout) {
this.httpClient = httpClient;
this.phonebookUrl = phonebookUrl;
this.httpTimeout = httpTimeout;
getPhonebook();
}

private void getPhonebook() {
try {
ContentResponse contentResponse = httpClient.newRequest(phonebookUrl).method(HttpMethod.GET)
.timeout(5, TimeUnit.SECONDS).send();
InputStream xml = new ByteArrayInputStream(contentResponse.getContent());

JAXBContext context = JAXBContext.newInstance(PhonebooksType.class);
XMLInputFactory xif = XMLInputFactory.newFactory();
xif.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false);
xif.setProperty(XMLInputFactory.SUPPORT_DTD, false);
XMLStreamReader xsr = xif.createXMLStreamReader(new StreamSource(xml));
Unmarshaller um = context.createUnmarshaller();
PhonebooksType phonebooksType = um.unmarshal(xsr, PhonebooksType.class).getValue();

phonebookName = phonebooksType.getPhonebook().getName();

phonebook = phonebooksType.getPhonebook().getContact().stream().map(contact -> {
String contactName = contact.getPerson().getRealName();
return contact.getTelephony().getNumber().stream()
.collect(Collectors.toMap(number -> normalizeNumber(number.getValue()), number -> contactName,
this::mergeSameContactNames));
}).collect(HashMap::new, HashMap::putAll, HashMap::putAll);
logger.debug("Downloaded phonebook {}: {}", phonebookName, phonebook);
} catch (JAXBException | InterruptedException | ExecutionException | TimeoutException | XMLStreamException e) {
logger.warn("Failed to get phonebook with URL {}:", phonebookUrl, e);
PhonebooksType phonebooksType = Util.getAndUnmarshalXML(httpClient, phonebookUrl, PhonebooksType.class,
httpTimeout);
if (phonebooksType == null) {
logger.warn("Failed to get phonebook with URL '{}'", phonebookUrl);
return;
}
phonebookName = phonebooksType.getPhonebook().getName();

phonebook = phonebooksType.getPhonebook().getContact().stream().map(contact -> {
String contactName = contact.getPerson().getRealName();
return contact.getTelephony().getNumber().stream().collect(Collectors.toMap(
number -> normalizeNumber(number.getValue()), number -> contactName, this::mergeSameContactNames));
}).collect(HashMap::new, HashMap::putAll, HashMap::putAll);
logger.debug("Downloaded phonebook {}: {}", phonebookName, phonebook);
}

// in case there are multiple phone entries with same number -> name mapping, i.e. in phonebooks exported from
Expand Down
11 changes: 6 additions & 5 deletions ...binding.tr064/src/main/java/org/smarthomej/binding/tr064/internal/soap/SOAPConnector.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,19 +60,20 @@
*/
@NonNullByDefault
public class SOAPConnector {
private static final int SOAP_TIMEOUT = 5; // in
private final Logger logger = LoggerFactory.getLogger(SOAPConnector.class);
private final HttpClient httpClient;
private final String endpointBaseURL;
private final SOAPValueConverter soapValueConverter;
private final int timeout;

private final ExpiringCacheMap<SOAPRequest, SOAPMessage> soapMessageCache = new ExpiringCacheMap<>(
Duration.ofMillis(2000));

public SOAPConnector(HttpClient httpClient, String endpointBaseURL) {
public SOAPConnector(HttpClient httpClient, String endpointBaseURL, int timeout) {
this.httpClient = httpClient;
this.endpointBaseURL = endpointBaseURL;
this.soapValueConverter = new SOAPValueConverter(httpClient);
this.timeout = timeout;
this.soapValueConverter = new SOAPValueConverter(httpClient, timeout);
}

/**
Expand Down Expand Up @@ -162,7 +163,7 @@ public SOAPMessage doSOAPRequest(SOAPRequest soapRequest) throws Tr064Communicat
*/
public synchronized SOAPMessage doSOAPRequestUncached(SOAPRequest soapRequest) throws Tr064CommunicationException {
try {
Request request = prepareSOAPRequest(soapRequest).timeout(SOAP_TIMEOUT, TimeUnit.SECONDS);
Request request = prepareSOAPRequest(soapRequest).timeout(timeout, TimeUnit.SECONDS);
if (logger.isTraceEnabled()) {
request.getContent().forEach(buffer -> logger.trace("Request: {}", new String(buffer.array())));
}
Expand All @@ -172,7 +173,7 @@ public synchronized SOAPMessage doSOAPRequestUncached(SOAPRequest soapRequest) t
// retry once if authentication expired
logger.trace("Re-Auth needed.");
httpClient.getAuthenticationStore().clearAuthenticationResults();
request = prepareSOAPRequest(soapRequest).timeout(SOAP_TIMEOUT, TimeUnit.SECONDS);
request = prepareSOAPRequest(soapRequest).timeout(timeout, TimeUnit.SECONDS);
response = request.send();
}
try (final ByteArrayInputStream is = new ByteArrayInputStream(response.getContent())) {
Expand Down
7 changes: 5 additions & 2 deletions ...ng.tr064/src/main/java/org/smarthomej/binding/tr064/internal/soap/SOAPValueConverter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,11 @@
public class SOAPValueConverter {
private final Logger logger = LoggerFactory.getLogger(SOAPValueConverter.class);
private final HttpClient httpClient;
private final int timeout;

public SOAPValueConverter(HttpClient httpClient) {
public SOAPValueConverter(HttpClient httpClient, int timeout) {
this.httpClient = httpClient;
this.timeout = timeout;
}

/**
Expand Down Expand Up @@ -289,7 +291,8 @@ private State processCallListJSON(State state, Tr064ChannelConfig channelConfig)
*/
private State processCallList(State state, @Nullable String days, CallListType type)
throws PostProcessingException {
Root callListRoot = Util.getAndUnmarshalXML(httpClient, state.toString() + "&days=" + days, Root.class);
Root callListRoot = Util.getAndUnmarshalXML(httpClient, state.toString() + "&days=" + days, Root.class,
timeout);
if (callListRoot == null) {
throw new PostProcessingException("Failed to get call list from URL " + state.toString());
}
Expand Down
9 changes: 6 additions & 3 deletions ...omej.binding.tr064/src/main/java/org/smarthomej/binding/tr064/internal/util/SCPDUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,19 +40,22 @@ public class SCPDUtil {
private SCPDRootType scpdRoot;
private final List<SCPDDeviceType> scpdDevicesList = new ArrayList<>();
private final Map<String, SCPDScpdType> serviceMap = new HashMap<>();
private final int timeout; // timeout for requests in s

public SCPDUtil(HttpClient httpClient, String endpoint) throws SCPDException {
SCPDRootType scpdRoot = Util.getAndUnmarshalXML(httpClient, endpoint + "/tr64desc.xml", SCPDRootType.class);
public SCPDUtil(HttpClient httpClient, String endpoint, int timeout) throws SCPDException {
SCPDRootType scpdRoot = Util.getAndUnmarshalXML(httpClient, endpoint + "/tr64desc.xml", SCPDRootType.class,
timeout);
if (scpdRoot == null) {
throw new SCPDException("could not get SCPD root");
}
this.scpdRoot = scpdRoot;
this.timeout = timeout;

scpdDevicesList.addAll(flatDeviceList(scpdRoot.getDevice()).collect(Collectors.toList()));
for (SCPDDeviceType device : scpdDevicesList) {
for (SCPDServiceType service : device.getServiceList()) {
SCPDScpdType scpd = serviceMap.computeIfAbsent(service.getServiceId(), serviceId -> Util
.getAndUnmarshalXML(httpClient, endpoint + service.getSCPDURL(), SCPDScpdType.class));
.getAndUnmarshalXML(httpClient, endpoint + service.getSCPDURL(), SCPDScpdType.class, timeout));
if (scpd == null) {
throw new SCPDException("could not get SCPD service");
}
Expand Down
8 changes: 4 additions & 4 deletions ...arthomej.binding.tr064/src/main/java/org/smarthomej/binding/tr064/internal/util/Util.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@
@NonNullByDefault
public class Util {
private static final Logger LOGGER = LoggerFactory.getLogger(Util.class);
private static final int HTTP_REQUEST_TIMEOUT = 5; // in s
// cache XML content for 5s
private static final ExpiringCacheMap<String, Object> XML_OBJECT_CACHE = new ExpiringCacheMap<>(
Duration.ofMillis(3000));
Expand Down Expand Up @@ -347,16 +346,17 @@ public static Optional<String> getSOAPElement(SOAPMessage soapMessage, String el
*
* @param uri the uri of the XML file
* @param clazz the class describing the XML file
* @param timeout timeout in s
* @return unmarshalling result
*/
@SuppressWarnings("unchecked")
public static <T> @Nullable T getAndUnmarshalXML(HttpClient httpClient, String uri, Class<T> clazz) {
public static <T> @Nullable T getAndUnmarshalXML(HttpClient httpClient, String uri, Class<T> clazz, int timeout) {
try {
T returnValue = (T) XML_OBJECT_CACHE.putIfAbsentAndGet(uri, () -> {
try {
LOGGER.trace("Refreshing cache for '{}'", uri);
ContentResponse contentResponse = httpClient.newRequest(uri)
.timeout(HTTP_REQUEST_TIMEOUT, TimeUnit.SECONDS).method(HttpMethod.GET).send();
ContentResponse contentResponse = httpClient.newRequest(uri).timeout(timeout, TimeUnit.SECONDS)
.method(HttpMethod.GET).send();
byte[] response = contentResponse.getContent();
if (LOGGER.isTraceEnabled()) {
LOGGER.trace("XML = {}", new String(response));
Expand Down
7 changes: 7 additions & 0 deletions bundles/org.smarthomej.binding.tr064/src/main/resources/OH-INF/thing/thing-types.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,13 @@
<default>60</default>
<unitLabel>s</unitLabel>
</parameter>
<parameter name="timeout" type="integer" unit="s">
<label>Timeout</label>
<description>Timout for all requests (SOAP requests, phonebook retrieval, call lists, ...).</description>
<default>5</default>
<unitLabel>s</unitLabel>
<advanced>true</advanced>
</parameter>
</config-description>
</bridge-type>

Expand Down