[gen] normalized fields with annotations holding original field name #3069

hochgi · 2024-08-30T11:29:13Z

PR ready for review

zio-http-gen/src/test/scala/zio/http/gen/scala/CodeGenSpec.scala

zio-http-gen/src/test/resources/inline_schema_weird_field_names.yaml

hochgi · 2024-08-31T22:17:07Z

zio-http-gen/src/test/resources/ComponentOrderWithAliases.scala

-  @zio.schema.annotation.validate[Int](zio.schema.validation.Validation.greaterThan(0)) quantity: Int,
-  @zio.schema.annotation.validate[Double](zio.schema.validation.Validation.greaterThan(-1.0)) price: Double,
+  @validate[Int](Validation.greaterThan(0)) quantity: Int,
+  @validate[Double](Validation.greaterThan(-1.0)) price: Double,


Not related to current PR, but the float/double validation logic is faulty.
minimum 0 does not mean > -1.0
Since e.g: -0.5 > -1.0, but does not adhere to original minimum requirement.

CC @987Nabil

True. But I think this is unsolvable then. There is no way to translate it to zio-schema correctly right now. We probably need to add new validations to zio schema

@987Nabil in second thought, perhaps you need a change for greaterOrEqualTo, but we can still use java.lang.Math.nextDown (or nextAfter) to get the closest representable floating point number that is smaller (or larger) than the input.

It might be ugly, e.g:

@ java.lang.Math.nextDown(0f) res0: Float = -1.4E-45F

but at least it'll be (effectively) correct.

WDYT?

I think that works work. But I would not write out the number, but generate the expression

Yup...

@validate(Validation.greaterThan(Math.nextDown(0f)))

Would be much better (and understandable) than generating weird epsilons

codecov-commenter · 2024-09-01T10:49:26Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 65.57%. Comparing base (16e2e2f) to head (4424bc5).

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3069   +/-   ##
=======================================
  Coverage   65.57%   65.57%           
=======================================
  Files         157      157           
  Lines       10251    10251           
  Branches     1918     1918           
=======================================
  Hits         6722     6722           
  Misses       3529     3529

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

project/Dependencies.scala

hochgi · 2024-09-01T11:28:03Z

zio-http-gen/src/main/scala/zio/http/gen/openapi/Config.scala

