Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validate published envelope topics #233

Merged
merged 1 commit into from
Oct 18, 2024

Conversation

neekolas
Copy link
Contributor

@neekolas neekolas commented Oct 16, 2024

tl;dr

AI Generated Summary

Refactored envelope handling and improved validation in the API service.

What changed?

  • Moved envelope-related test utilities to a dedicated package pkg/testutils/envelopes.
  • Enhanced ClientEnvelope struct with additional methods for topic handling and payload validation.
  • Updated Service.PublishEnvelopes to use the new envelope validation logic.
  • Improved error handling and messages in various API functions.
  • Introduced TopicMatchesPayload method to ensure consistency between envelope topic and payload.

How to test?

  1. Run the existing test suite to ensure all tests pass after the refactoring.
  2. Add new tests to cover the TopicMatchesPayload functionality in pkg/envelopes/client.go.
  3. Verify that publishing envelopes with mismatched topics and payloads now results in appropriate error messages.

Why make this change?

This refactoring improves code organization, enhances envelope validation, and increases the robustness of the API service. By centralizing envelope-related logic and adding stricter validation, we reduce the likelihood of processing invalid envelopes and improve overall system reliability.

Copy link

graphite-app bot commented Oct 16, 2024

Your org has enabled the Graphite merge queue for merging into main

Add the label “Queue” to the PR and Graphite will automatically add it to the merge queue when it’s ready to merge. Or use the label “Hotfix” to add to the merge queue as a hot fix.

You must have a Graphite account and log in to Graphite in order to use the merge queue. Sign up using this link.

Copy link
Contributor Author

neekolas commented Oct 16, 2024

@neekolas neekolas force-pushed the 10-16-validate_published_envelope_topics branch from b7a42d8 to 8ac11a5 Compare October 16, 2024 19:24
@neekolas neekolas marked this pull request as ready for review October 16, 2024 19:35
@neekolas neekolas requested a review from a team as a code owner October 16, 2024 19:35
@neekolas neekolas force-pushed the 10-16-validate_published_envelope_topics branch from 8ac11a5 to 16c1438 Compare October 16, 2024 19:38
pkg/envelopes/client.go Outdated Show resolved Hide resolved
@neekolas neekolas force-pushed the 10-16-validate_published_envelope_topics branch from 16c1438 to b58a961 Compare October 17, 2024 23:05
Copy link
Contributor Author

neekolas commented Oct 18, 2024

Merge activity

  • Oct 17, 5:18 PM PDT: A user started a stack merge that includes this pull request via Graphite.
  • Oct 17, 5:18 PM PDT: A user merged this pull request with Graphite.

@neekolas neekolas merged commit ccda751 into main Oct 18, 2024
7 checks passed
@neekolas neekolas deleted the 10-16-validate_published_envelope_topics branch October 18, 2024 00:18
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.

2 participants