Skip to content

Commit 610bb01

Browse files
bensynapseclaude
andcommitted
[livetennisapi] Address second review round (wborn)
- Pagination: MatchListResponse now parses meta; the client pages the live-match list forward on meta.has_more (capped at 5 pages) instead of treating the first page as a complete snapshot, and logs when it deliberately truncates. Falls back to the page-fill heuristic only when meta is absent. - Tournament retry: the transient retry runs on its own retryJob rather than scheduleInfoRefresh, which gated on infoJob — the currently executing job — and so could never schedule. dispose() cancels it. - Break point at 40-40 is now UNDEF: the REST API does not expose the scoring format, so 40-40 is undeterminable (break point under no-ad, deuce under advantage). Only the determinable cases are asserted. - Usage read decoupled from the live poll: live data is published first and a failing /usage call no longer discards it. - Disposal: handlers carry a disposed flag and no longer publish state or status after dispose(). - Player detail overlap: an in-progress guard stops the periodic, ONLINE-triggered and retry refreshes from running concurrently and wasting quota; the tournament fetch gets the same guard. - Defaults raised so a bridge plus one player fit the free tier: bridge 1800 s (48/day) + player 7200 s (24/day) = 72/day; README math fixed. - New player detailRefreshEnabled switch to disable the ranking and next-match refresh entirely, lowering 429 risk. - Tests added for the 40-40 UNDEF behaviour and for the paging loop. Signed-off-by: Ben <118375461+bensynapse@users.noreply.github.com> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 1f3c86e commit 610bb01

14 files changed

Lines changed: 341 additions & 55 deletions

File tree

bundles/org.openhab.binding.livetennisapi/README.md

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,14 +33,15 @@ Configure a `player` thing with a doubles team's id to track that team exactly a
3333
| Parameter | Type | Required | Default | Description |
3434
|-------------------|-----------|----------|---------|----------------------------------------------------------------------------------------------------------------------------------------------------|
3535
| `apiKey` | `text` | yes || API key for the Live Tennis API |
36-
| `refreshInterval` | `integer` | no | 900 | How often to poll the live match snapshot in seconds (min: 60). The default stays within the free tier's daily quota; lower values need a paid tier. |
36+
| `refreshInterval` | `integer` | no | 1800 | How often to poll the live match snapshot in seconds (min: 60). The default keeps the bridge plus one player thing within the free tier's daily quota; lower values need a paid tier. |
3737

3838
### `player` Thing
3939

4040
| Parameter | Type | Required | Default | Description |
4141
|-------------------------|-----------|----------|---------|----------------------------------------------------------------------------------------------------|
4242
| `playerId` | `integer` | yes || The player's or doubles team's id in the Live Tennis API; look it up with `GET /players?search=name` |
43-
| `detailRefreshInterval` | `integer` | no | 3600 | How often to refresh the next match and ranking in seconds (min: 300); two API requests per cycle |
43+
| `detailRefreshEnabled` | `boolean` | no | `true` | Whether to refresh the next match and ranking. Turn it off to track only live match state (pushed by the bridge at no extra cost) and spend no quota of this thing's own |
44+
| `detailRefreshInterval` | `integer` | no | 7200 | How often to refresh the next match and ranking in seconds when enabled (min: 300); two API requests per cycle |
4445

4546
### `tournament` Thing
4647

@@ -78,7 +79,7 @@ All `live` channels are `UNDEF` while the player has no match in progress.
7879
| `live#sets` | `String` | Sets won, e.g. `1-0` |
7980
| `live#points` | `String` | In-game points, e.g. `40-15` or `AD-40`; `UNDEF` when the feed states no points |
8081
| `live#serving` | `Switch` | `ON` while the tracked player is serving; `UNDEF` when no game is in progress |
81-
| `live#break-point` | `Switch` | `ON` while the current game stands at break point (receiver at `AD`, or receiver at `40` with the server at `0`/`15`/`30`); never `ON` in a tiebreak; `UNDEF` when the score state does not allow deriving it |
82+
| `live#break-point` | `Switch` | `ON` while the current game stands at break point (receiver at `AD`, or receiver at `40` with the server at `0`/`15`/`30`); never `ON` in a tiebreak; `UNDEF` when the score state does not allow deriving it, including at `40-40` where the API does not state whether advantage or no-advantage scoring is in play |
8283
| `live#tiebreak` | `Switch` | `ON` while a tiebreak is being played |
8384
| `next-match#opponent` | `String` | Opponent in the next scheduled match |
8485
| `next-match#start-time` | `DateTime` | Scheduled start of the next match; `UNDEF` until the order of play assigns a time |
@@ -110,20 +111,22 @@ Match channels show the tournament's first listed live match.
110111
## Request Budget and the Free Tier
111112

