Skip to content
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.

Sign up for GitHub

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

Default value for enumeratum #1852

Merged
merged 14 commits into from
Feb 18, 2022
Merged

Default value for enumeratum #1852

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
72aeebe
Regression test for Request Body with annotated defaults
mkrzemien Feb 7, 2022
6c5dacf
Test for query parameter as enum with defaults
mkrzemien Feb 7, 2022
3ffe984
Test for query parameter as enum with other default ignored
mkrzemien Feb 7, 2022
198ca6b
Test for Request Body with enum and annotated default
mkrzemien Feb 7, 2022
84638e5
Work-around with explicit encoded in default
mkrzemien Feb 8, 2022
25f0808
Cleanup - test name
mkrzemien Feb 8, 2022
0097f54
Cleanup - test name
mkrzemien Feb 10, 2022
6eae808
Cleanup - test name for ignored case
mkrzemien Feb 10, 2022
980a3f2
Draft macro for explicit encoded in default
mkrzemien Feb 11, 2022
9e6ab40
Cleanup - test names
mkrzemien Feb 14, 2022
70ec254
Fix for Scala 3
mkrzemien Feb 15, 2022
4851df1
Fixing the target of the default macro
mkrzemien Feb 15, 2022
c8a405f
Reverting the default macro after review
mkrzemien Feb 16, 2022
143b89f
Test macro with various combinations of annotation params
mkrzemien Feb 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions core/src/main/scala-2/sttp/tapir/generic/internal/EndpointAnnotationsMacro.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,8 @@ abstract class EndpointAnnotationsMacro(val c: blackbox.Context) {
.fold(i)(encodedExample => q"$i.schema(_.encodedExample($encodedExample))"),
i =>
util
.extractTreeFromAnnotation(field, schemaDefaultType)
.fold(i)(default => q"$i.default($default)"),
.extractTreeAndOptFromAnnotation(field, schemaDefaultType)
.fold(i)(default => q"$i.default(${default._1}, encoded=${default._2})"),
i =>
util
.extractStringArgFromAnnotation(field, schemaFormatType)
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/scala-2/sttp/tapir/generic/internal/SchemaMagnoliaDerivation.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ trait SchemaMagnoliaDerivation {
annotations.foldLeft(schema) {
case (schema, ann: Schema.annotations.description) => schema.description(ann.text)
case (schema, ann: Schema.annotations.encodedExample) => schema.encodedExample(ann.example)
case (schema, ann: Schema.annotations.default[X]) => schema.default(ann.default)
case (schema, ann: Schema.annotations.default[X]) => schema.default(ann.default, ann.encoded)
case (schema, ann: Schema.annotations.validate[X]) => schema.validate(ann.v)
case (schema, ann: Schema.annotations.format) => schema.format(ann.format)
case (schema, _: Schema.annotations.deprecated) => schema.deprecated(true)
Expand Down
11 changes: 11 additions & 0 deletions core/src/main/scala-2/sttp/tapir/internal/CaseClassUtil.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,15 @@ private[tapir] class CaseClassUtil[C <: blackbox.Context, T: C#WeakTypeTag](val
}
}
}

def extractTreeAndOptFromAnnotation(field: Symbol, annotationType: c.Type): Option[(Tree, Option[String])] = {
field.annotations.collectFirst {
case a if a.tree.tpe <:< annotationType =>
a.tree.children.tail match {
case List(t, Literal(Constant(str: String))) => (t, Some(str))
case List(t, TypeApply(Select(_, name @ TermName(_)), List(TypeTree()))) if name.decodedName.toString.startsWith("<init>$default") => (t, None)
case _ => throw new IllegalStateException(s"Cannot extract annotation argument from: ${c.universe.showRaw(a.tree)}")
}
}
}
}
2 changes: 1 addition & 1 deletion core/src/main/scala-3/sttp/tapir/internal/SchemaMagnoliaDerivation.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ trait SchemaMagnoliaDerivation {
annotations.foldLeft(schema) {
case (schema, ann: Schema.annotations.description) => schema.description(ann.text)
case (schema, ann: Schema.annotations.encodedExample) => schema.encodedExample(ann.example)
case (schema, ann: Schema.annotations.default[X @unchecked]) => schema.default(ann.default)
case (schema, ann: Schema.annotations.default[X @unchecked]) => schema.default(ann.default, ann.encoded)
case (schema, ann: Schema.annotations.validate[X @unchecked]) => schema.validate(ann.v)
case (schema, ann: Schema.annotations.format) => schema.format(ann.format)
case (schema, _: Schema.annotations.deprecated) => schema.deprecated(true)
Expand Down
5 changes: 4 additions & 1 deletion core/src/main/scala/sttp/tapir/EndpointIO.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,10 @@ object EndpointTransput {
schema(_.modifyUnsafe[U](Schema.ModifyCollectionElements)(_.validate(v)))

def description(d: String): ThisType[T] = copyWith(codec, info.description(d))
def default(d: T): ThisType[T] = copyWith(codec.schema(_.default(d, Some(codec.encode(d)))), info)
def default(d: T, encoded: Option[Any] = None): ThisType[T] = {
val e = Some(encoded.getOrElse(codec.encode(d)))
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we allow to pass encoded here? It can cause ambiguity if someone passes a value that doest not match the codec encoding

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without this change I get following compilation error:

[error] /home/mariusz/sml/tapir/core/src/test/scala/sttp/tapir/annotations/DeriveEndpointIOTest.scala:244:40: unknown parameter name: encoded
[error]     val derived = EndpointInput.derived[TapirRequestTest8].asInstanceOf[EndpointInput.Query[TapirRequestTest8]]
[error]                                        ^
[error] /home/mariusz/sml/tapir/core/src/test/scala/sttp/tapir/annotations/DeriveEndpointIOTest.scala:246:43: unknown parameter name: encoded
[error]     compareTransputs(EndpointInput.derived[TapirRequestTest8], expectedInput) shouldBe true
[error]                                           ^

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, we are passing encoded using this method in annotations macro.

Copy link
Member

Choose a reason for hiding this comment

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

Encoded defaults only have to be passed to schemas, not to inputs (as they have codecs). So I guess @kubinio123 is right, and we should instead opt out of using the encoded parameter on inputs (and document why :) )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I'll revert this change.
I just need to find another way to pass the encoded value from the @default annotation to the schema without using this method.

Copy link
Member

Choose a reason for hiding this comment

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

But the default value is already passed, being automatically encoded to the correct form?

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 turned out that the EndpointIO.default method was used by the macros instead of the Schema.default. I've updated the macros and reverted the change in EndpointIO.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reverting to using EndpointIO.default (as suggested in the review).
Adding more tests for macro handling annotation with additional parameter.

copyWith(codec.schema(_.default(d, e)), info)
}
def example(t: T): ThisType[T] = copyWith(codec, info.example(t))
def example(example: Example[T]): ThisType[T] = copyWith(codec, info.example(example))
def examples(examples: List[Example[T]]): ThisType[T] = copyWith(codec, info.examples(examples))
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/scala/sttp/tapir/Schema.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,7 @@ object Schema extends LowPrioritySchema with SchemaCompanionMacros {
object annotations {
class description(val text: String) extends StaticAnnotation
class encodedExample(val example: Any) extends StaticAnnotation
class default[T](val default: T) extends StaticAnnotation
class default[T](val default: T, val encoded: Option[Any] = None) extends StaticAnnotation
class format(val format: String) extends StaticAnnotation
class deprecated extends StaticAnnotation
class encodedName(val name: String) extends StaticAnnotation
Expand Down
54 changes: 54 additions & 0 deletions ...t/resources/enum/expected_enumeratum_enum_adding_default_when_encoded_value_specified.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
openapi: 3.0.3
info:
title: Fruits
version: '1.0'
paths:
/:
post:
operationId: postRoot
requestBody:
content:
application/json:
schema:
$ref:'#/components/schemas/FruitQueryWithEncoded'
required: true
responses:
'200':
description: ''
content:
application/json:
schema:
$ref: '#/components/schemas/FruitWithEnum'
'400':
description: 'Invalid value for: body'
content:
text/plain:
schema:
type: string
components:
schemas:
FruitQueryWithEncoded:
type: object
properties:
fruitType:
$ref: '#/components/schemas/FruitType'
FruitType:
type: string
default: PEAR
enum:
- APPLE
- PEAR
FruitWithEnum:
required:
- fruit
- amount
type: object
properties:
fruit:
type: string
amount:
type: integer
fruitType:
type: array
items:
$ref: '#/components/schemas/FruitType'
50 changes: 50 additions & 0 deletions docs/openapi-docs/src/test/resources/enum/expected_enumeratum_enum_default.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
openapi: 3.0.3
info:
title: Fruits
version: '1.0'
paths:
/:
get:
operationId: getRoot
parameters:
- name: type
in: query
required: false
schema:
$ref: '#/components/schemas/FruitType'
example: APPLE
responses:
'200':
description: ''
content:
application/json:
schema:
$ref: '#/components/schemas/FruitWithEnum'
'400':
description: 'Invalid value for: query parameter type'
content:
text/plain:
schema:
type: string
components:
schemas:
FruitType:
type: string
default: PEAR
enum:
- APPLE
- PEAR
FruitWithEnum:
required:
- fruit
- amount
type: object
properties:
fruit:
type: string
amount:
type: integer
fruitType:
type: array
items:
$ref: '#/components/schemas/FruitType'
53 changes: 53 additions & 0 deletions ...rces/enum/expected_enumeratum_enum_not_adding_default_when_no_encoded_value_specified.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
openapi: 3.0.3
info:
title: Fruits
version: '1.0'
paths:
/:
post:
operationId: postRoot
requestBody:
content:
application/json:
schema:
$ref:'#/components/schemas/FruitQuery'
required: true
responses:
'200':
description: ''
content:
application/json:
schema:
$ref: '#/components/schemas/FruitWithEnum'
'400':
description: 'Invalid value for: body'
content:
text/plain:
schema:
type: string
components:
schemas:
FruitQuery:
type: object
properties:
fruitType:
$ref: '#/components/schemas/FruitType'
FruitType:
type: string
enum:
- APPLE
- PEAR
FruitWithEnum:
required:
- fruit
- amount
type: object
properties:
fruit:
type: string
amount:
type: integer
fruitType:
type: array
items:
$ref: '#/components/schemas/FruitType'
71 changes: 71 additions & 0 deletions .../src/test/resources/enum/expected_enumeratum_enum_using_first_specified_default_value.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
openapi: 3.0.3
info:
title: Fruits
version: '1.0'
paths:
/fruit-by-type1:
get:
operationId: getFruit-by-type1
parameters:
- name: type1
in: query
required: false
schema:
$ref: '#/components/schemas/FruitType'
responses:
'200':
description: ''
content:
application/json:
schema:
$ref: '#/components/schemas/FruitWithEnum'
'400':
description: 'Invalid value for: query parameter type1'
content:
text/plain:
schema:
type: string
/fruit-by-type2:
get:
operationId: getFruit-by-type2
parameters:
- name: type2
in: query
required: false
schema:
$ref: '#/components/schemas/FruitType'
responses:
'200':
description: ''
content:
application/json:
schema:
$ref: '#/components/schemas/FruitWithEnum'
'400':
description: 'Invalid value for: query parameter type2'
content:
text/plain:
schema:
type: string
components:
schemas:
FruitType:
type: string
default: PEAR
enum:
- APPLE
- PEAR
FruitWithEnum:
required:
- fruit
- amount
type: object
properties:
fruit:
type: string
amount:
type: integer
fruitType:
type: array
items:
$ref: '#/components/schemas/FruitType'
34 changes: 34 additions & 0 deletions docs/openapi-docs/src/test/resources/expected_default_request_body.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
openapi: 3.0.3
info:
title: Entities
version: '1.0'
paths:
/:
post:
operationId: postRoot
requestBody:
content:
application/json:
schema:
$ref: '#/components/schemas/ObjectWithDefaults'
required: true
responses:
'200':
description: ''
'400':
description: 'Invalid value for: body'
content:
text/plain:
schema:
type: string
components:
schemas:
ObjectWithDefaults:
type: object
properties:
name:
type: string
default: foo
count:
type: integer
default: 12
Loading