Skip to content

Add number type conversion validation to ensure proper casting between numeric types #1205

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mcruzdev wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add number type conversion validation to ensure proper casting between numeric types #1205

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2d1b65a
Add number type conversion validation to ensure proper casting betwee…
mcruzdev Mar 5, 2026
a3768ee
Apply fjtirado suggestions
mcruzdev Mar 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions experimental/model/src/main/java/io/serverlessworkflow/impl/model/func/JavaModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,29 @@ public Optional<Number> asNumber() {
return object instanceof Number value ? Optional.of(value) : Optional.empty();
}

@Override
@SuppressWarnings("unchecked")
protected <T> Optional<T> asNumber(Class<?> targetNumberClass) {
if (!(object instanceof Number num)) {
return Optional.empty();
}
if (targetNumberClass == Integer.class) {
return (Optional<T>) Optional.of(num.intValue());
} else if (targetNumberClass == Long.class) {
return (Optional<T>) Optional.of(num.longValue());
} else if (targetNumberClass == Double.class) {
return (Optional<T>) Optional.of(num.doubleValue());
} else if (targetNumberClass == Float.class) {
return (Optional<T>) Optional.of(num.floatValue());
} else if (targetNumberClass == Short.class) {
return (Optional<T>) Optional.of(num.shortValue());
} else if (targetNumberClass == Byte.class) {
return (Optional<T>) Optional.of(num.byteValue());
} else {
return (Optional<T>) Optional.of(num);
}
}
Comment on lines +67 to +88
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue as the Jackson implementation: for targets like BigDecimal.class / BigInteger.class, the else branch returns the original num without ensuring it's an instance of targetNumberClass, which can surface as a ClassCastException for model.as(BigDecimal.class). Consider explicitly converting for common types (e.g., BigDecimal, BigInteger) and/or returning empty when !targetNumberClass.isInstance(convertedValue).

Copilot uses AI. Check for mistakes.

