Skip to content

Introduce validators in the http binding#435

Merged
jfallows merged 11 commits intoaklivity:feature/schema-registryfrom
attilakreiner:validator-http
Sep 22, 2023
Merged

Introduce validators in the http binding#435
jfallows merged 11 commits intoaklivity:feature/schema-registryfrom
attilakreiner:validator-http

Conversation

@attilakreiner
Copy link
Contributor

Description

Introduce validators in the http binding.

Fixes # (issue)

@attilakreiner attilakreiner marked this pull request as ready for review September 21, 2023 12:04
import io.aklivity.zilla.runtime.binding.http.internal.types.String8FW;
import io.aklivity.zilla.runtime.engine.internal.validator.config.AvroValidatorConfig;
import io.aklivity.zilla.runtime.engine.internal.validator.config.CatalogedConfig;
import io.aklivity.zilla.runtime.engine.internal.validator.config.StringValidatorConfig;
Copy link
Contributor

Choose a reason for hiding this comment

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

In preparation for hoisting out the validators, this should be using a TestValidator instead of specific implementations.

The string validator and avro validator config tests belong closer to those implementations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is coming in PR #440.


AvroValidatorConfigBuilder(
Function<AvroValidatorConfig, T> mapper)
Function<ValidatorConfig, T> mapper)
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand why we did this, but it doesn't feel right for the AvroValidatorConfigBuilder not to build an AvroValdiatorConfig when Function.identity() is passed as the mapper. Instead it will return ValidatorConfig from .build().

Maybe keep this concrete as AvroValidatorConfig, and handle the implicit cast in the AvroValidatorConfig.builder(...) methods by adjusting the mapper before passing in here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's fixed 🤔 pls chk

@jfallows jfallows merged commit fdae4e0 into aklivity:feature/schema-registry Sep 22, 2023
@attilakreiner attilakreiner deleted the validator-http branch September 22, 2023 05:53
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