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

[mcruzdev]

Changes

• Implement convertNumber method in AbstractWorkflowModel to handle conversions between numeric types
• Fix issue where asNumber() returned incompatible Number types without proper casting
• Ensure type safety when converting workflow model outputs to specific numeric types

Closes #1200

[Copilot]

Pull request overview

This PR adds a convertNumber private method to AbstractWorkflowModel to properly dispatch numeric type conversions when as(Class<T>) is called with a Number-derived class (previously returning whatever asNumber() produced, which could cause a ClassCastException at the call site). It also adds a comprehensive suite of integration tests covering several primitive wrapper conversions (Long↔Integer, Double↔Integer, Float↔Double, Short↔Integer, Byte↔Integer).

Changes:

• AbstractWorkflowModel.java: Replace the raw asNumber() call with asNumber().map(num -> convertNumber(num, clazz)), and add the convertNumber dispatch method.
• WorkflowNumberConversionTest.java: New integration test class exercising seven conversion scenarios.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
impl/core/src/main/java/io/serverlessworkflow/impl/AbstractWorkflowModel.java	Adds convertNumber method and wires it into as() for Number-typed requests
impl/test/src/test/java/io/serverlessworkflow/impl/test/WorkflowNumberConversionTest.java	Integration tests for numeric type conversions across multiple primitive wrapper types

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The convertNumber method does not handle BigDecimal or BigInteger types, even though both are Number subclasses supported by the codebase (as evidenced by JacksonModelFactory.from(Number) and JsonUtils handling them). When as(BigDecimal.class) or as(BigInteger.class) is called, execution reaches the else { return (T) num; } branch. Because asNumber() in JacksonModel always returns a Long (via node.asLong()), this unchecked cast will produce a ClassCastException at the call site. The convertNumber method should explicitly handle BigDecimal and BigInteger — for example, by calling new BigDecimal(num.toString()) or BigDecimal.valueOf(num.longValue()) for BigDecimal, and BigInteger.valueOf(num.longValue()) for BigInteger.

[fjtirado]

I think we need to handle this differently, rather than using the asNumber public interface method, add a new protected method here asNumber(Class<?> targetNumberClass) and implement it on the children classes.

That way, you can use JsonNode.asLong, asInt methods in JakcsonModel and copy this implementation to JavaModel. The approach you took implies two conversions: the one performed by asNumber (in JavaModel is straightforward, but in Jackson it is not) and the one performed by this method. It is more efficient to have one in the children class.

[fjtirado]

here is whereasNumber(class)method should be invoked

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

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]

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]

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]

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]

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);