Move validation code #1
Conversation
|
@armiol, @alexander-yevsyukov, PTAL. As there are many changed files, in this PR we will only migrate the codebase from |
armiol
left a comment
armiol
left a comment
There was a problem hiding this comment.
@dmdashenkov please see my comments.
Also, I am not sure which version this artifact has. I was looking for version.gradle.kts, but found none.
| // Represents a repeated `Value`. | ||
| ListValue list_value = 8; | ||
|
|
||
| // Represents a repeated `Value`. |
There was a problem hiding this comment.
ListValue is indeed a repeated element, but this is MapValue. Therefore, I would assume it represents the values of Protobuf map<.., ..> collection.
| @NotNull | ||
| private static Value primitiveValue(Type type) { | ||
| PrimitiveType primitiveType = type.getPrimitive(); | ||
| if (primitiveType == PrimitiveType.PT_UNKNOWN || primitiveType == PrimitiveType.UNRECOGNIZED) { |
There was a problem hiding this comment.
My IDEA tells me this line exceeds the max width.
Perhaps, a static import is missing?
| .setBytesValue(ByteString.EMPTY) | ||
| .vBuild(); | ||
| } | ||
| throw Exceptions.newIllegalStateException( |
There was a problem hiding this comment.
I would import this method statically.
|
|
||
| @Subscribe | ||
| void on(SimpleRuleAdded event) { | ||
| Rule roc = Rule |
There was a problem hiding this comment.
I am having hard time imagining why this is roc :)
I would understand if that's a rot ("rule of thumb"), but with "roc" you got me here and in the next method as well.
| * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. | ||
| */ | ||
|
|
||
| @CheckReturnValue |
There was a problem hiding this comment.
Please document the package.
BTW, do we really want to re-use this package for events, projections etc generated from Proto definitions? I would go for ... .event, ... .command — as we usually do for Bounded contexts.
|
@alexander-yevsyukov, PTAL. |
alexander-yevsyukov
left a comment
alexander-yevsyukov
left a comment
There was a problem hiding this comment.
Please see my comments.
| @React | ||
| public EitherOf2<SimpleRuleAdded, Nothing> whenever(@External FieldOptionDiscovered event) { | ||
| Option option = event.getOption(); | ||
| if (isRequired(option)) { |
There was a problem hiding this comment.
Please invert the if condition. It would reduce the nesting.
| .stream() | ||
| .filter(f -> f.getName().equals(event.getField())) | ||
| .findAny() | ||
| .orElseThrow(() -> newIllegalArgumentException( |
There was a problem hiding this comment.
Please extract creation on this and previous exception. It would make this complex method more readable.
| @SuppressWarnings({"DuplicateStringLiteralInspection", "RedundantSuppression"}) | ||
| // Duplication in generated code. | ||
| SimpleRule rule = SimpleRule.newBuilder() | ||
| .setErrorMessage("Field must be set.") |
There was a problem hiding this comment.
Can we wrap this block to the left, please.
| .setOtherValue(defaultValue) | ||
| .vBuild(); | ||
| return SimpleRuleAdded.newBuilder() | ||
| .setType(field.getDeclaringType()) |
There was a problem hiding this comment.
Can we wrap this block to the left, please.
| */ | ||
| final class RequiredRulePolicy extends Policy<FieldOptionDiscovered> { | ||
|
|
||
| @NotNull |
There was a problem hiding this comment.
Can we do without this annotation?
| public final class SpineOptionsProvider implements OptionsProvider { | ||
|
|
||
| @Override | ||
| public void dumpTo(@NotNull ExtensionRegistry registry) { |
There was a problem hiding this comment.
Please use annotations from Checker Framework.
armiol
left a comment
armiol
left a comment
There was a problem hiding this comment.
@dmdashenkov please see my comment, as per our discussion.
| import static io.spine.util.Exceptions.newIllegalStateException; | ||
|
|
||
| /** | ||
| * A factory of {@link Value}s representing default states of Protobuf message fields. |
There was a problem hiding this comment.
As discussed vocally, let's expand this documentation piece.
In particular, we need to describe the difference between this factory and io.spine.protobuf.Messages (which on the surface looks more promising, equipped with some caching strategy).
…ional.empty()` instead of throwing
|
@armiol, PTAL. |
In this PR we move the validation codegen from an example repository to this new dedicated repository.
Validation logic is now split into
model— a language-agnostic base, which assembles validation rules, andjava— a Java-specific implementation, which generates Java code based onmodel.