Skip to content

Commit d3c9204

Browse files
authored
[fmiweather] Fix compiler warnings and SAT issues (openhab#17621)
* Reduce number of warnings and SAT issues Signed-off-by: Jacob Laursen <[email protected]>
1 parent 1926847 commit d3c9204

24 files changed

+173
-231
lines changed

bundles/org.openhab.binding.fmiweather/src/main/java/org/openhab/binding/fmiweather/internal/AbstractWeatherHandler.java

+9-5
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import java.time.Instant;
1919
import java.time.ZoneId;
2020
import java.time.ZonedDateTime;
21+
import java.util.Objects;
2122
import java.util.Optional;
2223
import java.util.concurrent.ScheduledFuture;
2324
import java.util.concurrent.TimeUnit;
@@ -81,13 +82,11 @@ public AbstractWeatherHandler(Thing thing) {
8182
}
8283

8384
@Override
84-
@SuppressWarnings("PMD.CompareObjectsWithEquals")
8585
public void handleCommand(ChannelUID channelUID, Command command) {
8686
if (RefreshType.REFRESH == command) {
8787
ScheduledFuture<?> prevFuture = updateChannelsFutureRef.get();
88-
ScheduledFuture<?> newFuture = updateChannelsFutureRef
89-
.updateAndGet(fut -> fut == null || fut.isDone() ? submitUpdateChannelsThrottled() : fut);
90-
assert newFuture != null; // invariant
88+
ScheduledFuture<?> newFuture = Objects.requireNonNull(updateChannelsFutureRef
89+
.updateAndGet(fut -> fut == null || fut.isDone() ? submitUpdateChannelsThrottled() : fut));
9190
if (logger.isTraceEnabled()) {
9291
long delayRemainingMillis = newFuture.getDelay(TimeUnit.MILLISECONDS);
9392
if (delayRemainingMillis <= 0) {
@@ -97,14 +96,19 @@ public void handleCommand(ChannelUID channelUID, Command command) {
9796
delayRemainingMillis);
9897
}
9998
// Compare by reference to check if the future changed
100-
if (prevFuture == newFuture) {
99+
if (isSameFuture(prevFuture, newFuture)) {
101100
logger.trace("REFRESH received. Previous refresh ongoing, will wait for it to complete in {} ms",
102101
lastRefreshMillis + REFRESH_THROTTLE_MILLIS - System.currentTimeMillis());
103102
}
104103
}
105104
}
106105
}
107106

107+
@SuppressWarnings("PMD.CompareObjectsWithEquals")
108+
private boolean isSameFuture(@Nullable ScheduledFuture<?> future1, @Nullable ScheduledFuture<?> future2) {
109+
return future1 == future2;
110+
}
111+
108112
@Override
109113
public void initialize() {
110114
client = new Client();

bundles/org.openhab.binding.fmiweather/src/main/java/org/openhab/binding/fmiweather/internal/ForecastWeatherHandler.java

-2
Original file line numberDiff line numberDiff line change
@@ -232,7 +232,6 @@ private static int getTimeIndex(int hours) {
232232
return (int) (TimeUnit.HOURS.toMinutes(hours) / QUERY_RESOLUTION_MINUTES);
233233
}
234234

235-
@SuppressWarnings({ "unused", "null" })
236235
private static @Nullable String getDataField(ChannelUID channelUID) {
237236
Entry<String, @Nullable Unit<?>> entry = CHANNEL_TO_FORECAST_FIELD_NAME_AND_UNIT
238237
.get(channelUID.getIdWithoutGroup());
@@ -242,7 +241,6 @@ private static int getTimeIndex(int hours) {
242241
return entry.getKey();
243242
}
244243

245-
@SuppressWarnings({ "unused", "null" })
246244
private static @Nullable Unit<?> getUnit(ChannelUID channelUID) {
247245
Entry<String, @Nullable Unit<?>> entry = CHANNEL_TO_FORECAST_FIELD_NAME_AND_UNIT
248246
.get(channelUID.getIdWithoutGroup());

bundles/org.openhab.binding.fmiweather/src/main/java/org/openhab/binding/fmiweather/internal/ObservationWeatherHandler.java

+11-6
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,17 @@ public class ObservationWeatherHandler extends AbstractWeatherHandler {
6262
private static final long OBSERVATION_LOOK_BACK_SECONDS = TimeUnit.MINUTES.toSeconds(30);
6363
private static final int STEP_MINUTES = 10;
6464
private static final int POLL_INTERVAL_SECONDS = 600;
65-
private static BigDecimal HUNDRED = BigDecimal.valueOf(100);
66-
private static BigDecimal NA_CLOUD_MAX = BigDecimal.valueOf(8); // API value when having full clouds (overcast)
67-
private static BigDecimal NA_CLOUD_COVERAGE = BigDecimal.valueOf(9); // API value when cloud coverage could not be
68-
// determined.
65+
private static final BigDecimal HUNDRED = BigDecimal.valueOf(100);
66+
67+
/**
68+
* API value when having full clouds (overcast)
69+
*/
70+
private static final BigDecimal NA_CLOUD_MAX = BigDecimal.valueOf(8);
71+
72+
/**
73+
* API value when cloud coverage could not be determined.
74+
*/
75+
private static final BigDecimal NA_CLOUD_COVERAGE = BigDecimal.valueOf(9);
6976

7077
public static final Unit<Length> MILLIMETRE = MetricPrefix.MILLI(METRE);
7178
public static final Unit<Length> CENTIMETRE = MetricPrefix.CENTI(METRE);
@@ -175,7 +182,6 @@ protected void updateChannels() {
175182
}
176183
}
177184

178-
@SuppressWarnings({ "null", "unused" })
179185
private static @Nullable String getDataField(ChannelUID channelUID) {
180186
Entry<String, @Nullable Unit<?>> entry = CHANNEL_TO_OBSERVATION_FIELD_NAME_AND_UNIT
181187
.get(channelUID.getIdWithoutGroup());
@@ -185,7 +191,6 @@ protected void updateChannels() {
185191
return entry.getKey();
186192
}
187193

188-
@SuppressWarnings({ "null", "unused" })
189194
private static @Nullable Unit<?> getUnit(ChannelUID channelUID) {
190195
Entry<String, @Nullable Unit<?>> entry = CHANNEL_TO_OBSERVATION_FIELD_NAME_AND_UNIT
191196
.get(channelUID.getIdWithoutGroup());

bundles/org.openhab.binding.fmiweather/src/main/java/org/openhab/binding/fmiweather/internal/client/Client.java

+15-18
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
import java.io.IOException;
1616
import java.io.StringReader;
1717
import java.math.BigDecimal;
18-
import java.util.HashMap;
1918
import java.util.HashSet;
2019
import java.util.Iterator;
2120
import java.util.Map;
@@ -67,22 +66,20 @@
6766
@NonNullByDefault
6867
public class Client {
6968

70-
private final Logger logger = LoggerFactory.getLogger(Client.class);
69+
private static final String WEATHER_STATIONS_URL = "https://opendata.fmi.fi/wfs/fin?service=WFS&version=2.0.0&request=GetFeature&storedquery_id=fmi::ef::stations&networkid=121&";
7170

72-
public static final String WEATHER_STATIONS_URL = "https://opendata.fmi.fi/wfs/fin?service=WFS&version=2.0.0&request=GetFeature&storedquery_id=fmi::ef::stations&networkid=121&";
71+
private static final Map<String, String> NAMESPACES = Map.of( //
72+
"target", "http://xml.fmi.fi/namespace/om/atmosphericfeatures/1.1", //
73+
"gml", "http://www.opengis.net/gml/3.2", //
74+
"xlink", "http://www.w3.org/1999/xlink", //
75+
"ows", "http://www.opengis.net/ows/1.1", //
76+
"gmlcov", "http://www.opengis.net/gmlcov/1.0", //
77+
"swe", "http://www.opengis.net/swe/2.0", //
78+
"wfs", "http://www.opengis.net/wfs/2.0", //
79+
"ef", "http://inspire.ec.europa.eu/schemas/ef/4.0");
7380

74-
private static final Map<String, String> NAMESPACES = new HashMap<>();
75-
static {
76-
NAMESPACES.put("target", "http://xml.fmi.fi/namespace/om/atmosphericfeatures/1.1");
77-
NAMESPACES.put("gml", "http://www.opengis.net/gml/3.2");
78-
NAMESPACES.put("xlink", "http://www.w3.org/1999/xlink");
79-
NAMESPACES.put("ows", "http://www.opengis.net/ows/1.1");
80-
NAMESPACES.put("gmlcov", "http://www.opengis.net/gmlcov/1.0");
81-
NAMESPACES.put("swe", "http://www.opengis.net/swe/2.0");
81+
private final Logger logger = LoggerFactory.getLogger(Client.class);
8282

83-
NAMESPACES.put("wfs", "http://www.opengis.net/wfs/2.0");
84-
NAMESPACES.put("ef", "http://inspire.ec.europa.eu/schemas/ef/4.0");
85-
}
8683
private static final NamespaceContext NAMESPACE_CONTEXT = new NamespaceContext() {
8784
@Override
8885
public @Nullable String getNamespaceURI(@Nullable String prefix) {
@@ -91,12 +88,12 @@ public class Client {
9188

9289
@SuppressWarnings("rawtypes")
9390
@Override
94-
public @Nullable Iterator getPrefixes(@Nullable String val) {
91+
public @Nullable Iterator getPrefixes(@Nullable String namespaceURI) {
9592
return null;
9693
}
9794

9895
@Override
99-
public @Nullable String getPrefix(@Nullable String uri) {
96+
public @Nullable String getPrefix(@Nullable String namespaceURI) {
10097
return null;
10198
}
10299
};
@@ -173,7 +170,7 @@ public Set<Location> queryWeatherStations(int timeoutMillis)
173170
}
174171
}
175172

176-
private Set<Location> parseStations(String response) throws FMIExceptionReportException,
173+
protected Set<Location> parseStations(String response) throws FMIExceptionReportException,
177174
FMIUnexpectedResponseException, SAXException, IOException, XPathExpressionException {
178175
Document document = documentBuilder.parse(new InputSource(new StringReader(response)));
179176

@@ -218,7 +215,7 @@ private Set<Location> parseStations(String response) throws FMIExceptionReportEx
218215
* Parse FMI multipointcoverage formatted xml response
219216
*
220217
*/
221-
private FMIResponse parseMultiPointCoverageXml(String response) throws FMIUnexpectedResponseException,
218+
protected FMIResponse parseMultiPointCoverageXml(String response) throws FMIUnexpectedResponseException,
222219
FMIExceptionReportException, SAXException, IOException, XPathExpressionException {
223220
Document document = documentBuilder.parse(new InputSource(new StringReader(response)));
224221

bundles/org.openhab.binding.fmiweather/src/main/java/org/openhab/binding/fmiweather/internal/client/FMIResponse.java

+12-6
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import java.util.List;
1919
import java.util.Map;
2020
import java.util.Map.Entry;
21+
import java.util.Objects;
2122
import java.util.Optional;
2223
import java.util.Set;
2324

@@ -54,11 +55,15 @@ public Builder() {
5455

5556
public Builder appendLocationData(Location location, @Nullable Integer capacityHintForValues, String parameter,
5657
long epochSecond, @Nullable BigDecimal val) {
57-
timestampsByLocationByParameter.computeIfAbsent(location, k -> new HashMap<>()).computeIfAbsent(parameter,
58-
k -> capacityHintForValues == null ? new ArrayList<>() : new ArrayList<>(capacityHintForValues))
58+
Objects.requireNonNull(Objects
59+
.requireNonNull(timestampsByLocationByParameter.computeIfAbsent(location, k -> new HashMap<>()))
60+
.computeIfAbsent(parameter, k -> capacityHintForValues == null ? new ArrayList<>()
61+
: new ArrayList<>(capacityHintForValues)))
5962
.add(epochSecond);
60-
valuesByLocationByParameter.computeIfAbsent(location, k -> new HashMap<>()).computeIfAbsent(parameter,
61-
k -> capacityHintForValues == null ? new ArrayList<>() : new ArrayList<>(capacityHintForValues))
63+
Objects.requireNonNull(
64+
Objects.requireNonNull(valuesByLocationByParameter.computeIfAbsent(location, k -> new HashMap<>()))
65+
.computeIfAbsent(parameter, k -> capacityHintForValues == null ? new ArrayList<>()
66+
: new ArrayList<>(capacityHintForValues)))
6267
.add(val);
6368
return this;
6469
}
@@ -85,10 +90,11 @@ private void collectValuesForParameter(Map<Location, Map<String, Data>> out, Loc
8590
Entry<String, List<Long>> parameterEntry) {
8691
String parameter = parameterEntry.getKey();
8792
long[] timestamps = parameterEntry.getValue().stream().mapToLong(Long::longValue).toArray();
88-
BigDecimal[] values = valuesByLocationByParameter.get(location).get(parameter)
93+
BigDecimal[] values = Objects
94+
.requireNonNull(Objects.requireNonNull(valuesByLocationByParameter.get(location)).get(parameter))
8995
.toArray(new @Nullable BigDecimal[0]);
9096
Data dataValues = new Data(timestamps, values);
91-
out.get(location).put(parameter, dataValues);
97+
Objects.requireNonNull(out.get(location)).put(parameter, dataValues);
9298
}
9399
}
94100

bundles/org.openhab.binding.fmiweather/src/main/java/org/openhab/binding/fmiweather/internal/client/Request.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,8 @@ public class Request {
3737
public final String storedQueryId;
3838
public final String[] parameters;
3939

40-
private static ZoneId UTC = ZoneId.of("Z");
41-
private static DateTimeFormatter FORMATTER = DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss'Z'");
40+
private static final ZoneId UTC = ZoneId.of("Z");
41+
private static final DateTimeFormatter FORMATTER = DateTimeFormatter.ofPattern("yyyy-MM-dd'T'HH:mm:ss'Z'");
4242

4343
public Request(String storedQueryId, QueryParameter location, long startEpoch, long endEpoch,
4444
long timestepMinutes) {

bundles/org.openhab.binding.fmiweather/src/test/java/org/openhab/binding/fmiweather/AbstractFMIResponseParsingTest.java bundles/org.openhab.binding.fmiweather/src/test/java/org/openhab/binding/fmiweather/internal/AbstractFMIResponseParsingTest.java

+25-74
Original file line numberDiff line numberDiff line change
@@ -10,23 +10,16 @@
1010
*
1111
* SPDX-License-Identifier: EPL-2.0
1212
*/
13-
package org.openhab.binding.fmiweather;
13+
package org.openhab.binding.fmiweather.internal;
1414

15-
import static org.junit.jupiter.api.Assertions.fail;
16-
17-
import java.io.BufferedReader;
1815
import java.io.IOException;
19-
import java.lang.reflect.InvocationTargetException;
20-
import java.lang.reflect.Method;
21-
import java.net.URISyntaxException;
16+
import java.io.InputStream;
2217
import java.nio.charset.StandardCharsets;
23-
import java.nio.file.Files;
24-
import java.nio.file.Path;
25-
import java.nio.file.Paths;
2618
import java.util.HashSet;
27-
import java.util.Objects;
2819
import java.util.Set;
2920

21+
import javax.xml.xpath.XPathExpressionException;
22+
3023
import org.eclipse.jdt.annotation.NonNullByDefault;
3124
import org.eclipse.jdt.annotation.Nullable;
3225
import org.hamcrest.Description;
@@ -37,6 +30,9 @@
3730
import org.openhab.binding.fmiweather.internal.client.Data;
3831
import org.openhab.binding.fmiweather.internal.client.FMIResponse;
3932
import org.openhab.binding.fmiweather.internal.client.Location;
33+
import org.openhab.binding.fmiweather.internal.client.exception.FMIExceptionReportException;
34+
import org.openhab.binding.fmiweather.internal.client.exception.FMIUnexpectedResponseException;
35+
import org.xml.sax.SAXException;
4036

4137
/**
4238
* Base class for response parsing tests
@@ -47,41 +43,23 @@
4743
public class AbstractFMIResponseParsingTest {
4844

4945
@NonNullByDefault({})
50-
protected Client client;
46+
protected ClientExposed client;
5147

5248
@BeforeEach
5349
public void setUpClient() {
54-
client = new Client();
55-
}
56-
57-
protected Path getTestResource(String filename) {
58-
try {
59-
return Paths.get(getClass().getResource(filename).toURI());
60-
} catch (URISyntaxException e) {
61-
fail(e.getMessage());
62-
// Make the compiler happy by throwing here, fails already above
63-
throw new IllegalStateException();
64-
}
50+
client = new ClientExposed();
6551
}
6652

67-
protected String readTestResourceUtf8(String filename) {
68-
return readTestResourceUtf8(getTestResource(filename));
69-
}
70-
71-
protected String readTestResourceUtf8(Path path) {
72-
try {
73-
BufferedReader reader = Files.newBufferedReader(path, StandardCharsets.UTF_8);
74-
StringBuilder content = new StringBuilder();
75-
char[] buffer = new char[1024];
76-
int read = -1;
77-
while ((read = reader.read(buffer)) != -1) {
78-
content.append(buffer, 0, read);
53+
protected String readTestResourceUtf8(String filename) throws IOException {
54+
try (InputStream inputStream = AbstractFMIResponseParsingTest.class.getResourceAsStream(filename)) {
55+
if (inputStream == null) {
56+
throw new IOException("Input stream is null");
7957
}
80-
return content.toString();
81-
} catch (IOException e) {
82-
fail(e.getMessage());
83-
// Make the compiler happy by throwing here, fails already above
84-
throw new IllegalStateException();
58+
byte[] bytes = inputStream.readAllBytes();
59+
if (bytes == null) {
60+
throw new IOException("Resulting byte-array empty");
61+
}
62+
return new String(bytes, StandardCharsets.UTF_8);
8563
}
8664
}
8765

@@ -143,42 +121,15 @@ protected void describeMismatchSafely(Data dataValues, @Nullable Description mis
143121
};
144122
}
145123

146-
/**
147-
*
148-
* @param content
149-
* @return
150-
* @throws Throwable exception raised by parseMultiPointCoverageXml
151-
* @throws AssertionError exception raised when parseMultiPointCoverageXml method signature does not match excepted
152-
* (test & implementation is out-of-sync)
153-
*/
154-
protected FMIResponse parseMultiPointCoverageXml(String content) throws Throwable {
155-
try {
156-
Method parseMethod = Client.class.getDeclaredMethod("parseMultiPointCoverageXml", String.class);
157-
parseMethod.setAccessible(true);
158-
return Objects.requireNonNull((FMIResponse) parseMethod.invoke(client, content));
159-
} catch (InvocationTargetException e) {
160-
throw e.getTargetException();
161-
} catch (Exception e) {
162-
fail(String.format("Unexpected reflection error (code changed?) %s: %s", e.getClass().getName(),
163-
e.getMessage()));
164-
// Make the compiler happy by throwing here, fails already above
165-
throw new IllegalStateException();
124+
protected class ClientExposed extends Client {
125+
public FMIResponse parseMultiPointCoverageXml(String response) throws FMIUnexpectedResponseException,
126+
FMIExceptionReportException, SAXException, IOException, XPathExpressionException {
127+
return super.parseMultiPointCoverageXml(response);
166128
}
167-
}
168129

169-
@SuppressWarnings("unchecked")
170-
protected Set<Location> parseStations(String content) {
171-
try {
172-
Method parseMethod = Objects.requireNonNull(Client.class.getDeclaredMethod("parseStations", String.class));
173-
parseMethod.setAccessible(true);
174-
return Objects.requireNonNull((Set<Location>) parseMethod.invoke(client, content));
175-
} catch (InvocationTargetException e) {
176-
throw new RuntimeException(e.getTargetException());
177-
} catch (Exception e) {
178-
fail(String.format("Unexpected reflection error (code changed?) %s: %s", e.getClass().getName(),
179-
e.getMessage()));
180-
// Make the compiler happy by throwing here, fails already above
181-
throw new IllegalStateException();
130+
public Set<Location> parseStations(String response) throws FMIExceptionReportException,
131+
FMIUnexpectedResponseException, SAXException, IOException, XPathExpressionException {
132+
return super.parseStations(response);
182133
}
183134
}
184135
}

0 commit comments

Comments
 (0)