Skip to content

Respect composed adapter for unregistered structs#1384

Open
lopster568 wants to merge 1 commit into
cel-expr:masterfrom
lopster568:native-respect-base-adapter-unregistered-structs
Open

Respect composed adapter for unregistered structs#1384
lopster568 wants to merge 1 commit into
cel-expr:masterfrom
lopster568:native-respect-base-adapter-unregistered-structs

Conversation

@lopster568

Copy link
Copy Markdown

nativeTypeProvider.NativeToValue wrapped every struct-kind value as a native object regardless of whether the type was registered through ext.NativeTypes. Every other method on the provider checks the nativeTypes registry first and delegates unregistered types to the composed base provider or adapter; NativeToValue was the exception, so a cel.CustomTypeAdapter composed alongside ext.NativeTypes never saw struct values it should convert itself.

This mirrors the registry check from NewValue: registered struct types are still wrapped as native objects, and unregistered ones now reach the base adapter. This is the fix proposed in the issue and changes behavior for unregistered structs.

Fixes #1343

nativeTypeProvider.NativeToValue routed every struct-kind value to
newNativeObject, regardless of whether the type was registered via
ext.NativeTypes. Every other method on the provider checks the
nativeTypes registry first and falls back to the composed base adapter
when a type is not registered; NativeToValue was the exception, so a
custom adapter added with cel.CustomTypeAdapter never saw unregistered
structs it wanted to convert itself.

Mirror the registry check from NewValue: only wrap registered struct
types as native objects, and delegate the rest to the base adapter.

Fixes cel-expr#1343
@google-cla

google-cla Bot commented Jul 24, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@TristonianJones TristonianJones left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the fix! I just have one minor code maintenance request, but otherwise LGTM

Comment thread ext/native.go
@TristonianJones

Copy link
Copy Markdown
Collaborator

/gcbrun

@TristonianJones

Copy link
Copy Markdown
Collaborator

@lopster568 I'm just waiting on the CLA check before I can accept the contribution. Thanks for the fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

NativeTypes doesn't use baseAdapter when it should

2 participants