@Override
public Optional<Map<String, Object>> asMap() {
return object instanceof Map ? Optional.of((Map<String, Object>) object) : Optional.empty();
Expand Down
4 changes: 3 additions & 1 deletion impl/core/src/main/java/io/serverlessworkflow/impl/AbstractWorkflowModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ public abstract class AbstractWorkflowModel implements WorkflowModel {

protected abstract <T> Optional<T> convert(Class<T> clazz);

protected abstract <T> Optional<T> asNumber(Class<?> targetNumberClass);
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding a new abstract method to a public abstract class is a source/binary breaking change for any external subclasses. If AbstractWorkflowModel is intended to be extended outside this module, consider providing a backward-compatible default implementation (e.g., a non-abstract asNumber(...) that falls back to the existing numeric handling) or reducing the exposure (package-private/sealed) if external extension isn’t supported.

Suggested change
protected abstract <T> Optional<T> asNumber(Class<?> targetNumberClass);
@SuppressWarnings("unchecked")
protected <T> Optional<T> asNumber(Class<?> targetNumberClass) {
if (!Number.class.isAssignableFrom(targetNumberClass)) {
return Optional.empty();
}
return (Optional<T>) convert((Class<T>) targetNumberClass);
}

Copilot uses AI. Check for mistakes.

@Override
public <T> Optional<T> as(Class<T> clazz) {
if (WorkflowModel.class.isAssignableFrom(clazz)) {
Expand All @@ -35,7 +37,7 @@ public <T> Optional<T> as(Class<T> clazz) {
} else if (OffsetDateTime.class.isAssignableFrom(clazz)) {
return (Optional<T>) asDate();
} else if (Number.class.isAssignableFrom(clazz)) {
return (Optional<T>) asNumber();
return asNumber(clazz);
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new numeric path uses Class<?> + unchecked casts in implementations. To improve type safety and remove the need for @SuppressWarnings(\"unchecked\"), consider changing the hook to a bounded generic signature such as protected abstract <N extends Number> Optional<N> asNumber(Class<N> targetNumberClass); and calling it with clazz.asSubclass(Number.class) (or equivalent) in this branch.

Copilot uses AI. Check for mistakes.
} else if (Collection.class.isAssignableFrom(clazz)) {
Collection<?> collection = asCollection();
return collection.isEmpty() ? Optional.empty() : (Optional<T>) Optional.of(collection);
Expand Down
23 changes: 23 additions & 0 deletions impl/model/src/main/java/io/serverlessworkflow/impl/model/jackson/JacksonModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,29 @@ public Optional<Number> asNumber() {
return node.isNumber() ? Optional.of(node.asLong()) : Optional.empty();
}

@Override
@SuppressWarnings("unchecked")
protected <T> Optional<T> asNumber(Class<?> targetNumberClass) {
if (!node.isNumber()) {
return Optional.empty();
}
if (targetNumberClass == Integer.class) {
return (Optional<T>) Optional.of(node.asInt());
} else if (targetNumberClass == Long.class) {
return (Optional<T>) Optional.of(node.asLong());
} else if (targetNumberClass == Double.class) {
return (Optional<T>) Optional.of(node.asDouble());
} else if (targetNumberClass == Float.class) {
return (Optional<T>) Optional.of((float) node.asDouble());
} else if (targetNumberClass == Short.class) {
return (Optional<T>) Optional.of((short) node.asInt());
} else if (targetNumberClass == Byte.class) {
return (Optional<T>) Optional.of((byte) node.asInt());
} else {
return (Optional<T>) Optional.of(node.numberValue());
}
}
Comment on lines +72 to +93
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The else branch returns node.numberValue() for any Number subclass target (e.g., BigDecimal.class, BigInteger.class). numberValue() is not guaranteed to be an instance of targetNumberClass, so model.as(BigDecimal.class) can return an Optional containing an Integer/Long/... and then fail with a ClassCastException at the call site. Fix by either (1) explicitly handling BigDecimal/BigInteger using node.decimalValue() / node.bigIntegerValue(), and/or (2) checking targetNumberClass.isInstance(value) before returning and otherwise returning Optional.empty() (or performing a supported conversion).

Copilot uses AI. Check for mistakes.

@Override
public String toString() {
return node.toPrettyString();
Expand Down
179 changes: 179 additions & 0 deletions impl/test/src/test/java/io/serverlessworkflow/impl/test/WorkflowNumberConversionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
/*
* Copyright 2020-Present The Serverless Workflow Specification Authors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package io.serverlessworkflow.impl.test;

import static io.serverlessworkflow.fluent.func.dsl.FuncDSL.function;

import io.serverlessworkflow.api.types.Workflow;
import io.serverlessworkflow.fluent.func.FuncWorkflowBuilder;
import io.serverlessworkflow.impl.WorkflowApplication;
import io.serverlessworkflow.impl.WorkflowModel;
import java.util.function.Function;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.Test;

public class WorkflowNumberConversionTest {

@Test
void incompatible_test() {
Workflow workflow =
FuncWorkflowBuilder.workflow("numbers")
.tasks(
function(
"scoreProposal",
(Proposal input) -> {
Integer score = calculateScore(input.abstractText());
System.out.println("Score calculated having the result as: " + score);
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Avoid System.out.println in unit tests since it adds noise to test output and makes failures harder to scan. Prefer assertions only, or use a test logger if you need diagnostic output.

Suggested change
System.out.println("Score calculated having the result as: " + score);

Copilot uses AI. Check for mistakes.
return score;
},
Proposal.class)
.outputAs(
(Integer score) -> new ProposalScore(score, score >= 7), Integer.class))
.build();

try (WorkflowApplication app = WorkflowApplication.builder().build()) {
WorkflowModel model =
app.workflowDefinition(workflow)
.instance(new Proposal("Workflow, workflow, workflow..."))
.start()
.join();
Assertions.assertNotNull(model);
ProposalScore result = model.as(ProposalScore.class).orElseThrow();
Assertions.assertEquals(10, result.score());
Assertions.assertTrue(result.accepted());
}
}

@Test
void long_to_integer_conversion() {
Workflow workflow =
FuncWorkflowBuilder.workflow("longToInt")
.tasks(
function("convertLong", Function.identity(), Long.class)
.outputAs((Integer result) -> result * 2, Integer.class))
.build();

try (WorkflowApplication app = WorkflowApplication.builder().build()) {
WorkflowModel model = app.workflowDefinition(workflow).instance(100L).start().join();
Integer result = model.as(Integer.class).orElseThrow();
Assertions.assertEquals(200, result);
}
}

@Test
void integer_to_long_conversion() {
Workflow workflow =
FuncWorkflowBuilder.workflow("intToLong")
.tasks(
function("convertInt", Function.identity(), Integer.class)
.outputAs((Long result) -> result * 3L, Long.class))
.build();

try (WorkflowApplication app = WorkflowApplication.builder().build()) {
WorkflowModel model = app.workflowDefinition(workflow).instance(50).start().join();
Long result = model.as(Long.class).orElseThrow();
Assertions.assertEquals(150L, result);
}
}

@Test
void double_to_integer_conversion() {
Workflow workflow =
FuncWorkflowBuilder.workflow("doubleToInt")
.tasks(
function("convertDouble", Function.identity(), Double.class)
.outputAs((Integer result) -> result + 5, Integer.class))
.build();

try (WorkflowApplication app = WorkflowApplication.builder().build()) {
WorkflowModel model = app.workflowDefinition(workflow).instance(42.7).start().join();
Integer result = model.as(Integer.class).orElseThrow();
Assertions.assertEquals(47, result);
}
}

@Test
void float_to_double_conversion() {
Workflow workflow =
FuncWorkflowBuilder.workflow("floatToDouble")
.tasks(
function("convertFloat", Function.identity(), Float.class)
.outputAs((Double result) -> result * 1.5, Double.class))
.build();

try (WorkflowApplication app = WorkflowApplication.builder().build()) {
WorkflowModel model = app.workflowDefinition(workflow).instance(10.0f).start().join();
Double result = model.as(Double.class).orElseThrow();
Assertions.assertEquals(15.0, result, 0.001);
}
}

@Test
void short_to_integer_conversion() {
Workflow workflow =
FuncWorkflowBuilder.workflow("shortToInt")
.tasks(
function("convertShort", (Short input) -> input.intValue(), Short.class)
.outputAs((Integer result) -> result * 10, Integer.class))
.build();

try (WorkflowApplication app = WorkflowApplication.builder().build()) {
WorkflowModel model = app.workflowDefinition(workflow).instance((short) 5).start().join();
Integer result = model.as(Integer.class).orElseThrow();
Assertions.assertEquals(50, result);
}
}

@Test
void byte_to_integer_conversion() {
Workflow workflow =
FuncWorkflowBuilder.workflow("byteToInt")
.tasks(
function("convertByte", Function.identity(), Byte.class)
.outputAs((Integer result) -> result + 100, Integer.class))
.build();

try (WorkflowApplication app = WorkflowApplication.builder().build()) {
WorkflowModel model = app.workflowDefinition(workflow).instance((byte) 25).start().join();
Integer result = model.as(Integer.class).orElseThrow();
Assertions.assertEquals(125, result);
}
}

@Test
void number_conversion_with_string_output() {
// Test that when output is not a number, numeric conversion returns empty
// This tests the edge case where asNumber() returns Optional.empty()
Workflow workflow =
FuncWorkflowBuilder.workflow("stringOutput")
.tasks(function("returnString", (Integer input) -> "result: " + input, Integer.class))
.build();

try (WorkflowApplication app = WorkflowApplication.builder().build()) {
WorkflowModel model = app.workflowDefinition(workflow).instance(42).start().join();
Assertions.assertTrue(model.as(Integer.class).isEmpty());
Assertions.assertEquals("result: 42", model.as(String.class).orElseThrow());
}
}

private Integer calculateScore(String abstractText) {
return abstractText.contains("Workflow") ? 10 : 5;
}

public record ProposalScore(Integer score, boolean accepted) {}

public record Proposal(String abstractText) {}
}