112113
The free tier allows 100 requests per day.
113-
The bridge makes one counted request per refresh cycle (the usage read is quota-exempt), shared by all player and tournament things, so at the default 900 s interval the bridge uses 96 requests per day.
114-
Each player thing additionally makes two requests per detail refresh cycle — 48 per day at the default 3600 s interval.
114+
The bridge makes one counted request per refresh cycle (the usage read is quota-exempt), shared by all player and tournament things, so at the default 1800 s interval the bridge uses 48 requests per day.
115+
Each player thing additionally makes two requests per detail refresh cycle — 24 per day at the default 7200 s interval; setting `detailRefreshEnabled=false` drops those to zero and the thing then tracks only live match state.
115116

116-
A free key therefore fits the bridge alone at the defaults, or the bridge plus one player thing with slower settings (for example `refreshInterval=1800` and `detailRefreshInterval=7200`, together 72 requests per day).
117+
At the defaults a free key therefore fits the bridge plus one player thing (48 + 24 = 72 requests per day, within the 100 per day allowance).
117118
For several player things or faster live updates, a paid tier with a higher daily quota is required.
118119
The API answers requests over quota with HTTP 429; the bridge then goes `OFFLINE` with a communication error until a later poll succeeds, and the child things retry their own detail requests with a short backoff.
119120

121+
The live match list is paginated. The bridge reads the `meta.has_more` flag and pages forward until the snapshot is complete, so more than 200 concurrent live matches are not silently dropped; in practice the whole live board is well under one page, so this remains a single request per cycle.
122+
120123
## Full Example
121124

122125
### `livetennisapi.things`
123126

124127
```java
125-
Bridge livetennisapi:account:myaccount "Live Tennis API" [ apiKey="XXXX", refreshInterval=900 ] {
126-
Thing player alcaraz "Carlos Alcaraz" [ playerId=1234, detailRefreshInterval=3600 ]
128+
Bridge livetennisapi:account:myaccount "Live Tennis API" [ apiKey="XXXX", refreshInterval=1800 ] {
129+
Thing player alcaraz "Carlos Alcaraz" [ playerId=1234, detailRefreshInterval=7200 ]
127130
Thing tournament cincinnati "Cincinnati Open" [ tournamentId="atp-cincinnati" ]
128131
}
129132
```

bundles/org.openhab.binding.livetennisapi/src/main/java/org/openhab/binding/livetennisapi/internal/MatchStateMapper.java

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -109,9 +109,14 @@ private MatchStateMapper() {
109109
}
110110

