Skip to content

Make the Java validation extendable #215

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 53 commits into from
May 2, 2025

Conversation

yevhenii-nadtochii
Copy link
Contributor

@yevhenii-nadtochii yevhenii-nadtochii commented Apr 24, 2025

This PR makes the validation library extendable for Java users.

Now, users can create policies, views and Java generators for their custom Protobuf options.

ValidationOption interface

Users can create their own providers of policies, views and Java generators by extending from the ValidationOption interface, and marking their implementation with @AutoService(ValidationOption::class).

At the creation of JavaValidationPlugin, all implementations of ValidationOption are loaded and their views, policies and generators are being added to the plugin.

Generator dependencies

Previously, all generators were created right within a member of a bounded context. JavaValidationRenderer created them passing all dependencies they needed, including those provided by the context.

Now, instances of generators must be created before they get into the context. They are firstly passed to ValidationOption, and later bypassed to the context member.

As for now, there are two dependencies that can be provided only by a member of a bounded context:

  1. Querying – from our experience, every code generator needs this class to be able to fetch views, upon which the generator operators. Again, custom generators are created outside the context member. Thus, Querying must be injected after the generator gets into the context. For this, OptionGenerator is now an abstract class that has lateinit var querying: Querying populated later by the renderer.
  2. Some generators need EmptyFieldCheckJavaValueConverterTypeSystem. The generators use EmptyFieldCheck trait to check arbitrary Fields for emptiness, which introduces a transitive dependency on TypeSystem, which is a member property. We can handle TypeSystem just as Querying and allow custom options to utilize EmptyFieldCheck as is OR address Update EmptyFieldCheck to avoid usage of JavaValueConverter #199 because this class can be re-implemented without TypeSystem dependency. ProtoData events now have everything needed for codegen, the lacking information is fetched by policies. At the stage of codegen, the type system must not be used conceptually. So, I suggest addressing the mentioned issue instead of bypassing TypeSystem to the generators.

Represent current options as a set of ValidationOption implementations

In general, it looks possible. Each option now is a combination of a generator + view + policy.

But having them as a set of ValidationOptions breaks our separation of language-agnostic model and language-specific generators. This separation exists on the level of the validation library, but it is not reflected in the ValidationOption interface, which is Java-centric.

:java-api module

All re-usable members of the :java and :model modules now reside in :java-api module. The ValidationOption interface is also part of the API module. To implement a custom option, only the API module is needed.

MoneyValidationPlugin

MoneyValidationPlugin that handled a custom (currency) option with Rules became CurrencyOption.

Now, it is represented with a generator, a policy and a view.

@yevhenii-nadtochii yevhenii-nadtochii self-assigned this Apr 24, 2025
Copy link

codecov bot commented Apr 24, 2025

Codecov Report

Attention: Patch coverage is 5.92105% with 143 lines in your changes missing coverage. Please review.

Project coverage is 32.04%. Comparing base (b6578d4) to head (1611c3e).
Report is 54 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #215      +/-   ##
============================================
- Coverage     32.65%   32.04%   -0.62%     
- Complexity      236      238       +2     
============================================
  Files           119      128       +9     
  Lines          2811     2896      +85     
  Branches        205      208       +3     
============================================
+ Hits            918      928      +10     
- Misses         1841     1916      +75     
  Partials         52       52              
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@yevhenii-nadtochii yevhenii-nadtochii marked this pull request as ready for review April 24, 2025 16:14
@yevhenii-nadtochii
Copy link
Contributor Author

yevhenii-nadtochii commented Apr 24, 2025

@armiol @alexander-yevsyukov PTAL

As for now, I've made public only members that I needed to implement CurrnecyOption. This is the minimum that must be public. I believe we should open everything from :java besides the generator classes (they are final anyway), so that the present extensions and functions could be re-used in custom generators.

Regarding extension points. We have one for both language-agnostic model classes (policies, views) AND Java-specific option generators - CustomOption. Within the validation library itself, it works a bit differently. To be conceptually in-sync with the validation library, we must have two extension points: Java-specific (would receive custom generators) and model-specific (would receive custom context components).

This way, there are three steps:

  1. Add custom Proto option (create definition and register it).
  2. Implement the option model (usually, with a policy and a view).
  3. Implement generators for languages of interest (Java).

Copy link
Collaborator

@armiol armiol left a comment

Choose a reason for hiding this comment

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

@yevhenii-nadtochii please see the first-round comments.

I also think we need a separate module (and artifact) which clearly declares the public API for SPI users. Once this is done, we will need to update the documentation with this regard, and mark some types as @SPI.

As discussed, we will meet today on the call, and talk over the remaining points.

@@ -0,0 +1,59 @@
/*
* Copyright 2024, TeamDev. All rights reserved.
Copy link
Collaborator

Choose a reason for hiding this comment

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

/**
* The Java class of the option [view][View].
*/
public val view: Class<out View<*, *, *>>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's allow having multiple view and policy.

@@ -90,5 +96,5 @@ public abstract class ValidationPlugin(renderers: List<Renderer<*>> = emptyList(
SetOncePolicy(),
IfSetAgainPolicy(),
RequirePolicy()
)
) + policies
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please review this file for code formatting.

@@ -51,7 +52,12 @@ import io.spine.validation.required.RequiredPolicy
* The concrete implementations should provide [renderers], which implement
* these constraints for a specific programming language.
*/
public abstract class ValidationPlugin(renderers: List<Renderer<*>> = emptyList()) : Plugin(
public abstract class ValidationPlugin(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Having this type as public is now confusing. Given we don't suggest that end-users create their own plugins, but rather extend via CustomOption, this type should be @Internal at the very least.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was public before, I haven't changed its visibility.

It is public for consumers of the :model module, which are language-specific plugins.

/**
* Injects [Querying] into this instance of [OptionGenerator].
*/
internal fun inject(querying: Querying) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's mark this @Internal.

@yevhenii-nadtochii yevhenii-nadtochii changed the base branch from master to remove-rule-classes April 30, 2025 13:47
@armiol
Copy link
Collaborator

armiol commented May 1, 2025

@yevhenii-nadtochii is this a draft or something ready for reviewing?

Base automatically changed from remove-rule-classes to master May 2, 2025 08:28
@yevhenii-nadtochii
Copy link
Contributor Author

@yevhenii-nadtochii is this a draft or something ready for reviewing?

This is a draft.

@yevhenii-nadtochii yevhenii-nadtochii marked this pull request as ready for review May 2, 2025 11:41
@yevhenii-nadtochii
Copy link
Contributor Author

@armiol @alexander-yevsyukov PTAL

Copy link
Collaborator

@armiol armiol left a comment

Choose a reason for hiding this comment

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

@yevhenii-nadtochii LGTM.

In the following rounds we'll need to review the newly created module for clarity. However, it is easier to do in the IDE rather than via the GitHub PR review.

In the meantime, could you please append the repo-level README.md with some notes on how to provide own custom options.

@yevhenii-nadtochii yevhenii-nadtochii merged commit 25d2a2e into master May 2, 2025
7 checks passed
@yevhenii-nadtochii yevhenii-nadtochii deleted the make-codegen-extendable branch May 2, 2025 12:39
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