+     * unless original field is defined in the specialReplacements map.
+     */,
+    specialReplacements: Map[String, String], /*
+     * When normalization is enabled, a heuristic parser will attempt to normalize field names.


Map can be used also when auto normalization is not enabled. I should amend the documentation to be clearer

If specialReplacements can be used out side of normalization, does it belong in the normalization config?

I meant of course it's part of normalization. but it overrides auto normalization (i.e: using the parser to construct a different term).
Normalization has 2 modes:

manual replacements (user control exactly what to replace with what)

automatic (a heuristic parser will try to do the best it can)

both fall under normalization.
Perhaps I should rename as: automaticEnabled & manualReplacements?

Would turning the automated off lead to invalid code? Should it be always on? And the manual just be tried first, if the map is not empty?

no, not at all.
These are decoupled.

having e.g. auto off, and replacement on can yield from e.g:

Thing: type: object properties: S/N: type: string format: uuid N_A_M_E: type: String

with only replacing "N_A_M_E" -> "name" something like:

case class Thing ( `S/N`: UUID, name: String )

which is perfectly fine.

zio-http-gen/src/main/scala/zio/http/gen/openapi/EndpointGen.scala

zio-http-gen/src/main/scala/zio/http/gen/openapi/Config.scala

…n for zio#3031 (which will add more annotations). Specifically: use imports insteads of inlined FQCN when adding annotations

* add new configurations needed for normalization of fields

…r less), since previous implementation was slooooooooow. * propagate configuration to use fields normalization * implement & add relevant annotation when field name been normalized * test suite for normalization logic * add test to verify annotations are add correctly

…fields (and imports) when rendering

* Restore test cases fields original order * Reduced normalized test to have only 4 fields (up to 4 fields, order is consistent, since Map1, …, Map4 implementation is consistent between scala versions. More fields aren't guaranteed to have consistent order)

* renamed config normalizations setting to be clearer * moved comments to be scaladoc in configuration (scalafmt kep messing this up, so I've set '//format: off' - should we configure 'docstrings.style = keep' in scalafmt.conf to avoid it messing up nicely formatted scaladocs?)

… on different lines?

also, re-run CI since seems to be affected by a flaky test: Client / ClientSpec / request can be timed out manually while awaiting connection

algora-pbc bot mentioned this pull request Aug 30, 2024

[gen] support for normalized fields #3031

Closed

algora-pbc bot added the 🙋 Bounty claim label Aug 30, 2024

hochgi commented Aug 30, 2024

View reviewed changes

zio-http-gen/src/test/scala/zio/http/gen/scala/CodeGenSpec.scala Outdated Show resolved Hide resolved

hochgi force-pushed the gen-issue-3031-normalized-fields branch from bd080ac to ded2c32 Compare August 31, 2024 22:05

hochgi changed the title ~~[gen][WIP] normalized fields with annotations holding original field name~~ [gen] normalized fields with annotations holding original field name Aug 31, 2024

hochgi marked this pull request as ready for review August 31, 2024 22:07

hochgi requested review from jdegoes, vigoo and 987Nabil as code owners August 31, 2024 22:07

hochgi commented Aug 31, 2024

View reviewed changes

zio-http-gen/src/test/resources/inline_schema_weird_field_names.yaml Outdated Show resolved Hide resolved

hochgi commented Aug 31, 2024

View reviewed changes

hochgi commented Sep 1, 2024

View reviewed changes

project/Dependencies.scala Outdated Show resolved Hide resolved

hochgi commented Sep 1, 2024

View reviewed changes

zio-http-gen/src/main/scala/zio/http/gen/openapi/EndpointGen.scala Outdated Show resolved Hide resolved

987Nabil reviewed Sep 2, 2024

View reviewed changes

zio-http-gen/src/main/scala/zio/http/gen/openapi/Config.scala Outdated Show resolved Hide resolved

987Nabil reviewed Sep 3, 2024

View reviewed changes

zio-http-gen/src/main/scala/zio/http/gen/openapi/Config.scala Outdated Show resolved Hide resolved

hochgi added 11 commits September 3, 2024 09:59

[gen] clean some generated code annotations boilerplate in preparatio…

3fb209b

…n for zio#3031 (which will add more annotations). Specifically: use imports insteads of inlined FQCN when adding annotations

* add zio-parser & zio-config dependencies to zio-http-gen module

7cf1f73

* add new configurations needed for normalization of fields

inconsistent order of fields in different scala versions, so sorting …

1d6aef5

…fields (and imports) when rendering

adjusted test expectations to match sorted fields/imports change

45c85b0

more fields ordering fixes for tests

4efc2e6

re-enable flaky

0a0a752

remove redundant stuff from test file

dddbcf2

* Undo fields sorting

2e12817

* Restore test cases fields original order * Reduced normalized test to have only 4 fields (up to 4 fields, order is consistent, since Map1, …, Map4 implementation is consistent between scala versions. More fields aren't guaranteed to have consistent order)

fmt

3254386

hochgi force-pushed the gen-issue-3031-normalized-fields branch from 6ab7398 to 7ff855c Compare September 3, 2024 07:00

CI failed for docusaurus step, perhaps I should put scalafmt escaping…

dd23ea9

… on different lines?

3rd field was dropped from content, was redundant in config.

d826873

also, re-run CI since seems to be affected by a flaky test: Client / ClientSpec / request can be timed out manually while awaiting connection

987Nabil approved these changes Sep 4, 2024

View reviewed changes

987Nabil merged commit 200e816 into zio:main Sep 4, 2024
34 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[gen] normalized fields with annotations holding original field name #3069

[gen] normalized fields with annotations holding original field name #3069

hochgi commented Aug 30, 2024 •

edited

Loading

hochgi Aug 31, 2024 •

edited

Loading

987Nabil Sep 3, 2024

hochgi Sep 3, 2024 •

edited

Loading

987Nabil Sep 3, 2024

hochgi Sep 3, 2024 •

edited

Loading

codecov-commenter commented Sep 1, 2024

hochgi Sep 1, 2024

987Nabil Sep 2, 2024

hochgi Sep 3, 2024

987Nabil Sep 3, 2024

hochgi Sep 3, 2024

[gen] normalized fields with annotations holding original field name #3069

[gen] normalized fields with annotations holding original field name #3069

Conversation

hochgi commented Aug 30, 2024 • edited Loading

hochgi Aug 31, 2024 • edited Loading

Choose a reason for hiding this comment

987Nabil Sep 3, 2024

Choose a reason for hiding this comment

hochgi Sep 3, 2024 • edited Loading

Choose a reason for hiding this comment

987Nabil Sep 3, 2024

Choose a reason for hiding this comment

hochgi Sep 3, 2024 • edited Loading

Choose a reason for hiding this comment

codecov-commenter commented Sep 1, 2024

Codecov Report

hochgi Sep 1, 2024

Choose a reason for hiding this comment

987Nabil Sep 2, 2024

Choose a reason for hiding this comment

hochgi Sep 3, 2024

Choose a reason for hiding this comment

987Nabil Sep 3, 2024

Choose a reason for hiding this comment

hochgi Sep 3, 2024

Choose a reason for hiding this comment

hochgi commented Aug 30, 2024 •

edited

Loading

hochgi Aug 31, 2024 •

edited

Loading

hochgi Sep 3, 2024 •

edited

Loading

hochgi Sep 3, 2024 •

edited

Loading