111111
/**
112-
* Derives whether the current game stands at break point: the receiver holds AD, or the receiver holds 40 while
113-
* the server holds 0, 15 or 30. There are no break points in a tiebreak. Returns null (unknown) when the server
114-
* or the points are not stated.
112+
* Derives whether the current game stands at break point, only in the cases the data determines unambiguously:
113+
* the receiver holds AD, or the receiver holds 40 while the server holds 0, 15 or 30. There are no break points in
114+
* a tiebreak.
115+
*
116+
* Returns null (unknown) when the server or points are not stated, and — importantly — at 40-40: the API does not
117+
* expose the scoring format, so 40-40 is a break point under no-advantage scoring (a deciding point) but merely
118+
* deuce under advantage scoring. That is genuinely undeterminable from the data, so it is reported as unknown
119+
* rather than asserted either way.
115120
*/
116121
public static @Nullable Boolean isBreakPoint(@Nullable Score score) {
117122
if (score == null) {
@@ -130,6 +135,10 @@ private MatchStateMapper() {
130135
if (serverPoints == null || receiverPoints == null) {
131136
return null;
132137
}
138+
if ("40".equals(serverPoints) && "40".equals(receiverPoints)) {
139+
// 40-40: no-advantage vs advantage scoring diverge here and the API does not state which is in use.
140+
return null;
141+
}
133142
return "AD".equals(receiverPoints) || ("40".equals(receiverPoints)
134143
&& ("0".equals(serverPoints) || "15".equals(serverPoints) || "30".equals(serverPoints)));
135144
}

bundles/org.openhab.binding.livetennisapi/src/main/java/org/openhab/binding/livetennisapi/internal/api/LiveTennisApiClient.java

Lines changed: 47 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
*/
1313
package org.openhab.binding.livetennisapi.internal.api;
1414

15+
import java.util.ArrayList;
1516
import java.util.Comparator;
1617
import java.util.List;
1718
import java.util.concurrent.ExecutionException;
@@ -50,7 +51,10 @@ public class LiveTennisApiClient {
5051
private static final String API_BASE = "https://api.livetennisapi.com/api/public/v1";
5152
private static final String API_KEY_HEADER = "X-API-Key";
5253
private static final int TIMEOUT_S = 20;
53-
private static final int LIVE_MATCH_LIMIT = 200;
54+
private static final int LIVE_MATCH_PAGE_SIZE = 200;
55+
// Cap the live snapshot at a handful of pages: real concurrent live-match counts are far below this, so in
56+
// practice a single request is made, but a pathological page count can never fan out unbounded requests.
57+
private static final int MAX_LIVE_PAGES = 5;
5458
private static final int UPCOMING_MATCH_LIMIT = 10;
5559

5660
private final Logger logger = LoggerFactory.getLogger(LiveTennisApiClient.class);
@@ -73,18 +77,50 @@ public Usage getUsage() throws LiveTennisApiException {
7377
return get("/usage", Usage.class);
7478
}
7579

76-
/** Returns all matches currently in progress, with their latest score. */
80+
/**
81+
* Returns all matches currently in progress, with their latest score. The endpoint is paginated: this reads the
82+
* {@code meta.has_more} flag and pages forward until the snapshot is complete (or the page cap is reached), so
83+
* matches beyond the first page are not silently dropped.
84+
*/
7785
public List<Match> getLiveMatches() throws LiveTennisApiException {
78-
MatchListResponse response = get("/matches?status=live&limit=" + LIVE_MATCH_LIMIT, MatchListResponse.class);
79-
List<Match> matches = response.data;
80-
if (matches == null) {
81-
return List.of();
82-
}
83-
if (matches.size() >= LIVE_MATCH_LIMIT) {
84-
logger.warn("The live match snapshot hit the page limit of {}; any matches beyond it are not tracked",
85-
LIVE_MATCH_LIMIT);
86+
return collectLiveMatches(offset -> get(
87+
"/matches?status=live&limit=" + LIVE_MATCH_PAGE_SIZE + "&offset=" + offset, MatchListResponse.class));
88+
}
89+
90+
/**
91+
* A single page fetch of the live-match list, keyed by offset. Extracted so the paging loop can be exercised in a
92+
* unit test without a live HTTP endpoint.
93+
*/
94+
@FunctionalInterface
95+
interface LiveMatchPageFetcher {
96+
MatchListResponse fetch(int offset) throws LiveTennisApiException;
97+
}
98+
99+
/**
100+
* Pages forward through the live-match list, honouring {@code meta.has_more} rather than guessing from page size,
101+
* and stops at {@link #MAX_LIVE_PAGES}. When the API still reports more matches at the cap the snapshot is
102+
* truncated deliberately and that truncation is logged, never hidden.
103+
*/
104+
List<Match> collectLiveMatches(LiveMatchPageFetcher fetcher) throws LiveTennisApiException {
105+
List<Match> all = new ArrayList<>();
106+
for (int page = 0; page < MAX_LIVE_PAGES; page++) {
107+
MatchListResponse response = fetcher.fetch(all.size());
108+
List<Match> data = response.data;
109+
if (data == null || data.isEmpty()) {
110+
return all;
111+
}
112+
all.addAll(data);
113+
MatchListResponse.Meta meta = response.meta;
114+
// has_more is authoritative; only when it is absent do we fall back to the page-fill heuristic.
115+
boolean hasMore = meta != null ? Boolean.TRUE.equals(meta.hasMore) : data.size() >= LIVE_MATCH_PAGE_SIZE;
116+
if (!hasMore) {
117+
return all;
118+
}
86119
}
87-
return matches;
120+
logger.warn(
121+
"The live match snapshot still reports more results after {} pages of {}; matches beyond {} are not tracked this cycle",
122+
MAX_LIVE_PAGES, LIVE_MATCH_PAGE_SIZE, MAX_LIVE_PAGES * LIVE_MATCH_PAGE_SIZE);
123+
return all;
88124
}
89125

90126
/** Returns the given player's next upcoming match, or null if none is scheduled. */

bundles/org.openhab.binding.livetennisapi/src/main/java/org/openhab/binding/livetennisapi/internal/api/dto/MatchListResponse.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,4 +24,18 @@
2424
public class MatchListResponse {
2525

2626
public @Nullable List<Match> data;
27+
public @Nullable Meta meta;
28+
29+
/**
30+
* Pagination metadata. {@code has_more} is authoritative for whether further pages exist and must be read rather
31+
* than comparing {@code count} to the requested {@code limit}.
32+
*/
33+
public static class Meta {
34+
35+
public @Nullable Integer limit;
36+
public @Nullable Integer offset;
37+
public @Nullable Integer count;
38+
public @Nullable Integer total;
39+
public @Nullable Boolean hasMore;
40+
}
2741
}

bundles/org.openhab.binding.livetennisapi/src/main/java/org/openhab/binding/livetennisapi/internal/config/LiveTennisApiAccountConfiguration.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,5 +23,5 @@
2323
public class LiveTennisApiAccountConfiguration {
2424

2525
public String apiKey = "";
26-
public int refreshInterval = 900;
26+
public int refreshInterval = 1800;
2727
}

bundles/org.openhab.binding.livetennisapi/src/main/java/org/openhab/binding/livetennisapi/internal/config/LiveTennisApiPlayerConfiguration.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,5 +23,6 @@
2323
public class LiveTennisApiPlayerConfiguration {
2424

2525
public long playerId = -1;
26-
public int detailRefreshInterval = 3600;
26+
public int detailRefreshInterval = 7200;
27+
public boolean detailRefreshEnabled = true;
2728
}

bundles/org.openhab.binding.livetennisapi/src/main/java/org/openhab/binding/livetennisapi/internal/handler/LiveTennisApiAccountHandler.java

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ public class LiveTennisApiAccountHandler extends BaseBridgeHandler {
6161

6262
private @Nullable LiveTennisApiClient apiClient;
6363
private @Nullable ScheduledFuture<?> pollingJob;
64+
private volatile boolean disposed;
6465

6566
private List<Match> lastLiveMatches = List.of();
6667
private @Nullable Usage lastUsage;
@@ -85,6 +86,7 @@ public void initialize() {
8586
return;
8687
}
8788

89+
disposed = false;
8890
apiClient = new LiveTennisApiClient(httpClient, config.apiKey);
8991
updateStatus(ThingStatus.UNKNOWN);
9092

@@ -94,6 +96,7 @@ public void initialize() {
9496

9597
@Override
9698
public void dispose() {
99+
disposed = true;
97100
ScheduledFuture<?> job = pollingJob;
98101
if (job != null) {
99102
job.cancel(true);
@@ -150,22 +153,37 @@ private void poll() {
150153
if (client == null) {
151154
return;
152155
}
156+
List<Match> liveMatches;
153157
try {
154-
List<Match> liveMatches = client.getLiveMatches();
155-
Usage usage = client.getUsage();
156-
lastLiveMatches = liveMatches;
157-
lastUsage = usage;
158-
updateUsageChannels(usage);
159-
if (getThing().getStatus() != ThingStatus.ONLINE) {
160-
updateStatus(ThingStatus.ONLINE);
161-
}
162-
notifyChildHandlers(liveMatches);
158+
liveMatches = client.getLiveMatches();
163159
} catch (LiveTennisApiAuthenticationException e) {
164160
logger.debug("Authentication failed", e);
165161
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR, e.getMessage());
162+
return;
166163
} catch (LiveTennisApiException e) {
167164
logger.debug("Polling failed", e);
168165
updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, e.getMessage());
166+
return;
167+
}
168+
if (disposed) {
169+
return;
170+
}
171+
lastLiveMatches = liveMatches;
172+
if (getThing().getStatus() != ThingStatus.ONLINE) {
173+
updateStatus(ThingStatus.ONLINE);
174+
}
175+
notifyChildHandlers(liveMatches);
176+
177+
// The usage read is optional telemetry: a failure here must never discard the live data published above.
178+
try {
179+
Usage usage = client.getUsage();
180+
if (disposed) {
181+
return;
182+
}
183+
lastUsage = usage;
184+
updateUsageChannels(usage);
185+
} catch (LiveTennisApiException e) {
186+
logger.debug("Usage read failed; the live match data for this cycle was still published", e);
169187
}
170188
}
171189

0 commit comments

Comments
 (0)