[OPIK-121] Allow multiline in dataset description

[thiagohora]

Details

Allow multiline in the dataset description

Issues

OPIK-121

Testing

Add dataset with multiline description
Add project with multiline description

[andrescrz]

This started as a quick validation at the beginning of the project, but I believe the regex is starting to get out of hand and this bug is a proof of it. Also, because this already has many references in the code, so we should make sure it's endurable.

I think it's time to create a validation annotation, like the other ones that you've already created in the service.

It's trivial to check if null or not black with a validator class.

You could also extend it to work on collections, but no need to do it now.

You only need to be careful with our related PODAM utils. Basically, you need to make sure that:

1. Fields with the new annotation are properly randomly generated.
2. You should consider cleaning up the current PODAM utils for the Pattern annotation.

[thiagohora]

This started as a quick validation at the beginning of the project, but I believe the regex is starting to get out of hand and this bug is a proof of it. Also, because this already has many references in the code, so we should make sure it's endurable.

I think it's time to create a validation annotation, like the other ones that you've already created in the service.

It's trivial to check if null or not black with a validator class.

You could also extend it to work on collections, but no need to do it now.

You only need to be careful with our related PODAM utils. Basically, you need to make sure that:

1. Fields with the new annotation are properly randomly generated.
2. You should consider cleaning up the current PODAM utils for the Pattern annotation.

I understand your concern, and it's valid. However, there is a downside to using custom annotations, and the issue is that custom annotations are not documented in OpenApi docs. I added tests to all scenarios, and hopefully, those should be enough to give us confidence. Regarding documenting it, I can do it. I also usually use some tools to explain it.

https://zzzcode.ai/regex/explain?id=94d92613-3632-48e6-b80a-60ddfa6469b3

https://regexper.com/#%5E%5Cs*%28%5CS.*%5CS%7C%5CS%29%5Cs*%24

[andrescrz]

This started as a quick validation at the beginning of the project, but I believe the regex is starting to get out of hand and this bug is a proof of it. Also, because this already has many references in the code, so we should make sure it's endurable.
I think it's time to create a validation annotation, like the other ones that you've already created in the service.
It's trivial to check if null or not black with a validator class.
You could also extend it to work on collections, but no need to do it now.
You only need to be careful with our related PODAM utils. Basically, you need to make sure that:

1. Fields with the new annotation are properly randomly generated.
2. You should consider cleaning up the current PODAM utils for the Pattern annotation.

I understand your concern, and it's valid. However, there is a downside to using custom annotations, and the issue is that custom annotations are not documented in OpenApi docs. I added tests to all scenarios, and hopefully, those should be enough to give us confidence. Regarding documenting it, I can do it. I also usually use some tools to explain it.

https://zzzcode.ai/regex/explain?id=94d92613-3632-48e6-b80a-60ddfa6469b3

https://regexper.com/#%5E%5Cs*%28%5CS.*%5CS%7C%5CS%29%5Cs*%24

For pattern validation annotation, the OpenAPI docs aren't very descriptive either. They just show the raw regex string^(?!\s*$).+ which isn't very significative either.

If this is a concern, a quick solution is just to add a @Schema(description tag to the fields that apply, and that's not different for Pattern or the new annotation.

As longer term solution, maybe there's a way to integrate the custom annotation with swagger.

[thiagohora]

This started as a quick validation at the beginning of the project, but I believe the regex is starting to get out of hand and this bug is a proof of it. Also, because this already has many references in the code, so we should make sure it's endurable.
I think it's time to create a validation annotation, like the other ones that you've already created in the service.
It's trivial to check if null or not black with a validator class.
You could also extend it to work on collections, but no need to do it now.
You only need to be careful with our related PODAM utils. Basically, you need to make sure that:

1. Fields with the new annotation are properly randomly generated.
2. You should consider cleaning up the current PODAM utils for the Pattern annotation.

I understand your concern, and it's valid. However, there is a downside to using custom annotations, and the issue is that custom annotations are not documented in OpenApi docs. I added tests to all scenarios, and hopefully, those should be enough to give us confidence. Regarding documenting it, I can do it. I also usually use some tools to explain it.
https://zzzcode.ai/regex/explain?id=94d92613-3632-48e6-b80a-60ddfa6469b3
https://regexper.com/#%5E%5Cs*%28%5CS.*%5CS%7C%5CS%29%5Cs*%24

For pattern validation annotation, the OpenAPI docs aren't very descriptive either. They just show the raw regex string^(?!\s*$).+ which isn't very significative either.

If this is a concern, a quick solution is just to add a @Schema(description tag to the fields that apply, and that's not different for Pattern or the new annotation.

As longer term solution, maybe there's a way to integrate the custom annotation with swagger.

To me, the issue is not just description but code generation. The OpenApi code generator supports patterns, which allow the code to be validated on the client side. It's not a must, but I believe it's a good property to have. I think this is a good reason to keep it, but if want I can remove it. I just think with the regex explainers, it's pretty easy to understand it

[andrescrz]

This started as a quick validation at the beginning of the project, but I believe the regex is starting to get out of hand and this bug is a proof of it. Also, because this already has many references in the code, so we should make sure it's endurable.
I think it's time to create a validation annotation, like the other ones that you've already created in the service.
It's trivial to check if null or not black with a validator class.
You could also extend it to work on collections, but no need to do it now.
You only need to be careful with our related PODAM utils. Basically, you need to make sure that:

1. Fields with the new annotation are properly randomly generated.
2. You should consider cleaning up the current PODAM utils for the Pattern annotation.

I understand your concern, and it's valid. However, there is a downside to using custom annotations, and the issue is that custom annotations are not documented in OpenApi docs. I added tests to all scenarios, and hopefully, those should be enough to give us confidence. Regarding documenting it, I can do it. I also usually use some tools to explain it.
https://zzzcode.ai/regex/explain?id=94d92613-3632-48e6-b80a-60ddfa6469b3
https://regexper.com/#%5E%5Cs*%28%5CS.*%5CS%7C%5CS%29%5Cs*%24

For pattern validation annotation, the OpenAPI docs aren't very descriptive either. They just show the raw regex string^(?!\s*$).+ which isn't very significative either.
If this is a concern, a quick solution is just to add a @Schema(description tag to the fields that apply, and that's not different for Pattern or the new annotation.
As longer term solution, maybe there's a way to integrate the custom annotation with swagger.

To me, the issue is not just description but code generation. The OpenApi code generator supports patterns, which allow the code to be validated on the client side. It's not a must, but I believe it's a good property to have. I think this is a good reason to keep it, but if want I can remove it. I just think with the regex explainers, it's pretty easy to understand it

It's a fair point, but I don't think those client side validations are crucial. At least not at the expense of other problems in the backend. I'd like to double check if the Fern generation of a Python client really considers this.

Anyway, I'm going to take a look at the second revision.

[andrescrz]

As discussed, this can go through as you improved it in the next revision:

1. Added javadoc.
2. Added additional tests.

And also because it's the simpler solution at the moment.

If this causes any other issue in the future (bug or user misunderstanding), we'll explore the approach of generating a custom annotation.

[andrescrz]

Minor: better use assertJ assertions instead of the ones from JUnit.

[thiagohora]

I will address in following PR

[andrescrz]

Minor: new line at the end.

[thiagohora]

Same