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

Feature/ws init func (rebased on master) #406

Merged
merged 12 commits into from
Oct 5, 2022

Conversation

argoyle
Copy link
Contributor

@argoyle argoyle commented Sep 4, 2022

This is a rebased version of #275

package subscription

// InitPayload is a structure that is parsed from the websocket init message payload.
type InitPayload map[string]interface{}
Copy link
Member

Choose a reason for hiding this comment

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

This assumes that InitPayload is a map. What if it's any other valid JSON? Could it be an Array or a string, bool or number?

I see no other issues with this PR, but would suggest that you rewrite this to accept any kind of JSON, or maybe even a binary payload. What are your thoughts on this? (just making it a bit more generic) Nothing against the Authorization helper method. It can live in some way, but the init handler should be unopinionated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looking at the type of connectionParams in subscription-transport-ws it seems to always be a map:

export declare type ConnectionParams = {
    [paramName: string]: any;
};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, coming back to this one finally. I changed InitPayload to be a json.RawMessage and then trying to unmarshal it when getting a string value. That should make it handle any kind of value, right?

argoyle and others added 5 commits October 1, 2022 09:34
Changing to json.RawMessage and deferring parsing until actually trying to read a value.
* chore: add windows to build matrix
chore: use math.MaxInt instead of byte shift

* chore: try to fix test for windows

* chore: use goldie v2
chore: replace line ending under windows goldie assertions

* chore: line ending for goldie win

* chore: skip goldie on a windows

* fix imports pkg

* chore: fix benchmark crash
chore: make defer test more predictable

* chore: fix stream array test

* chore: update workflow

* chore: disable stream test

* enable goldie

* revert "chore: fix benchmark crash"
disable streaming tests on windows

* chore: use normalize line ending in some tests

* chore: ignore kafka tests on windows

* chore: add step to configure git line endings in ci

* chore: skip sub+federation test for windows pipeline

* skip all imports test on win

* Revert "chore: use normalize line ending in some tests"

* chore: cleanup

* chore: use separate fixtures for win in imports pkg

* chore: fix build

* chore: fix imports test on win

* chore: cleanup
@jensneuse
Copy link
Member

@spetrunin can you give this PR a review? I think it looks fine.

@devsergiy devsergiy merged commit a4fdba3 into wundergraph:master Oct 5, 2022
@devsergiy devsergiy mentioned this pull request Oct 5, 2022
@argoyle argoyle deleted the feature/ws-init-func branch October 7, 2022 07:23
@argoyle
Copy link
Contributor Author

argoyle commented Oct 7, 2022

🎉 Thanks!

pvormste referenced this pull request in TykTechnologies/graphql-go-tools Jun 23, 2023
* Add WebSocket init function for authorizing connect

* fix handleInit

* fix subscripton tests

* fix code style

* delete debug info

* chore: change InitPayload to handle any kind of data

Changing to json.RawMessage and deferring parsing until actually trying to read a value.

* change ci triggers

* chore: add windows to build matrix (#418)

* chore: add windows to build matrix
chore: use math.MaxInt instead of byte shift

* chore: try to fix test for windows

* chore: use goldie v2
chore: replace line ending under windows goldie assertions

* chore: line ending for goldie win

* chore: skip goldie on a windows

* fix imports pkg

* chore: fix benchmark crash
chore: make defer test more predictable

* chore: fix stream array test

* chore: update workflow

* chore: disable stream test

* enable goldie

* revert "chore: fix benchmark crash"
disable streaming tests on windows

* chore: use normalize line ending in some tests

* chore: ignore kafka tests on windows

* chore: add step to configure git line endings in ci

* chore: skip sub+federation test for windows pipeline

* skip all imports test on win

* Revert "chore: use normalize line ending in some tests"

* chore: cleanup

* chore: use separate fixtures for win in imports pkg

* chore: fix build

* chore: fix imports test on win

* chore: cleanup

Co-authored-by: chedom <domanchukits@gmail.com>
Co-authored-by: Jens Neuse <jens.neuse@gmx.de>
Co-authored-by: Sergey Petrunin <spetrunin@users.noreply.github.com>
pvormste referenced this pull request in TykTechnologies/graphql-go-tools Jun 30, 2023
* Add WebSocket init function for authorizing connect

* fix handleInit

* fix subscripton tests

* fix code style

* delete debug info

* chore: change InitPayload to handle any kind of data

Changing to json.RawMessage and deferring parsing until actually trying to read a value.

* change ci triggers

* chore: add windows to build matrix (#418)

* chore: add windows to build matrix
chore: use math.MaxInt instead of byte shift

* chore: try to fix test for windows

* chore: use goldie v2
chore: replace line ending under windows goldie assertions

* chore: line ending for goldie win

* chore: skip goldie on a windows

* fix imports pkg

* chore: fix benchmark crash
chore: make defer test more predictable

* chore: fix stream array test

* chore: update workflow

* chore: disable stream test

* enable goldie

* revert "chore: fix benchmark crash"
disable streaming tests on windows

* chore: use normalize line ending in some tests

* chore: ignore kafka tests on windows

* chore: add step to configure git line endings in ci

* chore: skip sub+federation test for windows pipeline

* skip all imports test on win

* Revert "chore: use normalize line ending in some tests"

* chore: cleanup

* chore: use separate fixtures for win in imports pkg

* chore: fix build

* chore: fix imports test on win

* chore: cleanup

Co-authored-by: chedom <domanchukits@gmail.com>
Co-authored-by: Jens Neuse <jens.neuse@gmx.de>
Co-authored-by: Sergey Petrunin <spetrunin@users.noreply.github.com>
pvormste added a commit to TykTechnologies/graphql-go-tools that referenced this pull request Jul 3, 2023
- sync wg commit:
wundergraph#406

---------

Co-authored-by: chedom <domanchukits@gmail.com>
Co-authored-by: Jens Neuse <jens.neuse@gmx.de>
Co-authored-by: Sergey Petrunin <spetrunin@users.noreply.github.com>
pvormste added a commit to TykTechnologies/graphql-go-tools that referenced this pull request Jul 3, 2023
- sync wg commit:
wundergraph#406

---------

Co-authored-by: chedom <domanchukits@gmail.com>
Co-authored-by: Jens Neuse <jens.neuse@gmx.de>
Co-authored-by: Sergey Petrunin <spetrunin@users.noreply.github.com>
pvormste added a commit to TykTechnologies/graphql-go-tools that referenced this pull request Jul 3, 2023
- sync wg commit:
wundergraph#406

---------

Co-authored-by: chedom <domanchukits@gmail.com>
Co-authored-by: Jens Neuse <jens.neuse@gmx.de>
Co-authored-by: Sergey Petrunin <spetrunin@users.noreply.github.com>
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.

None yet

4 participants