Fix Axis.getDirection() to never return null (#80)#81
Fix Axis.getDirection() to never return null (#80)#81desruisseaux merged 6 commits intoOSGeo:mainfrom
Conversation
|
Do you have the Well Known Text (WKT) definition of the Coordinate Reference System (CRS) for which the axis direction is null? It would be for checking if we can infer a better axis direction than |
|
Thank you for the feedback, @desruisseaux! Here’s the WKT of the CRS where AxisDirection was null: ' ' ' wkt |
|
While latitude is very likely to be oriented toward North, we cannot be sure as it could also be South. Same for longitude. It is safer to not try to guess. I do not know if the issue is on PROJ side or elsewhere. On one side, PROJ-JNI is implemented only as below, relying fully on PROJ: value = get_shared_object<CoordinateSystemAxis>(env, object)->direction().toString().c_str();On the other side, PROJ seems to have the information somewhere since it appears in the WKT. I suggest to keep the pull request as-is with only one amendment (I will post a comment directly on the line of code). Maybe the issue will disappear in some future PROJ version. |
| return search(AxisDirection.class, dir); | ||
| AxisDirection direction = search(AxisDirection.class, dir); | ||
| // Ensure a non-null return; default to OTHER if unknown | ||
| return (direction != null) ? direction : AxisDirection.OTHER; |
There was a problem hiding this comment.
Replace AxisDirection.OTHER by AxisDirection.valueOf("UNSPECIFIED") and add the following comment:
// TODO: Use AxisDirection.UNSPECIFIED in GeoAPI 3.1.
The "Unspecified" axis direction has been added in the ISO 19111:2019 international standard. GeoAPI has not yet been updated for that revision, but this is planed for GeoAPI 3.1. In the meantime, the use of valueOf(String) should allow us to anticipate.
Ensure a non-null return for axis direction, defaulting to UNSPECIFIED if unknown.
Update getDirection to handle null values
|
Thanks, @desruisseaux — updated accordingly. This approach should maintain compatibility with current GeoAPI versions while anticipating the addition of UNSPECIFIED. The logic still guarantees a non-null return value, ensuring downstream safety. |
| public AxisDirection getDirection() { | ||
| final String dir = impl.getStringProperty(Property.DIRECTION); | ||
| return search(AxisDirection.class, dir); | ||
| final String dir = impl.getStringProperty(Property.DIRECTION); |
|
@desruisseaux |
|
Merged, thanks! |
2 participants
This pull request addresses issue #80, where Axis.getDirection() could return null if the underlying Property.DIRECTION was missing or unrecognized. Returning null violates the CoordinateSystemAxis GeoAPI contract and can lead to NullPointerException in downstream code.
Approach:
Updated the getDirection() method in Axis.java to check the result of the internal search method.
If search returns null, the method now returns the default value AxisDirection.OTHER.
Preserved existing behavior for recognized axis directions.
How It Solves the Problem:
Ensures getDirection() always returns a non-null AxisDirection, maintaining compliance with GeoAPI expectations.
Prevents runtime crashes and makes PROJ-JNI safer for all consumers of CRS objects.
No breaking changes for existing valid axis directions.
Example:
AxisDirection direction = axis.getDirection(); // Never null, defaults to OTHER if unknown
Impact:
Fixes null-safety issue in Axis class.
Improves robustness and GeoAPI compliance.