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

Validation Process rethink, part 3 #1135

Closed
XVincentX opened this issue Apr 25, 2020 · 7 comments
Closed

Validation Process rethink, part 3 #1135

XVincentX opened this issue Apr 25, 2020 · 7 comments
Labels

Comments

@XVincentX
Copy link
Contributor

XVincentX commented Apr 25, 2020

It turns out that validation is a very complicated part of Prism and, although we have reworked it several times we still haven't got that right.

With the recent change in the way the proxy works for invalid requests in case the --errors flag is provided, we can really likely extract the inputValidation parts from proxy and mock (they are now duplicated, with almost the same code) and make the inputValidation step return a real Either<NonEmptyArray<IPrismDiagnostic>,InputPart> and halt the request processing there.

It's a very good opportunity to make the code easier to read and simplify some things in Prism.

The only big problem I found while trying to tame this on my own is that the Core package does not allow for any manipulation of stuff because it's too generic. The benefits of having the core package is starting to be a little bit questionable now and it might make sense to break that and transfer all the code in prism-http directly.

@karol-maciaszek
Copy link
Contributor

karol-maciaszek commented Apr 30, 2020

Either<NonEmptyArray,InputPart>

That makes it impossible to pass through the erroneous request in proxy mode, right?

The only big problem I found while trying to tame this on my own is that the Core package does not allow for any manipulation of stuff because it's too generic. The benefits of having the core package is starting to be a little bit questionable now and it might make sense to break that and transfer all the code in prism-http directly.

This is actually a big question. Do we have plans to support other protocols? IMO it would be so cool to do support.

@XVincentX
Copy link
Contributor Author

That makes it impossible to pass through the erroneous request in proxy mode

Most likely we can still make it happen by returning a Right in some special cases, that part does not concern me.

Do we have plans to support other protocols? IMO it would be so cool to do support.

I think we do, but when's this happening?

  • I think the support for another protocol is not happening anytime soon (>= 1year)
  • Duplication is cheaper than having the wrong abstraction, and core might be a wrong abstraction.

Let's think about it without committing to anything specific yet and we'll go from there.

@karol-maciaszek
Copy link
Contributor

We could take a rough look at other protocols to know what we are dealing with. I have a feeling that we have gone too far with core being compatible with HTTP. That may answer the question of whether there is a good abstraction.

@XVincentX
Copy link
Contributor Author

Definitely more to ponder!

@XVincentX XVincentX changed the title Validation Process rethink, part 2 Validation Process rethink, part 3 May 14, 2020
@philsturgeon
Copy link
Contributor

philsturgeon commented Sep 14, 2020

I think the idea was to consider supporting other protocols such as those in AsyncAPI right, like MQTT or AMQP? If so, it's going to be wildly different. Let's keep focused on HTTP and we can consider what abstractions are necessary when we try to focus on those protocols, but for now it seems like a premature abstraction thats making it hard to maintain functionality for the one protocol we actually support.

@XVincentX
Copy link
Contributor Author

@philsturgeon You're on the wrong rail — this issue has nothing to do with supporting other protocol.

To keep it short, at the moment we're parsing a part of the http request twice because we're losing the information along the chain of the request processing.

The proposal here would modify the way we use Either to keep carrying such information — and reuse it when necessary.

It's not a big deal at the moment, but it would better for sure

This was referenced Sep 21, 2020
@chohmann
Copy link
Contributor

Closing this issue out. We may address this in future, but will open up a new issue at that time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants