Skip to content

Commit 95f0808

Browse files
SplatcrafterSplatcrafter
authored
Merge pull request #248 from aether-framework/bugfix/212-215-216-217-218-spring-boot-starter-fixes
Refactor status handling in `DataFixerEndpoint`
2 parents 88d878d + 0338289 commit 95f0808

6 files changed

Lines changed: 44 additions & 27 deletions

File tree

‎aether-datafixers-spring-boot-starter/src/main/java/de/splatgames/aether/datafixers/spring/actuator/DataFixerEndpoint.java‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,8 @@
9393
* {
9494
* "domain": "game",
9595
* "currentVersion": -1,
96-
* "status": "DOWN: Schema not initialized"
96+
* "status": "DOWN",
97+
* "error": "Schema not initialized"
9798
* }
9899
* }</pre>
99100
*
@@ -201,12 +202,14 @@ public DataFixersSummary summary() {
201202
try {
202203
domains.put(domain, new DomainSummary(
203204
fixer.currentVersion().getVersion(),
204-
"UP"
205+
"UP",
206+
null
205207
));
206208
} catch (final Exception e) {
207209
domains.put(domain, new DomainSummary(
208210
-1,
209-
"DOWN: " + e.getMessage()
211+
"DOWN",
212+
e.getMessage()
210213
));
211214
}
212215
}
@@ -249,13 +252,15 @@ public DomainDetails domainDetails(@Selector final String domain) {
249252
return new DomainDetails(
250253
domain,
251254
fixer.currentVersion().getVersion(),
252-
"UP"
255+
"UP",
256+
null
253257
);
254258
} catch (final Exception e) {
255259
return new DomainDetails(
256260
domain,
257261
-1,
258-
"DOWN: " + e.getMessage()
262+
"DOWN",
263+
e.getMessage()
259264
);
260265
}
261266
}
@@ -303,7 +308,7 @@ public record DataFixersSummary(Map<String, DomainSummary> domains) {
303308
* <p><b>Status Values</b></p>
304309
* <ul>
305310
* <li>{@code "UP"} - The DataFixer is operational and responding normally</li>
306-
* <li>{@code "DOWN: {message}"} - The DataFixer failed with the given error message</li>
311+
* <li>{@code "DOWN"} - The DataFixer failed; see {@code error} for details</li>
307312
* </ul>
308313
*
309314
* <p><b>Version Semantics</b></p>
@@ -313,11 +318,12 @@ public record DataFixersSummary(Map<String, DomainSummary> domains) {
313318
* </ul>
314319
*
315320
* @param currentVersion the current schema version of the domain, or -1 on error
316-
* @param status the operational status ("UP" or "DOWN: {error}")
321+
* @param status the operational status ("UP" or "DOWN")
322+
* @param error the error message if status is "DOWN", or {@code null} if healthy
317323
* @author Erik Pförtner
318324
* @since 0.4.0
319325
*/
320-
public record DomainSummary(int currentVersion, String status) {
326+
public record DomainSummary(int currentVersion, String status, @Nullable String error) {
321327
}
322328

323329
/**
@@ -339,14 +345,16 @@ public record DomainSummary(int currentVersion, String status) {
339345
*
340346
* @param domain the domain name (echoed from the request path)
341347
* @param currentVersion the current schema version of the domain, or -1 on error
342-
* @param status the operational status ("UP" or "DOWN: {error}")
348+
* @param status the operational status ("UP" or "DOWN")
349+
* @param error the error message if status is "DOWN", or {@code null} if healthy
343350
* @author Erik Pförtner
344351
* @since 0.4.0
345352
*/
346353
public record DomainDetails(
347354
String domain,
348355
int currentVersion,
349-
String status
356+
String status,
357+
@Nullable String error
350358
) {
351359
}
352360
}

‎aether-datafixers-spring-boot-starter/src/main/java/de/splatgames/aether/datafixers/spring/autoconfigure/DataFixerRegistry.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,12 +151,12 @@ public class DataFixerRegistry {
151151
public void register(@NotNull final String domain, @NotNull final AetherDataFixer fixer) {
152152
Preconditions.checkNotNull(domain, "domain must not be null");
153153
Preconditions.checkNotNull(fixer, "fixer must not be null");
154-
if (this.fixers.containsKey(domain)) {
154+
final AetherDataFixer existing = this.fixers.putIfAbsent(domain, fixer);
155+
if (existing != null) {
155156
throw new IllegalArgumentException(
156157
"DataFixer already registered for domain: " + domain
157158
);
158159
}
159-
this.fixers.put(domain, fixer);
160160
}
161161

162162
/**

‎aether-datafixers-spring-boot-starter/src/main/java/de/splatgames/aether/datafixers/spring/metrics/MigrationMetrics.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,10 @@ public void recordFailure(
320320
// Record timing (even for failures)
321321
getOrCreateTimer(domain).record(duration);
322322

323+
// Record version span (even for failures, to correlate failure rates with migration span)
324+
final int span = Math.abs(toVersion - fromVersion);
325+
getOrCreateVersionSpan(domain).record(span);
326+
323327
// Record failure count with error type
324328
getOrCreateFailureCounter(domain, error.getClass().getSimpleName()).increment();
325329
}

‎aether-datafixers-spring-boot-starter/src/main/java/de/splatgames/aether/datafixers/spring/service/DefaultMigrationService.java‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
import de.splatgames.aether.datafixers.core.AetherDataFixer;
3030
import de.splatgames.aether.datafixers.spring.autoconfigure.DataFixerRegistry;
3131
import de.splatgames.aether.datafixers.spring.metrics.MigrationMetrics;
32-
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
3332
import org.jetbrains.annotations.NotNull;
3433
import org.jetbrains.annotations.Nullable;
3534
import org.slf4j.Logger;
@@ -410,8 +409,12 @@ public MigrationResult execute() {
410409
final DataVersion from = this.fromVersion;
411410
final DataVersion to = this.toLatest ? fixer.currentVersion() : this.toVersion;
412411

413-
assert from != null : "fromVersion must be set";
414-
assert to != null : "toVersion must be set";
412+
if (from == null) {
413+
throw new IllegalStateException("fromVersion must be set");
414+
}
415+
if (to == null) {
416+
throw new IllegalStateException("toVersion must be set");
417+
}
415418

416419
LOG.debug("Starting migration from v{} to v{} in domain '{}'",
417420
from.getVersion(), to.getVersion(), this.domain);

‎aether-datafixers-spring-boot-starter/src/test/java/de/splatgames/aether/datafixers/spring/actuator/DataFixerEndpointTest.java‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,8 @@ void setsDownStatusForUnhealthyFixers() {
123123

124124
DataFixerEndpoint.DataFixersSummary summary = endpoint.summary();
125125

126-
assertThat(summary.domains().get("game").status()).startsWith("DOWN:");
126+
assertThat(summary.domains().get("game").status()).isEqualTo("DOWN");
127+
assertThat(summary.domains().get("game").error()).isEqualTo("Error");
127128
assertThat(summary.domains().get("game").currentVersion()).isEqualTo(-1);
128129
}
129130
}
@@ -167,7 +168,8 @@ void handlesFixerErrorsGracefully() {
167168
assertThat(details).isNotNull();
168169
assertThat(details.domain()).isEqualTo("game");
169170
assertThat(details.currentVersion()).isEqualTo(-1);
170-
assertThat(details.status()).contains("DOWN").contains("Init failed");
171+
assertThat(details.status()).isEqualTo("DOWN");
172+
assertThat(details.error()).isEqualTo("Init failed");
171173
}
172174
}
173175

@@ -179,7 +181,7 @@ class RecordDTOs {
179181
@DisplayName("DataFixersSummary record works correctly")
180182
void dataFixersSummaryRecordWorks() {
181183
var domains = java.util.Map.of(
182-
"game", new DataFixerEndpoint.DomainSummary(200, "UP")
184+
"game", new DataFixerEndpoint.DomainSummary(200, "UP", null)
183185
);
184186
var summary = new DataFixerEndpoint.DataFixersSummary(domains);
185187

@@ -190,20 +192,22 @@ void dataFixersSummaryRecordWorks() {
190192
@Test
191193
@DisplayName("DomainSummary record works correctly")
192194
void domainSummaryRecordWorks() {
193-
var summary = new DataFixerEndpoint.DomainSummary(200, "UP");
195+
var summary = new DataFixerEndpoint.DomainSummary(200, "UP", null);
194196

195197
assertThat(summary.currentVersion()).isEqualTo(200);
196198
assertThat(summary.status()).isEqualTo("UP");
199+
assertThat(summary.error()).isNull();
197200
}
198201

199202
@Test
200203
@DisplayName("DomainDetails record works correctly")
201204
void domainDetailsRecordWorks() {
202-
var details = new DataFixerEndpoint.DomainDetails("game", 200, "UP");
205+
var details = new DataFixerEndpoint.DomainDetails("game", 200, "UP", null);
203206

204207
assertThat(details.domain()).isEqualTo("game");
205208
assertThat(details.currentVersion()).isEqualTo(200);
206209
assertThat(details.status()).isEqualTo("UP");
210+
assertThat(details.error()).isNull();
207211
}
208212
}
209213
}

‎aether-datafixers-spring-boot-starter/src/test/java/de/splatgames/aether/datafixers/spring/metrics/MigrationMetricsTest.java‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -235,20 +235,18 @@ void recordsTimerOnFailure() {
235235
}
236236

237237
@Test
238-
@DisplayName("does not record version span on failure")
239-
void doesNotRecordVersionSpanOnFailure() {
238+
@DisplayName("records version span on failure")
239+
void recordsVersionSpanOnFailure() {
240240
metrics.recordFailure("game", 100, 200, Duration.ofMillis(50),
241241
new RuntimeException("Test"));
242242

243-
// Version span should not exist or be zero
244243
DistributionSummary summary = registry.find("aether.datafixers.migrations.version.span")
245244
.tag("domain", "game")
246245
.summary();
247246

248-
// Either null or count 0
249-
if (summary != null) {
250-
assertThat(summary.count()).isZero();
251-
}
247+
assertThat(summary).isNotNull();
248+
assertThat(summary.count()).isEqualTo(1);
249+
assertThat(summary.totalAmount()).isEqualTo(100.0);
252250
}
253251

254252
@Test

0 commit comments

Comments
 (0)