fix: resolve 7 latent issues and document 18 telemetry bugs

base-marshalling/src/main/java/build/base/marshalling/ConcurrentSchemaFactory.java

@@ -227,6 +227,20 @@ public Type type() {
             marshalDependencies.stream(),
             marshalParameters.stream()));
 
+        // -----------
+        // fail fast if @Unmarshal is mistakenly placed on a method (only constructors are supported)
+        final var unmarshalMethods = Stream.of(marshallableClass.getDeclaredMethods())
+            .filter(method -> method.isAnnotationPresent(Unmarshal.class))
+            .toList();
+
+        if (!unmarshalMethods.isEmpty()) {
+            this.unmarshallableClasses.add(marshallableClass);
+            throw new IllegalArgumentException("The class " + marshallableClass.getName()
+                + " has @Unmarshal on method(s) " + unmarshalMethods.stream()
+                    .map(Method::getName).toList()
+                + " — @Unmarshal is only supported on constructors");
+        }
+
         // -----------
         // discover the @Unmarshal annotated Constructors (there may be many)
         final var unmarshallConstructors = Stream.of(marshallableClass.getDeclaredConstructors())
@@ -323,7 +337,7 @@ public Type type() {
 
     @Override
     @SuppressWarnings("unchecked")
-    public <T> Optional<Schema<T>> getMashallingSchema(final Class<T> marshallableClass) {
+    public <T> Optional<Schema<T>> getMarshallingSchema(final Class<T> marshallableClass) {
 
         if (marshallableClass == null) {
             return Optional.empty();
@@ -336,7 +350,7 @@ public <T> Optional<Schema<T>> getMashallingSchema(final Class<T> marshallableCl
 
     @Override
     @SuppressWarnings("unchecked")
-    public <T> Stream<Schema<T>> getUnmashallingSchemas(final Class<T> marshallableClass) {
+    public <T> Stream<Schema<T>> getUnmarshallingSchemas(final Class<T> marshallableClass) {
 
         if (marshallableClass == null) {
             return Stream.empty();

base-marshalling/src/main/java/build/base/marshalling/SchemaFactory.java

@@ -69,7 +69,7 @@ default <T> void register(final Class<T> marshallableClass) {
      * @param <T>               the type of <i>marshallable</i> {@link Class}
      * @return the {@link Optional} {@link Schema} or {@link Optional#empty()} if no such registration exists
      */
-    <T> Optional<Schema<T>> getMashallingSchema(Class<T> marshallableClass);
+    <T> Optional<Schema<T>> getMarshallingSchema(Class<T> marshallableClass);
 
     /**
      * Obtains the {@link Schema} that can be used to unmarshal the specified {@link Class}.
@@ -78,7 +78,7 @@ default <T> void register(final Class<T> marshallableClass) {
      * @param <T>               the type of <i>marshallable</i> {@link Class}
      * @return the {@link Stream} of {@link Schema} for unmarshalling the specified {@link Class}
      */
-    <T> Stream<Schema<T>> getUnmashallingSchemas(Class<T> marshallableClass);
+    <T> Stream<Schema<T>> getUnmarshallingSchemas(Class<T> marshallableClass);
 
     /**
      * Determine if the specified {@link Class} is marshallable.

base-marshalling/src/main/java/build/base/marshalling/Unmarshal.java

@@ -33,7 +33,7 @@
  * @see Marshal
  * @since Nov-2024
  */
-@Target({ ElementType.METHOD, ElementType.CONSTRUCTOR })
+@Target(ElementType.CONSTRUCTOR)
 @Retention(RetentionPolicy.RUNTIME)
 public @interface Unmarshal {
 

base-marshalling/src/test/java/build/base/marshalling/MarshallingTests.java

@@ -44,7 +44,7 @@ void shouldStaticallyRegisterMarshallableClass() {
         final var schemaFactory = Marshalling.globalSchemaFactory();
 
         final var marshallingSchema = schemaFactory
-            .getMashallingSchema(StaticallyRegisteredPoint.class)
+            .getMarshallingSchema(StaticallyRegisteredPoint.class)
             .orElseThrow();
 
         assertThat(schemaFactory.isMarshallable(StaticallyRegisteredPoint.class))
@@ -69,10 +69,10 @@ void shouldStaticallyRegisterMarshallableClass() {
             .map(Parameter::type))
             .allMatch(Integer.class::equals);
 
-        assertThat(schemaFactory.getUnmashallingSchemas(StaticallyRegisteredPoint.class))
+        assertThat(schemaFactory.getUnmarshallingSchemas(StaticallyRegisteredPoint.class))
             .hasSize(1);
 
-        final var unmashallingSchema = schemaFactory.getUnmashallingSchemas(StaticallyRegisteredPoint.class)
+        final var unmashallingSchema = schemaFactory.getUnmarshallingSchemas(StaticallyRegisteredPoint.class)
             .findFirst()
             .orElseThrow();
 
@@ -106,7 +106,7 @@ void shouldDynamicallyRegisterMarshallableClass() {
         schemaFactory.register(Point.class);
 
         final var marshallingSchema = schemaFactory
-            .getMashallingSchema(Point.class)
+            .getMarshallingSchema(Point.class)
             .orElseThrow();
 
         assertThat(marshallingSchema)
@@ -128,10 +128,10 @@ void shouldDynamicallyRegisterMarshallableClass() {
             .map(Parameter::type))
             .allMatch(Integer.class::equals);
 
-        assertThat(schemaFactory.getUnmashallingSchemas(Point.class))
+        assertThat(schemaFactory.getUnmarshallingSchemas(Point.class))
             .hasSize(1);
 
-        final var unmashallingSchema = schemaFactory.getUnmashallingSchemas(Point.class)
+        final var unmashallingSchema = schemaFactory.getUnmarshallingSchemas(Point.class)
             .findFirst()
             .orElseThrow();
 
@@ -160,7 +160,7 @@ void shouldDynamicallyRegisterMarshallableClass() {
     @Test
     void shouldNotObtainSchemaForUnmarshallableClass() {
         assertThat(Marshalling.globalSchemaFactory()
-            .getMashallingSchema(String.class))
+            .getMarshallingSchema(String.class))
             .isEmpty();
     }
 

base-retryable/src/main/java/build/base/retryable/option/RetryFrequency.java

@@ -107,12 +107,12 @@ public RetryFrequency within(final Duration floor, final Duration ceiling) {
         Objects.requireNonNull(floor, "floor can't be null");
         Objects.requireNonNull(ceiling, "ceiling can't be null");
 
-        if (!floor.isNegative()) {
-            throw new IllegalArgumentException("floor must be positive");
+        if (floor.isNegative()) {
+            throw new IllegalArgumentException("floor must not be negative");
         }
 
-        if (ceiling.compareTo(floor) >= 0) {
-            throw new IllegalArgumentException("ceiling must be greater than floor");
+        if (ceiling.compareTo(floor) < 0) {
+            throw new IllegalArgumentException("ceiling must be greater than or equal to floor");
         }
 
         final RetryFrequency frequency = this;
@@ -163,7 +163,7 @@ public String toString() {
      * @return the new frequency, or {@code this} if limit == {@link Long#MAX_VALUE}.
      */
     public RetryFrequency maxRetriesOf(final long limit) {
-        if (limit <= 0) {
+        if (limit < 0) {
             throw new IllegalArgumentException("limit can't be negative");
         }
 

base-retryable/src/test/java/build/base/retryable/RetryFrequencyTests.java

@@ -0,0 +1,80 @@
+package build.base.retryable;
+
+import build.base.retryable.option.RetryFrequency;
+import org.junit.jupiter.api.Test;
+
+import java.time.Duration;
+import java.util.Iterator;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatNoException;
+
+/**
+ * Tests for {@link RetryFrequency}.
+ *
+ * @author reed.vonredwitz
+ */
+class RetryFrequencyTests {
+
+    /**
+     * Ensure {@link RetryFrequency#within} accepts a valid non-negative floor.
+     */
+    @Test
+    void withinShouldAcceptNonNegativeFloor() {
+        assertThatNoException().isThrownBy(() ->
+            RetryFrequency.every(Duration.ofSeconds(10))
+                .within(Duration.ZERO, Duration.ofSeconds(10)));
+    }
+
+    /**
+     * Ensure {@link RetryFrequency#within} accepts a valid ceiling greater than floor.
+     */
+    @Test
+    void withinShouldAcceptCeilingGreaterThanFloor() {
+        assertThatNoException().isThrownBy(() ->
+            RetryFrequency.every(Duration.ofSeconds(10))
+                .within(Duration.ofSeconds(1), Duration.ofSeconds(5)));
+    }
+
+    /**
+     * Ensure {@link RetryFrequency#within} clamps values above the ceiling down to the ceiling.
+     */
+    @Test
+    void withinShouldClampValuesToCeiling() {
+        final var ceiling = Duration.ofSeconds(3);
+        final Iterator<Duration> iter = RetryFrequency.every(Duration.ofSeconds(10))
+            .within(Duration.ZERO, ceiling)
+            .iterator();
+
+        assertThat(iter.next()).isEqualTo(ceiling);
+    }
+
+    /**
+     * Ensure {@link RetryFrequency#within} clamps values below the floor up to the floor.
+     */
+    @Test
+    void withinShouldClampValuesToFloor() {
+        final var floor = Duration.ofSeconds(5);
+        final Iterator<Duration> iter = RetryFrequency.every(Duration.ofSeconds(1))
+            .within(floor, Duration.ofSeconds(10))
+            .iterator();
+
+        assertThat(iter.next()).isEqualTo(floor);
+    }
+
+    /**
+     * Ensure {@link RetryFrequency#never} does not throw on construction.
+     */
+    @Test
+    void neverShouldNotThrowOnConstruction() {
+        assertThatNoException().isThrownBy(RetryFrequency::never);
+    }
+
+    /**
+     * Ensure {@link RetryFrequency#never} produces an iterator with no elements.
+     */
+    @Test
+    void neverShouldProduceNoElements() {
+        assertThat(RetryFrequency.never().iterator().hasNext()).isFalse();
+    }
+}

base-table/src/main/java/build/base/table/Table.java

@@ -171,7 +171,7 @@ public Table addRow(final String... cells) {
      * @return the {@link Row} at the specified index, or <code>null</code>
      */
     public Row getRow(final int index) {
-        return index < 0 || index > this.rows.size() ? null : this.rows.get(index);
+        return index < 0 || index >= this.rows.size() ? null : this.rows.get(index);
     }
 
     @Override

base-table/src/test/java/build/base/table/TableTests.java

@@ -151,6 +151,16 @@ void shouldNestTables() {
             .isEqualTo(expected);
     }
 
+    /**
+     * Ensure {@link Table#getRow} returns null for an out-of-bounds index equal to the row count.
+     */
+    @Test
+    void getRowShouldReturnNullForIndexEqualToSize() {
+        final var table = Table.of(Row.of("only"));
+
+        assertThat(table.getRow(1)).isNull();
+    }
+
     /**
      * Ensure irregularly shaped {@link Table}s, with different numbers of cells per row, can be output.
      */

base-telemetry/src/main/java/build/base/telemetry/Progress.java

@@ -54,7 +54,7 @@ public interface Progress
      *
      * @return the percentage complete
      */
-    int percentage();
+    double percentage();
 
     /**
      * Obtains the {@link NamedUnit} of measure for this {@link Progress}.
@@ -114,8 +114,8 @@ public int maximum() {
             }
 
             @Override
-            public int percentage() {
-                return (int) ((double) current() / maximum() * 100.0);
+            public double percentage() {
+                return maximum() == 0 ? 0.0 : (double) current() / maximum() * 100.0;
             }
 
             @Override

base-telemetry/src/main/java/build/base/telemetry/TextualRange.java

@@ -59,7 +59,7 @@ static TextualRange create(final TextualPosition start,
                                final TextualPosition end) {
 
         Objects.requireNonNull(start, "The start TextualPosition must not be null");
-        Objects.requireNonNull(start, "The end TextualPosition must not be null");
+        Objects.requireNonNull(end, "The end TextualPosition must not be null");
 
         return new TextualRange() {
 

base-telemetry/src/test/java/build/base/telemetry/TextualRangeTests.java

@@ -0,0 +1,27 @@
+package build.base.telemetry;
+
+import org.junit.jupiter.api.Test;
+
+import java.net.URI;
+
+import static org.assertj.core.api.Assertions.assertThatNullPointerException;
+
+/**
+ * Tests for {@link TextualRange}.
+ */
+class TextualRangeTests {
+
+    private static final URI DOC = URI.create("file:///test.txt");
+
+    /**
+     * Ensure {@link TextualRange#create} rejects a null {@code end} position.
+     */
+    @Test
+    void createShouldRejectNullEnd() {
+        final var start = TextualPosition.create(DOC, 1, 0);
+
+        assertThatNullPointerException()
+            .isThrownBy(() -> TextualRange.create(start, null))
+            .withMessageContaining("end");
+    }
+}

base-transport-json/src/main/java/build/base/transport/json/JsonTransport.java

@@ -352,7 +352,7 @@ public <T> Marshalled<T> read(final JsonParser parser,
         }
 
         // determine the Schemas, their Parameters and corresponding Out values that are candidates for unmarshalling
-        final var schemas = this.schemaFactory.getUnmashallingSchemas(typeClass)
+        final var schemas = this.schemaFactory.getUnmarshallingSchemas(typeClass)
             .map(schema -> Pair.of(
                 schema,
                 schema.parameters().stream()