Conversation
There was a problem hiding this comment.
Pull request overview
Restores the previously removed ExceptionFactory in the JVM runtime, updates diagnostics packaging by moving ViolationText to the top-level validation package, removes deprecated runtime option types, and bumps project/dependency versions accordingly.
Changes:
- Reintroduce
ExceptionFactoryand add tests (with stub implementations) to validate its behavior. - Move
ViolationTextout of thediagssubpackage and adjust call sites/imports. - Remove deprecated
io.spine.validation.optionruntime option types and update dependency versions/reports.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| version.gradle.kts | Bumps published Validation SDK version to 2.0.0-SNAPSHOT.407. |
| tests/runtime/src/test/kotlin/io/spine/validation/ValidateUtilitySpec.kt | Removes outdated ViolationText import after package move. |
| pom.xml | Updates project and dependency versions (incl. Spine/Compiler/KSP/Protobuf). |
| jvm-runtime/src/test/kotlin/io/spine/validation/given/ExceptionFactoryStubs.kt | Adds stub ExceptionFactory/exception used by tests. |
| jvm-runtime/src/test/kotlin/io/spine/validation/diags/ViolationTextSpec.kt | Updates import to new ViolationText package. |
| jvm-runtime/src/test/kotlin/io/spine/validation/ExceptionFactorySpec.kt | Adds coverage asserting exception/error contents produced by ExceptionFactory. |
| jvm-runtime/src/main/java/io/spine/validation/option/ValidatingOption.java | Removes deprecated runtime option API. |
| jvm-runtime/src/main/java/io/spine/validation/option/Required.java | Removes deprecated runtime option implementation. |
| jvm-runtime/src/main/java/io/spine/validation/option/FieldValidatingOption.java | Removes deprecated runtime option base type. |
| jvm-runtime/src/main/java/io/spine/validation/option/AlwaysRequired.java | Removes deprecated runtime option specialization. |
| jvm-runtime/src/main/java/io/spine/validation/diags/package-info.java | Removes diags package metadata as the package is eliminated. |
| jvm-runtime/src/main/java/io/spine/validation/ViolationText.java | Moves ViolationText into io.spine.validation. |
| jvm-runtime/src/main/java/io/spine/validation/ValidationException.java | Adjusts ViolationText usage and adds serialVersionUID. |
| jvm-runtime/src/main/java/io/spine/validation/ExceptionFactory.java | Restores the abstract factory for building validation exceptions/errors. |
| dependencies.md | Updates generated dependency/license report to the new versions. |
| buildSrc/src/main/kotlin/io/spine/dependency/local/Validation.kt | Bumps referenced Validation dependency version to ...406. |
| buildSrc/src/main/kotlin/io/spine/dependency/local/ToolBase.kt | Bumps ToolBase versions to ...375. |
| buildSrc/src/main/kotlin/io/spine/dependency/local/Compiler.kt | Bumps Compiler fallback versions to ...041. |
| buildSrc/src/main/kotlin/io/spine/dependency/local/Base.kt | Bumps Spine Base versions to ...386. |
| buildSrc/src/main/kotlin/io/spine/dependency/build/Ksp.kt | Bumps KSP version to 2.3.6. |
Comments suppressed due to low confidence (1)
jvm-runtime/src/main/java/io/spine/validation/ViolationText.java:28
- Moving
ViolationTextfromio.spine.validation.diagstoio.spine.validationis a binary/source breaking change for any consumers importing the old package. If backward compatibility is still desired for this SNAPSHOT line, consider keeping a deprecated shim inio.spine.validation.diags(e.g., a delegating wrapper with the same static API) so existing code continues to compile while users migrate.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This reverts commit 4e01e27.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 20 changed files in this pull request and generated 5 comments.
Files not reviewed (1)
- .idea/inspectionProfiles/Project_Default.xml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| package io.spine.validation.given | ||
|
|
||
| import com.google.protobuf.Message |
There was a problem hiding this comment.
com.google.protobuf.Message is imported but never used in this test stub, which can fail the build if Kotlin warnings are treated as errors. Please remove the unused import.
| import com.google.protobuf.Message |
| * the type of the {@code Message}, typically it would be a grouping interface such as | ||
| * {@link io.spine.base.EventMessage} or {@link io.spine.base.CommandMessage} | ||
| * @param <C> | ||
| * the type of the {@linkplain io.spine.type.MessageClass} of {@code |M|} |
There was a problem hiding this comment.
Javadoc for type parameter C contains {@code |M|}, which looks like a formatting artifact and will render incorrectly. Replace it with the intended reference (e.g. {@code M}) so the generated docs are correct.
| * the type of the {@linkplain io.spine.type.MessageClass} of {@code |M|} | |
| * the type of the {@linkplain io.spine.type.MessageClass} of {@code M} |
| * an invalid event message | ||
| * @param violations | ||
| * constraint violations for the event message |
There was a problem hiding this comment.
Constructor parameter docs refer to an “event message”, but ExceptionFactory is generic over any invalid Message type (including commands). Please update the wording to avoid misleading API docs.
| * an invalid event message | |
| * @param violations | |
| * constraint violations for the event message | |
| * an invalid message | |
| * @param violations | |
| * constraint violations for the message |
| protected abstract Map<String, Value> getMessageTypeAttribute(M message); | ||
|
|
||
| /** | ||
| * Defines the way to create an instance of exception basing on the source {@code Message}, |
There was a problem hiding this comment.
The phrase “create an instance of exception basing on…” is grammatically incorrect and makes the Javadoc harder to read. Please reword it (e.g. “based on”).
| * Defines the way to create an instance of exception basing on the source {@code Message}, | |
| * Defines how to create an exception instance based on the source {@code Message}, |
| <inspection_tool class="FieldNamingConvention" enabled="true" level="WARNING" enabled_by_default="true"> | ||
| <extension name="ConstantNamingConvention" enabled="false"> | ||
| <option name="m_regex" value="[A-Z_\d]*" /> | ||
| <option name="m_minLength" value="5" /> | ||
| <option name="m_maxLength" value="32" /> | ||
| </extension> | ||
| <extension name="InstanceVariableNamingConvention" enabled="true"> | ||
| <option name="m_regex" value="[a-z][A-Za-z\d]*" /> | ||
| <option name="m_minLength" value="2" /> | ||
| <option name="m_maxLength" value="32" /> | ||
| </extension> | ||
| </inspection_tool> |
There was a problem hiding this comment.
This PR is primarily about restoring ExceptionFactory and related validation types, but it also changes the IntelliJ inspection profile (e.g., adds FieldNamingConvention). If these IDE settings changes are not strictly required for the restoration, consider moving them to a separate PR to keep review history focused.
This PR restores the
ExceptionFactoryclass which was accidentally deleted in one of the previous PRs. This PR also adds tests to ensure that this abstract class is used via stub implementation.Other notable changes
ViolationTextclass was moved out of thediagssubpackage to the upper level so that we don't have the one-class package.io.spine.optionpackage were removed.ExceptionFactory.getMessageTypeAttribute()now accepts parameter of the generic typeMinstead of justMessage.