Add an option to limit number of errors returned

[bc-dima-pasieka]

What?

Add an option to limit the number of errors returned.

Why?

There is a possibility that the lack of limits could be abused to induce memory pressure in the server/service handling the request and can lead to the potential denial of service (i.e. no DDoS needed).

[yanns]

thanks for this PR. 💯
There are some binary breaking changes (default values are set at caller site).

I think that we add some methods to avoid some of those

• apply(rules: List[ValidationRule]): RuleBasedQueryValidator in the companion object
• a similar apply in ValidationContext

Would you take care of it or should I?

[yanns]

Making errorsLimit immutable as ValidationContext is a call.

Suggested change

	errorsLimit: Option[Int] = None) {
	val errorsLimit: Option[Int] = None) {

[bc-dima-pasieka]

omitting val will make it private to the class since it's not a case class, won't it? 🤔 Do we want to make it public?

[yanns]

oh yes sorry. Keep it like this.

[bc-dima-pasieka]

apply(rules: List[ValidationRule]): RuleBasedQueryValidator in the companion object

It looks like it will be a breaking change if we do it this way - we will not be able to call new RuleBasedQueryValidator(newRules) anymore (only RuleBasedQueryValidator(newRules))

object RuleBasedQueryValidator {
  def apply(rules: List[ValidationRule]): RuleBasedQueryValidator = RuleBasedQueryValidator(rules, None)
}

The same with ValidationContext.

🤔 Would we be fine introducing breaking change?

[yanns]

RuleBasedQueryValidator is more likely being called from user.
Sure that we cannot keep the new RuleBasedQueryValidator(rules), but let's try to keep the apply method.

For ValidationContext, I think it's ok to make a breaking change. It's unlikely that users initiate it themselves.
I'll add this to the release notes.

[bc-dima-pasieka]

@yanns let me know if it looks better now 🙌

[yanns]

It's looking good, thanks!
I'll take care of adding exceptions for the rest of binary incompatibilities,

[yanns]

I propose to set a default limit to make default usage safe: aab5f5b
WDYT? 10 errors are sufficient?

[yanns]

I propose to set a default limit to make default usage safe: aab5f5b
WDYT? 10 errors are sufficient?

[bc-dima-pasieka]

10 sounds like a good default value! 👍