Closed intech closed 1 year ago
This: https://github.com/moleculerjs/moleculer-web/blob/5c2a1ff3d51b4b74cc063c3a3f3fa1ad75571248/src/index.js#L579 and this: https://github.com/moleculerjs/moleculer-web/blob/5c2a1ff3d51b4b74cc063c3a3f3fa1ad75571248/src/index.js#L584
is this logic required in some cases? Now the correct values come to $params
and when req is passed, they are lost for validation
alias.type == "stream" ? req : req.$params
When streaming the params are placed into $params
. So for validation you need to set something like this:
params: {
$params: {
type: "object", props: {
id: "uuid"
}
}
}
Right?
@AndreMaz yes, you can do a nested check. Question need to split logic for stream? We cannot pass there other parameters except in the url. Fields in multipart are defined differently and stored in meta.$multipart
After setting your example with NATS I see what you mean by "$params
are lost". So what do you suggest? Move $params
into meta
?
@AndreMaz I will suggest an option later. I need to conduct several tests and figure out what changes can be made in order to maintain compatibility and performance.
@intech great. Thank you :+1:
I will update my thoughts on this issue.
I believe that it would be correct to remove stream
from ctx.params
and leave the parameters in ctx.params
universal.
And for stream
use ctx.stream
or ctx.$stream
by analogy with ctx.$req/$res
This solution will allow you to get rid of ctx.meta.$multipart
and transfer their fields to ctx.params
.
This will be a backward compatibility issue. Therefore, it is impossible to do this now.
For example:
// "PUT /upload/:id": "stream:file.save" -> PUT /upload/123
// in handler:
const {id} = ctx.params;
let length = 0;
for await(const chunk of ctx.stream) {
length += chunk.length;
}
return { length, id };
But I think it will be a nice design, universal use of ctx.params
.
It will also solve the tracing problem: https://github.com/moleculerjs/moleculer/issues/777
@icebob we need you
My plan is that I will change the params is a Stream
logic, that we can transfer params besides a stream. In the packet, no need to add a new field because in the stream packets there is a seq
sequence variable. So the idea is that the first packet (with seq: 0
) doesn't transfer stream data, instead transfers the params
. Additional packets send the stream data as before.
It means you can send stream as:
this.broker.call("file.archive", { folder: "data" }, { stream: myStream });
To receive a stream via ctx.stream
(same as you wrote):
const {id} = ctx.params;
let length = 0;
for await(const chunk of ctx.stream) {
length += chunk.length;
}
return { length, id };
But it causes breaking change so it comes in 0.15 only. Or we can turn this new logic on/off with an experimental feature flag which is off by default, so it can be implemented in 0.14, and switch to on in 0.15.
By the way, I'm moving this issue into core repo because it's a common issue, not just moleculer-web.
@icebob if I understand correctly, in this way we will also solve the problem with tracing, since stream will no longer be in the context of the default parameters that are collected?
Yeah, sure.
Implemented in next branch https://github.com/moleculerjs/moleculer/pull/1177
PoC: https://repl.it/@IvanZhuravlev/validationforstream#index.js