Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
5 changes: 3 additions & 2 deletions bundles/org.openhab.binding.pihole/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,10 @@ The Pi-hole Binding allows you to monitor Pi-hole statistics and control its fun

| Name | Type | Description | Default | Required | Advanced |
|-----------------|---------|-------------------------------------------------------------------------------------------|---------|----------|----------|
| hostname | text | Hostname or IP address of the device | N/A | yes | no |
| hostname | text | Url (hostname or IP address) of the pihole web server | N/A | yes | no |
| token | text | Token to access the device. To generate token go to `settings` > `API` > `Show API token` | N/A | yes | no |
| refreshInterval | integer | Interval the device is polled in sec. | 600 | no | yes |
| refreshInterval | integer | Interval the device is polled in sec | 600 | no | yes |
| serverVersion | text | Defines the API to be used with this server | v5 | no | yes |

## Channels

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,11 @@ public void disableBlocking(
return;
}

if (timeUnit == null) {
timeUnit = SECONDS;
}

var local = handler;
if (local == null) {
return;
}
local.disableBlocking(timeUnit.toSeconds(time));
local.disableBlocking((timeUnit == null ? SECONDS : timeUnit).toSeconds(time));
}

public static void disableBlocking(@Nullable ThingActions actions, long time, @Nullable TimeUnit timeUnit)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,11 @@
*/
@NonNullByDefault
public class PiHoleConfiguration {
public static final String API_V6 = "v6";
public static final String API_V5 = "v5";

public String hostname = "";
public String token = "";
public int refreshIntervalSeconds = 600;
public String serverVersion = API_V5;
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,10 @@
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.eclipse.jetty.client.HttpClient;
import org.openhab.binding.pihole.internal.PiHoleBindingConstants.Channels.DisableEnable;
import org.openhab.binding.pihole.internal.rest.AdminService;
import org.openhab.binding.pihole.internal.rest.JettyAdminService;
import org.openhab.binding.pihole.internal.rest.JettyAdminServiceV6;
import org.openhab.binding.pihole.internal.rest.model.DnsStatistics;
import org.openhab.core.i18n.TimeZoneProvider;
import org.openhab.core.library.types.DateTimeType;
Expand All @@ -51,28 +53,32 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.google.gson.Gson;

/**
* The {@link PiHoleHandler} is responsible for handling commands, which are
* sent to one of the channels.
*
* @author Martin Grzeslowski - Initial contribution
*/
@NonNullByDefault
public class PiHoleHandler extends BaseThingHandler implements AdminService {
public class PiHoleHandler extends BaseThingHandler {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Making AdminService a class required you to remove implements from here. It's not perfect cause adding new methods to JettyAdminService will not require you to expose those methods here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not see any obvious reason to expose these methods here. They are still available to actions.

private static final int HTTP_DELAY_SECONDS = 1;
private final Logger logger = LoggerFactory.getLogger(PiHoleHandler.class);
private final Object lock = new Object();
private final TimeZoneProvider timeZoneProvider;
private final HttpClient httpClient;
private final Gson gson;

private @Nullable AdminService adminService;
private @Nullable DnsStatistics dnsStatistics;
private @Nullable ScheduledFuture<?> scheduledFuture;

public PiHoleHandler(Thing thing, TimeZoneProvider timeZoneProvider, HttpClient httpClient) {
public PiHoleHandler(Thing thing, TimeZoneProvider timeZoneProvider, HttpClient httpClient, Gson gson) {
super(thing);
this.timeZoneProvider = timeZoneProvider;
this.httpClient = httpClient;
this.gson = gson;
}

@Override
Expand Down Expand Up @@ -101,7 +107,11 @@ public void initialize() {
updateStatus(OFFLINE, CONFIGURATION_ERROR, "@text/handler.init.noToken");
return;
}
adminService = new JettyAdminService(config.token, hostname, httpClient);

adminService = PiHoleConfiguration.API_V6.equals(config.serverVersion)
? new JettyAdminServiceV6(config.token, hostname, httpClient, gson)
: new JettyAdminService(config.token, hostname, httpClient, gson);

scheduledFuture = scheduler.scheduleWithFixedDelay(this::update, 0, config.refreshIntervalSeconds, SECONDS);

// do not set status here, the background task will do it.
Expand Down Expand Up @@ -234,7 +244,6 @@ public void dispose() {
super.dispose();
}

@Override
public Optional<DnsStatistics> summary() throws PiHoleException {
var local = adminService;
if (local == null) {
Expand All @@ -243,7 +252,6 @@ public Optional<DnsStatistics> summary() throws PiHoleException {
return local.summary();
}

@Override
public void disableBlocking(long seconds) throws PiHoleException {
var local = adminService;
if (local == null) {
Expand All @@ -259,7 +267,6 @@ public void disableBlocking(long seconds) throws PiHoleException {
}
}

@Override
public void enableBlocking() throws PiHoleException {
var local = adminService;
if (local == null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@
import org.osgi.service.component.annotations.Component;
import org.osgi.service.component.annotations.Reference;

import com.google.gson.FieldNamingPolicy;
import com.google.gson.Gson;
import com.google.gson.GsonBuilder;

/**
* The {@link PiHoleHandlerFactory} is responsible for creating things and thing
* handlers.
Expand All @@ -38,8 +42,10 @@
@NonNullByDefault
@Component(configurationPid = "binding.pihole", service = ThingHandlerFactory.class)
public class PiHoleHandlerFactory extends BaseThingHandlerFactory {

private static final Set<ThingTypeUID> SUPPORTED_THING_TYPES_UIDS = Set.of(PI_HOLE_TYPE);
private static final Gson GSON = new GsonBuilder()
.setFieldNamingPolicy(FieldNamingPolicy.LOWER_CASE_WITH_UNDERSCORES).create();

private final TimeZoneProvider timeZoneProvider;
private final HttpClientFactory httpClientFactory;

Expand All @@ -59,10 +65,8 @@ public boolean supportsThingType(ThingTypeUID thingTypeUID) {
protected @Nullable ThingHandler createHandler(Thing thing) {
ThingTypeUID thingTypeUID = thing.getThingTypeUID();

if (PI_HOLE_TYPE.equals(thingTypeUID)) {
return new PiHoleHandler(thing, timeZoneProvider, httpClientFactory.getCommonHttpClient());
}

return null;
return PI_HOLE_TYPE.equals(thingTypeUID)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not happy with this change. Adding more handlers will require you to get back to previous approach

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I'll revert.

? new PiHoleHandler(thing, timeZoneProvider, httpClientFactory.getCommonHttpClient(), GSON)
: null;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,36 +13,67 @@
package org.openhab.binding.pihole.internal.rest;

import java.util.Optional;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeoutException;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.api.ContentResponse;
import org.eclipse.jetty.client.api.Request;
import org.openhab.binding.pihole.internal.PiHoleException;
import org.openhab.binding.pihole.internal.rest.model.DnsStatistics;

import com.google.gson.Gson;

/**
* @author Martin Grzeslowski - Initial contribution
* @author Gaël L'hopital - Changed from 'interface' to abstract class
*/
@NonNullByDefault
public interface AdminService {
public abstract class AdminService {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you make it a class? i do not see any logic added

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it shares properties (and 1 method) between the two versions of the AdminService

protected static final long TIMEOUT_SECONDS = 10L;

protected final String token;

protected final HttpClient client;
protected final Gson gson;

public AdminService(String token, HttpClient client, Gson gson) {
super();
this.token = token;
this.client = client;
this.gson = gson;
}

/**
* Retrieves a summary of DNS statistics.
*
* @return An optional containing the DNS statistics.
* @throws PiHoleException In case of error
*/
Optional<DnsStatistics> summary() throws PiHoleException;
public abstract Optional<DnsStatistics> summary() throws PiHoleException;

/**
* Disables blocking for a specified duration.
*
* @param seconds The duration in seconds for which blocking should be disabled.
* @throws PiHoleException In case of error
*/
void disableBlocking(long seconds) throws PiHoleException;
public abstract void disableBlocking(long seconds) throws PiHoleException;

/**
* Enables blocking.
*
* @throws PiHoleException In case of error
*/
void enableBlocking() throws PiHoleException;
public abstract void enableBlocking() throws PiHoleException;

protected static ContentResponse send(Request request) throws PiHoleException {
try {
return request.send();
} catch (InterruptedException | TimeoutException | ExecutionException e) {
throw new PiHoleException(
"Exception while sending request to Pi-hole. %s".formatted(e.getLocalizedMessage()), e);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,39 +16,28 @@

import java.net.URI;
import java.util.Optional;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeoutException;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.client.api.ContentResponse;
import org.eclipse.jetty.client.api.Request;
import org.openhab.binding.pihole.internal.PiHoleException;
import org.openhab.binding.pihole.internal.rest.model.DnsStatistics;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.google.gson.FieldNamingPolicy;
import com.google.gson.Gson;
import com.google.gson.GsonBuilder;

/**
* @author Martin Grzeslowski - Initial contribution
*/
@NonNullByDefault
public class JettyAdminService implements AdminService {
private static final Gson GSON = new GsonBuilder()
.setFieldNamingPolicy(FieldNamingPolicy.LOWER_CASE_WITH_UNDERSCORES).create();
private static final long TIMEOUT_SECONDS = 10L;
public class JettyAdminService extends AdminService {
private final Logger logger = LoggerFactory.getLogger(JettyAdminService.class);
private final String token;

private final URI baseUrl;
private final HttpClient client;

public JettyAdminService(String token, URI baseUrl, HttpClient client) {
this.token = token;
public JettyAdminService(String token, URI baseUrl, HttpClient client, Gson gson) {
super(token, client, gson);
this.baseUrl = baseUrl;
this.client = client;
}

@Override
Expand All @@ -58,16 +47,7 @@ public Optional<DnsStatistics> summary() throws PiHoleException {
var request = client.newRequest(url).timeout(TIMEOUT_SECONDS, SECONDS);
var response = send(request);
var content = response.getContentAsString();
return Optional.ofNullable(GSON.fromJson(content, DnsStatistics.class));
}

private static ContentResponse send(Request request) throws PiHoleException {
try {
return request.send();
} catch (InterruptedException | TimeoutException | ExecutionException e) {
throw new PiHoleException(
"Exception while sending request to Pi-hole. %s".formatted(e.getLocalizedMessage()), e);
}
return Optional.ofNullable(gson.fromJson(content, DnsStatistics.class));
}

@Override
Expand Down
Loading