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.

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

use SString schema for LocalDateTime codec #989

Merged
merged 1 commit into from Feb 8, 2021
Merged

use SString schema for LocalDateTime codec #989

merged 1 commit into from Feb 8, 2021

Conversation

dkarwacki
Copy link
Contributor

No description provided.

@adamw
Copy link
Member

adamw commented Feb 5, 2021

I think the solution here might be a bit simpler :)

First of all, examples are not part of the schema type - they can be provided by the user, but don't have to be. Hence I'd leave the SDateTime intact.

The test is good - let's leave it - however we need to somehow provide example values. Following the docs on customising schemas, this could be sth like:

implicit val testSchemaForInstant: Schema[Instant] = Schema.instant.example(an example instant instance)

That is, we take the default schema for instant and extend it with a higher-priority implicit.

As for the Date codec in circe - that's a whole separate concern (JSON parsing/encoding), so I'd leave that out. It's outside the responsibility of tapir to extend what circe provides; we just provide the integration layer

@adamw
Copy link
Member

adamw commented Feb 5, 2021

Ah, one of the original issues might be that we don't have a schema for LocalDateTime?

dkarwacki added a commit that referenced this pull request Feb 5, 2021
@dkarwacki dkarwacki changed the title Add example values for date-time fields in OpenAPI docs consistent with circe serialization results use SString schema for OffsetDateTime, LocalDateTime and ZonedDateTime codecs Feb 8, 2021
@dkarwacki
Copy link
Contributor Author

Swagger uses https://tools.ietf.org/html/rfc3339 specification and treats date-time fields as points in time in UTC zone, so only Instant and java.util.Date classes are compatibile with that

@dkarwacki dkarwacki changed the title use SString schema for OffsetDateTime, LocalDateTime and ZonedDateTime codecs use SString schema for LocalDateTime codec Feb 8, 2021
@@ -142,7 +139,7 @@ object Schema extends SchemaExtensions with SchemaMagnoliaDerivation with LowPri
implicit val schemaForZonedDateTime: Schema[ZonedDateTime] = Schema(SDateTime)
implicit val schemaForOffsetDateTime: Schema[OffsetDateTime] = Schema(SDateTime)
implicit val schemaForDate: Schema[Date] = Schema(SDateTime)
implicit val schemaForLocalDateTime: Schema[LocalDateTime] = Schema(SDateTime)
implicit val schemaForLocalDateTime: Schema[LocalDateTime] = Schema(SString)
implicit val schemaForLocalDate: Schema[LocalDate] = Schema(SDate)
Copy link
Member

Choose a reason for hiding this comment

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

what about LocalDate? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would leave SDate schema for it, as swagger using it returns correct example, eg. "2017-07-21"

@adamw adamw merged commit da00a0e into master Feb 8, 2021
@mergify mergify bot deleted the bug/761 branch February 8, 2021 11:18
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.

[BUG] - Trailing Z in swagger doc examples for LocalDateTime with circe and http4s
2 participants