Skip to content
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.

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

AbstractUnit.asType() will accept any si-units as valid class input #435

Open
xKrummy opened this issue Feb 14, 2025 · 1 comment
Open

Comments

@xKrummy
Copy link

xKrummy commented Feb 14, 2025

Problem:

We are using the framework within our project and do some tweaks to ensure that the dimension is always specific and correctly set (so we don't need to use wildcards in our quantites).

When we want to create a unit from a String, we use the AbstractUnit method call "asType" to ensure that the unit can be parsed into the correct dimension.
This works well for Units from thejavax.measure:unit-api(2.2) but not for the si.uom:si-quantity (2.2).

Reason:

The current implementation looks like this:

public final <T extends Quantity<T>> Unit<T> asType(Class<T> type) {
	Dimension typeDimension = UnitDimension.of(type);
	if (typeDimension != null && !typeDimension.equals(this.getDimension()))
		throw new ClassCastException("The unit: " + this + " is not compatible with quantities of type " + type);
	return (Unit<T>) this;
}

and UnitDimension.of(type) returns always null for the SI-Units

How to reproduce:

var unitAsString = "kg/h/l";
var unit = AbstractUnit.parse(unitAsString);

// The following should make sure that the given unit from string is matching the expected dimension. It will
// throw an exception if it does not match for javax units, but not for si-quantity unit.
var mass = unit.asType(Mass.class); // throws error -> correct
var dimensionless = unit.asType(Dimensionless.class); // throws error -> correct
var density = unit.asType(Density.class); // no error -> incorrect
var luminance = unit.asType(Luminance.class); // no error -> incorrect

var unitKilogramPerLiter = Units.KILOGRAM.divide(Units.HOUR.divide(Units.LITRE));

assertEquals(unitKilogramPerLiter.toString(), density.toString()); // success -> incorrect behaviour
assertEquals(unitKilogramPerLiter.toString(), luminance.toString()); // success -> incorrect behaviour
Correct behaviour (throwing errors) Incorrect behaviour (no errors thrown)
Image Image
@xKrummy xKrummy changed the title AbstractUnit.asType() will accept any si-units as valid class input Bug: AbstractUnit.asType() will accept any si-units as valid class input Feb 14, 2025
@keilw keilw added the analysis label Feb 18, 2025
@keilw keilw changed the title Bug: AbstractUnit.asType() will accept any si-units as valid class input AbstractUnit.asType() will accept any si-units as valid class input Feb 18, 2025
@keilw
Copy link
Member

keilw commented Feb 18, 2025

I changed this to feature, because UnitDimension.of() states it returns

results from the default {@link javax.measure.spi.SystemOfUnits SystemOfUnits} or null if the specified
quantity is unknown

And in this case the types from other unit systems are just unknown. It also has a TODO for new or custom types.

Unfortunately it is not possible to go over all the registered unit systems via their service providers because AbstractUnit.asType() is called throughout the static declaration of unit constants, and trying to call the ServiceProvider there won't work, it creates some kind of "chicken and egg" problem during the initialization and causes initialization errors.

keilw added a commit to unitsofmeasurement/uom-systems that referenced this issue Feb 18, 2025
keilw added a commit to unitsofmeasurement/si-units that referenced this issue Feb 18, 2025
@keilw keilw added this to the 2.2.3 milestone Feb 19, 2025
@keilw keilw moved this to 🏗 In progress in Units of Measurement backlog Feb 19, 2025
keilw added a commit to unitsofmeasurement/si-units that referenced this issue Feb 19, 2025
keilw added a commit to unitsofmeasurement/si-units that referenced this issue Feb 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 🏗 In progress
Development

No branches or pull requests

2 participants