fix(CedarJavaFFI): exhaustive match broken on EntitiesError update

[mark-creamer-amazon]

Problem

validate_entities in CedarJavaFFI/src/interface.rs matches exhaustively on cedar_policy::entities_errors::EntitiesError, enumerating all five variants with no wildcard arm. cedar-policy-core 4.12.0 added a sixth variant, InvalidEntityStructure, so the crate no longer compiles:

error[E0004]: non-exhaustive patterns: `EntitiesError::InvalidEntityStructure(_)` not covered
   --> src/interface.rs:509:37
    |
509 |             let err_message = match error {
    |                                     ^^^^^ pattern `EntitiesError::InvalidEntityStructure(_)` not covered
    |
note: `EntitiesError` defined here
   --> cedar-policy-core-4.12.0/src/entities/err.rs:24:1
    |
24  | pub enum EntitiesError {
    | ^^^^^^^^^^^^^^^^^^^^^^
...
50  |     InvalidEntityStructure(#[from] InvalidEntityStructureError),
    |     ---------------------- not covered

error: could not compile `cedar-java-ffi` (lib) due to 1 previous error

main is affected because CedarJavaFFI/Cargo.toml sources cedar from branch = "main", which now resolves to 4.12.0. Release branches such as release/4.10.x track frozen cedar branches and are unaffected.

Reproduce with a clean clone of main (cedar 4.12.0 requires rustc ≥ 1.89):

cd CedarJavaFFI && cargo +1.89 check

Fix

Keep explicit arms only for the variants whose own Display impl summarizes rather than delegating, and add a catch-all for the rest:

let err_message = match error {
    EntitiesError::Deserialization(err) => err.to_string(),
    EntitiesError::TransitiveClosureError(err) => err.to_string(),
    EntitiesError::InvalidEntity(err) => err.to_string(),
    err => err.to_string(),
};

Those three carry #[error("...")] messages that drop the inner error ("error during entity deserialization", "transitive closure computation/enforcement error", "entity does not conform to the schema"), so unwrapping them preserves the specific diagnostic returned to Java callers. Serialization interpolates its source via {0}, and Duplicate and InvalidEntityStructure are #[error(transparent)], so the catch-all loses no detail for them. These attributes are identical in 4.10.0 and 4.12.0.

This keeps the FFI compiling when new variants are added upstream, while retaining the detail the explicit match was there to provide.

Testing

cargo +1.89 check against cedar-policy-core 4.12.0 (efb14530) — compiles cleanly; the two remaining warnings are pre-existing on main.

Note for maintainers

EntitiesError is publicly re-exported as cedar_policy::entities_errors::EntitiesError and is not marked #[non_exhaustive], so adding a variant in 4.12.0 was a breaking change in a minor release. Maybe we should consider marking it #[non_exhaustive] in the cedar repo to prevent recurrence for downstream consumers?

[lianah]

If someone adds a new EntitiesError that has it's own Display summarizing the issue without interpolating this would not break cedar-java compilation but it would silently drop the inner detailed error message right? I wonder if a compilation error forcing us to fix this is not preferable to silently dropping the inner error message.

[teruokun]

The issue you have is that right now, your consumers are the ones broken since your own package doesn't lock down to compatible versions in any way and this is why libraries with Enums they plan to update often add a non-exhaustive directive to ensure proper compatibility guarantees are kept. In other words, you're violating core sem-ver principles in Rust where you're actually breaking on minor version updates to your underlying dependencies but don't constrain your dependencies for your consumed libraries to match.

So the ask would be either to:

1. Add such constraints as either an upper version boundary or precise version locking similar to the cedar-policy packages themselves (using "=VERSION" or <MAJOR.minor for the last minor version you know your release is compatible with.
2. Make the package flexible enough to accommodate additions that aren't considered "breaking" for the purposes of Rust semver

Otherwise you're just making all of your consumers have to lock or constrain otherwise transitive dependencies that they don't directly use. I'm not saying it isn't an available tool, but it's effectively passing breakages to your consumers for reporting them much like this one which doesn't engender trust if that isn't well-understood at outset.

[john-h-kastner-aws]

The new error message variant shouldn't have made it into the release. We have cargo-semver-checks to guard against this, but it's not perfect.

Probably the best option for now is to roll forward, leaving the new variant in place and patching the Java as propsed here. Adding #[non_exhaustive] is also breaking so unfortunately we can't just patch that in.

[mark-creamer-amazon]

Agreed with the above! In that case, are we aligned on this change to patch Cedar Java?

[mark-creamer-amazon]

Adding a fix for the SharedIntegrationTests.java to successfully run the corpus tests after cedar-policy/cedar-integration-tests#40 since policyFormat and schemaFormat are new to the serializer.

[mark-creamer-amazon]

Leaving this here until #365 is merged. I can also implement this case along with #365