Skip to content

Conversation

@dconeybe
Copy link
Contributor

@dconeybe dconeybe commented Dec 23, 2025

This PR refactors the "path" handling logic within the dataconnect testing utilities, namely removing duplication by consolidating path-related functions and types into a dedicated test extension file, and migrating all dependent code to use these unified types and utilities.

Highlights

  • De-duplication of Path Logic: The ProtoValuePath.kt file has been removed, eliminating redundant path-related logic within the dataconnect testing module.
  • Consolidation of Test Utilities: A new file, DataConnectPathSegmentTestExts.kt, has been introduced to centralize test-specific utilities for DataConnectPathSegment, including functions for path string conversion, adding segments, and comparators.
  • Migration to DataConnectPath: Existing test utilities for proto diffing, mapping, and walking have been updated to utilize the DataConnectPath and MutableDataConnectPath type aliases, ensuring consistency across the codebase.
  • New Type Alias: An internal type alias MutableDataConnectPath has been added in DataConnectPathSegment.kt to represent mutable lists of DataConnectPathSegment.
Changelog
  • DataConnectPathSegment.kt
    • Added internal typealias MutableDataConnectPath = MutableList<DataConnectPathSegment>.
  • DataConnectPathSegmentTestExts.kt
    • New file created, containing various path utility functions (toPathString, addField, withAddedField, etc.) and comparators (DataConnectPathComparator, DataConnectPathSegmentComparator) for DataConnectPathSegment.
  • ProtoDiff.kt
    • Replaced ProtoValuePath with DataConnectPath and MutableProtoValuePath with MutableDataConnectPath in data classes and function signatures.
    • Updated method calls from withAppendedStructKey to withAddedField and withAppendedListIndex to withAddedListIndex.
    • Changed structKeyOrThrow() to fieldOrThrow().
  • ProtoMap.kt
    • Replaced ProtoValuePath with DataConnectPath and MutableProtoValuePath with MutableDataConnectPath in function signatures.
    • Updated method calls from withAppendedStructKey to withAddedField and withAppendedListIndex to withAddedListIndex.
  • ProtoValuePath.kt
    • File removed, as its functionality has been de-duplicated and migrated.
  • ProtoWalk.kt
    • Replaced ProtoValuePathPair with DataConnectPathValuePair in function signatures and data structures.
    • Updated method calls from withAppendedStructKey to withAddedField and withAppendedListIndex to withAddedListIndex.
  • proto.kt
    • Updated imports and type aliases from ProtoValuePath to DataConnectPath and ProtoValuePathPair to DataConnectPathValuePair.
    • Updated method calls from withAppendedListIndex to withAddedListIndex and withAppendedStructKey to withAddedField.
  • ProtoDiffUnitTest.kt
    • Updated type aliases from ProtoValuePath to DataConnectPath and ProtoValuePathPair to DataConnectPathValuePair.
    • Changed filter condition from it.path.lastOrNull().isStructKey() to it.path.lastOrNull() is DataConnectPathSegment.Field.
    • Updated structKeyOrThrow() to fieldOrThrow().
  • ProtoWalkUnitTest.kt
    • Updated type aliases from ProtoValuePathPair to DataConnectPathValuePair.
  • protoUnitTest.kt
    • Updated imports and type aliases from ProtoValuePathComponent to DataConnectPathSegment and ProtoValuePathPair to DataConnectPathValuePair.
    • Updated path component creation from ProtoValuePathComponent.StructKey to DataConnectPathSegment.Field and ProtoValuePathComponent.ListIndex to DataConnectPathSegment.ListIndex.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request is a nice refactoring that de-duplicates the logic for handling proto value paths in the test utilities. By removing the custom ProtoValuePath implementation and using DataConnectPathSegment from the main source set, the code is simplified and more consistent. The introduction of DataConnectPathSegmentTestExts.kt to house test-specific helpers is a good approach. I have a couple of minor suggestions for this new file to improve clarity and maintainability.

@dconeybe dconeybe marked this pull request as ready for review December 23, 2025 17:20
@firebase firebase deleted a comment from gemini-code-assist bot Dec 23, 2025
@firebase firebase deleted a comment from google-oss-bot Dec 23, 2025
Copy link

@stephenarosaj stephenarosaj left a comment

Choose a reason for hiding this comment

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

Awesome.

@dconeybe dconeybe merged commit 90182b3 into main Dec 23, 2025
42 checks passed
@dconeybe dconeybe deleted the dconeybe/dataconnect/ProtoValuePathDelete branch December 23, 2025 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants