Skip to content

fix(gcs): use snake_case field names in notification request and response#82

Open
nicoechegut wants to merge 2 commits into
floci-io:mainfrom
nicoechegut:fix-create-notification-fields
Open

fix(gcs): use snake_case field names in notification request and response#82
nicoechegut wants to merge 2 commits into
floci-io:mainfrom
nicoechegut:fix-create-notification-fields

Conversation

@nicoechegut

Copy link
Copy Markdown

Summary

The Google Storage REST API uses snake_case for notification fields (payload_format, event_types, object_name_prefix, custom_attributes), but createNotification was reading camelCase keys from the request body, silently dropping those values. The GET response also serialized as camelCase, so clients reading back a notification would receive empty values for those fields.

Fix GcsService to accept snake_case keys and add @JsonProperty annotations to StoredNotification so the response matches the API spec: https://docs.cloud.google.com/storage/docs/json_api/v1/notifications

Type of change

  • Bug fix (fix:)
  • New feature (feat:)
  • Breaking change (feat!: or fix!:)
  • Docs / chore

GCP Compatibility

Verified by creating a GCS Notification via a Terraform test and the hashicorp/google Terraform provider.
Before the fix, Terraform fails to apply with error, indicating the API returns empty fields:

│ Error: Provider produced inconsistent result after apply
│ 
│ When applying changes to
│ module.gcs_bucket.google_storage_notification.this[0], provider
│ "provider[\"registry.terraform.io/hashicorp/google\"]" produced an
│ unexpected new value: .payload_format: was cty.StringVal("JSON_API_V1"),
│ but now cty.StringVal("").
│ 
│ This is a bug in the provider, which should be reported in the provider's
│ own issue tracker.

Ran the sdk-test-java compatibility tests which include GcsNotificationTest.

Checklist

  • ./mvnw test passes locally
  • New or updated integration test added
  • Commit messages follow Conventional Commits

@nicoechegut
nicoechegut marked this pull request as ready for review July 2, 2026 17:16
@greptile-apps

greptile-apps Bot commented Jul 2, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes a bug where createNotification was silently dropping payload_format, event_types, custom_attributes, and object_name_prefix because it was looking up camelCase keys in a JSON body that the GCS REST API sends as snake_case. The fix is minimal and correct: key lookups in GcsService are switched to snake_case, StoredNotification gains @JsonProperty annotations so responses also serialize as snake_case, and @JsonAlias entries preserve backward compatibility for any existing stored camelCase data.

  • GcsService.createNotification now reads payload_format, event_types, custom_attributes, and object_name_prefix from the request body, matching the GCS JSON API v1 wire format.
  • StoredNotification adds @JsonProperty to serialize responses in snake_case, plus @JsonAlias for backward-compatible deserialization of previously-persisted camelCase values.
  • GcsNotificationTest is extended to assert that all four fields round-trip correctly through create and get.

Confidence Score: 5/5

Safe to merge — the change corrects a silent data-loss bug in notification creation and response serialization with no collateral impact.

The fix is minimal and mechanical: four key-name corrections in a single method and four Jackson annotations on the model. The @JsonAlias entries ensure any existing persisted camelCase data remains readable after the upgrade. The compatibility test suite now explicitly validates all four fields round-trip through create and get, matching what was broken before.

No files require special attention.

Important Files Changed

Filename Overview
src/main/java/io/floci/gcp/services/gcs/GcsService.java Four snake_case key lookups replace their camelCase counterparts in createNotification; logic is otherwise unchanged and correct.
src/main/java/io/floci/gcp/services/gcs/model/StoredNotification.java @JsonProperty annotations added to serialize the four compound fields as snake_case; @JsonAlias entries provide backward compatibility when reading previously-stored camelCase JSON.
compatibility-tests/sdk-test-java/src/test/java/io/floci/gcp/test/GcsNotificationTest.java Stores the full Notification object instead of just the ID, and adds assertions for payloadFormat, eventTypes, customAttributes, and objectNamePrefix on both create and get paths.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Client as GCP SDK / Terraform
    participant Controller as GcsController
    participant Service as GcsService
    participant Store as notificationStore

    Client->>Controller: "POST /storage/v1/b/{bucket}/notificationConfigs"
    Controller->>Service: createNotification(bucket, body Map)
    Service->>Service: body.get("payload_format") ✓
    Service->>Store: put(bucket:id, StoredNotification)
    Service-->>Controller: StoredNotification
    Controller-->>Client: "{ payload_format, event_types, custom_attributes, object_name_prefix }"

    Client->>Controller: "GET /storage/v1/b/{bucket}/notificationConfigs/{id}"
    Controller->>Service: getNotification(bucket, id)
    Service->>Store: get(bucket:id)
    Store-->>Service: StoredNotification
    Service-->>Controller: StoredNotification
    Controller-->>Client: "{ payload_format, event_types, custom_attributes, object_name_prefix }"
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Client as GCP SDK / Terraform
    participant Controller as GcsController
    participant Service as GcsService
    participant Store as notificationStore

    Client->>Controller: "POST /storage/v1/b/{bucket}/notificationConfigs"
    Controller->>Service: createNotification(bucket, body Map)
    Service->>Service: body.get("payload_format") ✓
    Service->>Store: put(bucket:id, StoredNotification)
    Service-->>Controller: StoredNotification
    Controller-->>Client: "{ payload_format, event_types, custom_attributes, object_name_prefix }"

    Client->>Controller: "GET /storage/v1/b/{bucket}/notificationConfigs/{id}"
    Controller->>Service: getNotification(bucket, id)
    Service->>Store: get(bucket:id)
    Store-->>Service: StoredNotification
    Service-->>Controller: StoredNotification
    Controller-->>Client: "{ payload_format, event_types, custom_attributes, object_name_prefix }"
Loading

Reviews (2): Last reviewed commit: "test: Add round-trip assertions for all ..." | Re-trigger Greptile

@hectorvent

hectorvent commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Thanks for the fix!

Verified against the JSON API spec. notifications really is the one GCS resource using snake_case, so this is exactly right. It also fixes event-type/prefix filtering, which was silently ignored before.

Two small asks:

1. Add round-trip assertions to GcsNotificationTest (it only checks topic/id, so it can't catch a regression here):

assertThat(notification.getPayloadFormat()).isEqualTo(NotificationInfo.PayloadFormat.JSON_API_V1);
assertThat(notification.getEventTypes())
        .containsExactlyInAnyOrder(NotificationInfo.EventType.OBJECT_FINALIZE,
                NotificationInfo.EventType.OBJECT_DELETE);

2. Add @JsonAlias so existing persisted data (camelCase) still loads:

@JsonProperty("payload_format")
@JsonAlias("payloadFormat")
private String payloadFormat = "JSON_API_V1";

Otherwise LGTM

@hectorvent hectorvent added storage Cloud Storage (GCS) waiting-contributor labels Jul 7, 2026
@nicoechegut
nicoechegut force-pushed the fix-create-notification-fields branch from 4448771 to dcbe741 Compare July 15, 2026 10:31
@nicoechegut
nicoechegut force-pushed the fix-create-notification-fields branch from dcbe741 to 51b4200 Compare July 15, 2026 10:32
@nicoechegut

Copy link
Copy Markdown
Author

Thanks for the review @hectorvent
I've addressed the feedback: added round-trip assertions and Json alias on all notification fields.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants