Skip to content

Commit

Permalink
[unifi] Fix bug with combination of other data/ports (openhab#14060)
Browse files Browse the repository at this point in the history 
- It seems to throw an exception when updating internal cache. It can happen if you have a switch that has both PoE ports and other PoE ports or data in the port override.
- Fixed logout, should be POST instead of GET.
- Fixed typo in channel-type.config.unifi.poeEnable.mode.option.pasv24 should be without appending v.
- Removed compiler warnings.

Signed-off-by: Hilbrand Bouwkamp <[email protected]>
  • Loading branch information
@Hilbrand @borazslo
Hilbrand authored and borazslo committed Jan 8, 2023
1 parent 0d3897d commit 820f8de
Show file tree
Hide file tree
Showing 15 changed files with 242 additions and 106 deletions.
23 changes: 11 additions & 12 deletions ...b.binding.unifi/src/main/java/org/openhab/binding/unifi/internal/api/UniFiController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,17 @@

import java.util.Collection;
import java.util.List;
import java.util.Map;

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.eclipse.jetty.client.HttpClient;
import org.eclipse.jetty.http.HttpMethod;
import org.openhab.binding.unifi.internal.api.cache.UniFiControllerCache;
import org.openhab.binding.unifi.internal.api.dto.UnfiPortOverrideJsonElement;
import org.openhab.binding.unifi.internal.api.dto.UnfiPortOverrideJsonObject;
import org.openhab.binding.unifi.internal.api.dto.UniFiClient;
import org.openhab.binding.unifi.internal.api.dto.UniFiDevice;
import org.openhab.binding.unifi.internal.api.dto.UniFiPortTuple;
import org.openhab.binding.unifi.internal.api.dto.UniFiSite;
import org.openhab.binding.unifi.internal.api.dto.UniFiSwitchPorts;
import org.openhab.binding.unifi.internal.api.dto.UniFiUnknownClient;
import org.openhab.binding.unifi.internal.api.dto.UniFiWiredClient;
import org.openhab.binding.unifi.internal.api.dto.UniFiWirelessClient;
Expand All @@ -42,6 +41,7 @@
import com.google.gson.FieldNamingPolicy;
import com.google.gson.Gson;
import com.google.gson.GsonBuilder;
import com.google.gson.JsonObject;

/**
* The {@link UniFiController} is the main communication point with an external instance of the Ubiquiti Networks
Expand Down Expand Up @@ -94,7 +94,7 @@ public UniFiController(final HttpClient httpClient, final String host, final int
.registerTypeAdapter(UniFiWiredClient.class, clientInstanceCreator)
.registerTypeAdapter(UniFiWirelessClient.class, clientInstanceCreator).create();
this.poeGson = new GsonBuilder()
.registerTypeAdapter(UnfiPortOverrideJsonElement.class, new UnfiPortOverrideJsonElementDeserializer())
.registerTypeAdapter(UnfiPortOverrideJsonObject.class, new UnfiPortOverrideJsonElementDeserializer())
.create();
}

Expand Down Expand Up @@ -133,7 +133,7 @@ public void login() throws UniFiException {

public void logout() throws UniFiException {
csrfToken = "";
final UniFiControllerRequest<Void> req = newRequest(Void.class, HttpMethod.GET, gson);
final UniFiControllerRequest<Void> req = newRequest(Void.class, HttpMethod.POST, gson);
req.setPath(unifios ? "/api/auth/logout" : "/logout");
executeRequest(req);
}
Expand All @@ -153,7 +153,7 @@ public UniFiControllerCache getCache() {
return cache;
}

public @Nullable Map<Integer, UniFiPortTuple> getSwitchPorts(@Nullable final String deviceId) {
public @Nullable UniFiSwitchPorts getSwitchPorts(@Nullable final String deviceId) {
return cache.getSwitchPorts(deviceId);
}

Expand All @@ -175,10 +175,9 @@ public void reconnect(final UniFiClient client) throws UniFiException {
refresh();
}

public boolean poeMode(final UniFiDevice device, final List<UnfiPortOverrideJsonElement> data)
throws UniFiException {
public boolean poeMode(final UniFiDevice device, final List<JsonObject> data) throws UniFiException {
// Safety check to make sure no empty data is send to avoid corrupting override data on the device.
if (data.isEmpty() || data.stream().anyMatch(p -> p.getJsonObject().entrySet().isEmpty())) {
if (data.isEmpty() || data.stream().anyMatch(p -> p.entrySet().isEmpty())) {
logger.info("Not overriding port for '{}', because port data contains empty json: {}", device.getName(),
poeGson.toJson(data));
return false;
Expand Down Expand Up @@ -225,7 +224,7 @@ private <T> UniFiControllerRequest<T> newRequest(final Class<T> responseType, fi
throws UniFiException {
T result;
try {
result = request.execute();
result = (T) request.execute();
csrfToken = request.getCsrfToken();
} catch (final UniFiExpiredSessionException e) {
if (fromLogin) {
Expand All @@ -234,11 +233,11 @@ private <T> UniFiControllerRequest<T> newRequest(final Class<T> responseType, fi
throw new UniFiCommunicationException(e);
} else {
login();
result = executeRequest(request);
result = (T) executeRequest(request);
}
} catch (final UniFiNotAuthorizedException e) {
logger.warn("Not Authorized! Please make sure your controller credentials have administrator rights");
result = null;
result = (T) null;
}
return result;
}
Expand Down
4 changes: 2 additions & 2 deletions ...ng.unifi/src/main/java/org/openhab/binding/unifi/internal/api/UniFiControllerRequest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,14 +124,14 @@ public void setQueryParameter(final String key, final Object value) {
}

public @Nullable T execute() throws UniFiException {
T result = null;
T result = (T) null;
final String json = getContent();
// mgb: only try and unmarshall non-void result types
if (!Void.class.equals(resultType)) {
final JsonObject jsonObject = JsonParser.parseString(json).getAsJsonObject();

if (jsonObject.has(PROPERTY_DATA) && jsonObject.get(PROPERTY_DATA).isJsonArray()) {
result = gson.fromJson(jsonObject.getAsJsonArray(PROPERTY_DATA), resultType);
result = (T) gson.fromJson(jsonObject.getAsJsonArray(PROPERTY_DATA), resultType);
}
}
return result;
Expand Down
4 changes: 3 additions & 1 deletion ....binding.unifi/src/main/java/org/openhab/binding/unifi/internal/api/cache/UniFiCache.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,9 @@ public final void putAll(final T @Nullable [] values) {
logger.debug("Put #{} entries in {}: {}", values.length, getClass().getSimpleName(),
lazyFormatAsList(values));
for (final T value : values) {
put(value.getId(), value);
if (value != null) {
put(value.getId(), value);
}
}
}
}
Expand Down
41 changes: 20 additions & 21 deletions ...nifi/src/main/java/org/openhab/binding/unifi/internal/api/cache/UniFiControllerCache.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
package org.openhab.binding.unifi.internal.api.cache;

import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand All @@ -23,12 +22,11 @@

import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.binding.unifi.internal.api.dto.UnfiPortOverrideJsonElement;
import org.openhab.binding.unifi.internal.api.dto.UniFiClient;
import org.openhab.binding.unifi.internal.api.dto.UniFiDevice;
import org.openhab.binding.unifi.internal.api.dto.UniFiPortTable;
import org.openhab.binding.unifi.internal.api.dto.UniFiPortTuple;
import org.openhab.binding.unifi.internal.api.dto.UniFiSite;
import org.openhab.binding.unifi.internal.api.dto.UniFiSwitchPorts;
import org.openhab.binding.unifi.internal.api.dto.UniFiWlan;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -49,7 +47,7 @@ public class UniFiControllerCache {
private final UniFiDeviceCache devicesCache = new UniFiDeviceCache();
private final UniFiClientCache clientsCache = new UniFiClientCache();
private final UniFiClientCache insightsCache = new UniFiClientCache();
private final Map<String, Map<Integer, UniFiPortTuple>> devicesToPortTables = new ConcurrentHashMap<>();
private final Map<String, UniFiSwitchPorts> devicesToPortTables = new ConcurrentHashMap<>();

public void clear() {
sitesCache.clear();
Expand Down Expand Up @@ -94,23 +92,23 @@ public void putDevices(final UniFiDevice @Nullable [] devices) {
devicesCache.putAll(devices);
if (devices != null) {
Stream.of(devices).filter(Objects::nonNull).forEach(d -> {
Stream.ofNullable(d.getPortTable()).flatMap(pt -> Stream.of(pt)).filter(UniFiPortTable::isPortPoe)
.forEach(p -> {
final Map<Integer, UniFiPortTuple> tupleTable = devicesToPortTables
.computeIfAbsent(d.getMac(), tt -> new HashMap<>());
final UniFiPortTuple tuple = tupleTable.computeIfAbsent(p.getPortIdx(),
t -> new UniFiPortTuple());

tuple.setDevice(d);
tuple.setTable(p);
});
Stream.ofNullable(d.getPortTable()).forEach(pt -> {
final UniFiSwitchPorts switchPorts = devicesToPortTables.computeIfAbsent(d.getMac(),
p -> new UniFiSwitchPorts());

Stream.of(pt).forEach(p -> {
@SuppressWarnings("null")
final UniFiPortTuple tuple = switchPorts.computeIfAbsent(p.getPortIdx());

tuple.setDevice(d);
tuple.setTable(p);
});
});
Stream.ofNullable(d.getPortOverrides()).forEach(po -> {
final Map<Integer, UniFiPortTuple> tupleTable = devicesToPortTables.get(d.getMac());
final UniFiSwitchPorts tupleTable = devicesToPortTables.get(d.getMac());

if (tupleTable != null) {
Stream.of(po).filter(pof -> !pof.getAsJsonObject().entrySet().isEmpty())
.map(UnfiPortOverrideJsonElement::new).forEach(p -> tupleTable
.computeIfAbsent(p.getPortIdx(), t -> new UniFiPortTuple()).setJsonElement(p));
Stream.of(po).forEach(p -> tupleTable.setOverride(p));
}
});
});
Expand All @@ -121,11 +119,12 @@ public void putDevices(final UniFiDevice @Nullable [] devices) {
return devicesCache.get(id);
}

public Map<Integer, UniFiPortTuple> getSwitchPorts(@Nullable final String deviceId) {
return deviceId == null ? Map.of() : devicesToPortTables.getOrDefault(deviceId, Map.of());
public UniFiSwitchPorts getSwitchPorts(@Nullable final String deviceId) {
return deviceId == null ? new UniFiSwitchPorts()
: devicesToPortTables.getOrDefault(deviceId, new UniFiSwitchPorts());
}

public Collection<Map<Integer, UniFiPortTuple>> getSwitchPorts() {
public Collection<UniFiSwitchPorts> getSwitchPorts() {
return devicesToPortTables.values();
}

Expand Down
3 changes: 2 additions & 1 deletion ...ng.unifi/src/main/java/org/openhab/binding/unifi/internal/api/cache/UniFiDeviceCache.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ public UniFiDeviceCache() {
switch (prefix) {
case MAC:
return device.getMac();
default:
return null;
}
return null;
}
}
11 changes: 7 additions & 4 deletions .../api/dto/UnfiPortOverrideJsonElement.java → ...l/api/dto/UnfiPortOverrideJsonObject.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
*/
package org.openhab.binding.unifi.internal.api.dto;

import com.google.gson.JsonElement;
import com.google.gson.JsonObject;

/**
Expand All @@ -22,22 +21,26 @@
*
* @author Hilbrand Bouwkamp - Initial contribution
*/
public class UnfiPortOverrideJsonElement {
public class UnfiPortOverrideJsonObject {

private static final String PORT_IDX = "port_idx";
private static final String PORT_CONF_ID = "port_conf_id";
private static final String POE_MODE = "poe_mode";

private final JsonObject jsonObject;

public UnfiPortOverrideJsonElement(final JsonElement element) {
this.jsonObject = element.getAsJsonObject();
public UnfiPortOverrideJsonObject(final JsonObject Object) {
this.jsonObject = Object.getAsJsonObject();
}

public JsonObject getJsonObject() {
return jsonObject;
}

public static boolean hasPortIdx(final JsonObject jsonObject) {
return jsonObject.has(PORT_IDX);
}

public int getPortIdx() {
return jsonObject.get(PORT_IDX).getAsInt();
}
Expand Down
6 changes: 3 additions & 3 deletions ...b.binding.unifi/src/main/java/org/openhab/binding/unifi/internal/api/dto/UniFiDevice.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import org.openhab.binding.unifi.internal.api.cache.UniFiControllerCache;
import org.openhab.binding.unifi.internal.api.util.UniFiTidyLowerCaseStringDeserializer;

import com.google.gson.JsonElement;
import com.google.gson.JsonObject;
import com.google.gson.annotations.JsonAdapter;
import com.google.gson.annotations.SerializedName;

Expand Down Expand Up @@ -44,7 +44,7 @@ public class UniFiDevice implements HasId {

private UniFiPortTable[] portTable;

private JsonElement[] portOverrides;
private JsonObject[] portOverrides;

public UniFiDevice(final UniFiControllerCache cache) {
this.cache = cache;
Expand Down Expand Up @@ -75,7 +75,7 @@ public UniFiPortTable[] getPortTable() {
return portTable;
}

public JsonElement[] getPortOverrides() {
public JsonObject[] getPortOverrides() {
return portOverrides;
}

Expand Down
8 changes: 4 additions & 4 deletions ...inding.unifi/src/main/java/org/openhab/binding/unifi/internal/api/dto/UniFiPortTuple.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

/**
* Tuple to store both the {@link UniFiPortTable}, which contains the all information related to the port,
* and the {@link UnfiPortOverrideJsonElement}, which contains the raw json data of the port override.
* and the {@link UnfiPortOverrideJsonObject}, which contains the raw json data of the port override.
*
* @author Hilbrand Bouwkamp - Initial contribution
*/
Expand All @@ -24,7 +24,7 @@ public class UniFiPortTuple {

private UniFiPortTable table;

private UnfiPortOverrideJsonElement jsonElement;
private UnfiPortOverrideJsonObject jsonElement;

public UniFiDevice getDevice() {
return device;
Expand All @@ -46,11 +46,11 @@ public void setTable(final UniFiPortTable table) {
this.table = table;
}

public UnfiPortOverrideJsonElement getJsonElement() {
public UnfiPortOverrideJsonObject getJsonElement() {
return jsonElement;
}

public void setJsonElement(final UnfiPortOverrideJsonElement jsonElement) {
public void setJsonElement(final UnfiPortOverrideJsonObject jsonElement) {
this.jsonElement = jsonElement;
}
}
Loading

0 comments on commit 820f8de

Please sign in to comment.