Skip to content

Enhance ux/dx #61

Open
Open

Description

@duglin

From @oising (#59):

I went through the current design with copilot and instructed it on how to have better cohesion between commands as well as improving the ux/dx, and I came up with this plan -- it wouldn't strictly be breaking, but I would be inclined to bump a minor version at least:

Plan: Pipeline binding improvements for CloudEvents.Sdk

Current state (src/CloudEventsPowerShell/CloudEvents.Sdk.psm1)

  • 9 functions: New-CloudEvent, Set-CloudEvent{,Json,Xml}Data, Read-CloudEvent{,Json,Xml}Data, ConvertTo-HttpMessage, ConvertFrom-HttpMessage.
  • Only -CloudEvent has ValueFromPipeline. No parameter uses ValueFromPipelineByPropertyName (ConvertTo-/ConvertFrom-HttpMessage set it explicitly to $false).
  • No [OutputType], no [Alias], no parameter sets. Output is emitted with Write-Output.

Output → input cohesion gaps

  • ConvertTo-HttpMessage emits {Headers, Body}, but ConvertFrom-HttpMessage can't take it from the pipeline.
  • Invoke-WebRequest/Invoke-RestMethod responses expose Headers and Content. Body doesn't match Content, so nothing binds.
  • ConvertTo-HttpMessage output can't be piped into Invoke-WebRequest. This is acceptable, but splatting should be documented.
  • New-CloudEvent parameters (Type/Source/Id/Time/DataSchema) mirror CloudEvent property names but can't take objects (CSV rows, other events, custom objects) from the pipeline.
  • Set-CloudEvent*Data can't take -Data from the pipeline.
  • Reading from one event and writing to another needs a manual step.

Bugs coupled to parameter metadata

  • Set-CloudEventData:
    • -DataContentType is a [string] that is never validated or converted.
    • The ContentType object built with New-Object is unused.
    • It throws when -DataContentType is omitted.
    • The raw string is assigned to $cloudEvent.DataContentType.
  • Set-CloudEventXmlData: -AttributesKeysInElementAttributes is a mandatory [bool]. Changing it to a switch would be breaking, so it is deferred.
  • Validation attributes are inconsistent (ValidateNotNullOrEmpty vs ValidateNotNull). -Body on ConvertFrom-HttpMessage is untyped.

Improvement list (non-breaking unless noted)

  1. ConvertFrom-HttpMessage
    • Add ValueFromPipelineByPropertyName = $true to -Headers and -Body.
    • Add [Alias('Content')] on -Body.
    • Accept [System.Collections.IDictionary] for -Headers, a superset of hashtable that handles the Dictionary<string,string[]> from Invoke-WebRequest in PS7.
    • Result: ConvertTo-HttpMessage | ConvertFrom-HttpMessage and Invoke-WebRequest … | ConvertFrom-HttpMessage both work.
    • Positional and named usage stays identical.
  2. ConvertTo-HttpMessage
    • Remove the explicit ByPropertyName = $false.
    • Add [OutputType] and a PSTypeName (e.g. CloudEvents.HttpMessage) on the output so it is discoverable.
    • Emit the same Headers/Body properties.
  3. New-CloudEvent
    • Add ValueFromPipelineByPropertyName = $true on Type, Source, Id, Time, DataSchema.
    • Enables Import-Csv | New-CloudEvent and event-to-event cloning.
    • Make -Id and -Time optional, defaulting to [guid]::NewGuid() and the current time. This is additive and doesn't break existing callers.
    • Add [OutputType([CloudNative.CloudEvents.CloudEvent])].
  4. Set-CloudEventData / Json / Xml
    • Add ValueFromPipelineByPropertyName to -Data, -DataContentType and -DataSchema (Json/Xml: -Data and -DataSchema).
    • Keep -CloudEvent as ValueFromPipeline, so by-value binding wins for CloudEvent input.
    • Decision: -Data binds by property name. Mitigate the case where a piped CloudEvent binds its own Data (via parameter sets or guard logic) and cover it with a test.
  5. Read-CloudEvent*Data
    • Add [OutputType] (object / hashtable / etc.).
    • Add ValueFromPipelineByPropertyName to -CloudEvent, so wrapper objects with a CloudEvent property bind.
    • Make -Depth defaults consistent between Set and Read Json.
  6. Consistent -CloudEvent parameter
    • Add [Alias('InputObject','Event')] in all 8 functions that take a CloudEvent.
    • Unify validation to [ValidateNotNull()].
    • Drop the redundant = $false attribute noise.
  7. Pass-through: Set-* already emit the event, so no -PassThru is needed. Only simplify Write-Output if there is no behavior change.
  8. Bug fixes (low risk)
    • Set-CloudEventData: only build the ContentType when -DataContentType is supplied, and assign the ContentType object.
    • Review -ConvertMode handling on Read-CloudEventXmlData.
  9. Deferred (breaking, next major)
    • Make -AttributesKeysInElementAttributes a [switch].
    • Make -ConvertMode non-mandatory with a default.
  10. Help and docs
    • Fix help inconsistencies (Add-CloudEvent*Data → Set-CloudEvent*Data, typos).
    • Add pipeline examples plus .INPUTS/.OUTPUTS.
    • Add pipeline recipes to the README.
  11. Tests (test/unit/*.Tests.ps1)
    • Property-bound piping into New-CloudEvent.
    • ConvertTo-HttpMessage | ConvertFrom-HttpMessage round trip.
    • An IWR-shaped object (Headers, Content) piped into ConvertFrom-HttpMessage.
    • Set-*Data with Data/DataContentType bound by property name.
    • A regression test for the Set-CloudEventData content-type bug.
    • Run with test/RunTests.ps1.

Test policy

  • Run the existing suite before changes (baseline) and after.
  • If any existing test breaks because of a behavioral change, document it (test name, old vs new behavior, reason) in the PR description / CHANGELOG notes and in the final summary.
  • Add new, independent Pester tests (self-contained, no shared state or ordering dependence) for every improvement.

Todos

  1. Implement items 1–2 (HTTP cohesion).
  2. Implement item 3 (New-CloudEvent).
  3. Implement item 4 (Set-* data).
  4. Implement items 5–6 (Read-*, aliases, validation, OutputType).
  5. Fix the item 8 bugs.
  6. Update docs and help (item 10).
  7. Add tests (item 11) and run the unit suite before and after.

Decisions made

  • -Data on Set-CloudEvent*Data accepts pipeline-by-property: yes.
  • New-CloudEvent -Id/-Time auto-default: yes.
  • New output type name CloudEvents.HttpMessage: proposed, not yet confirmed

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions