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

fix: DTOData bug with renamed fields #2144

Merged
merged 1 commit into from Aug 10, 2023

Conversation

abdulhaq-e
Copy link
Member

@abdulhaq-e abdulhaq-e commented Aug 10, 2023

Pull Request Checklist

  • New code has 100% test coverage
  • (If applicable) The prose documentation has been updated to reflect the changes introduced by this PR
  • (If applicable) The reference documentation has been updated to reflect the changes introduced by this PR

Description

Implementing a fix for DTOData #2065 introduced a new bug. Originally, fields passed to DTOData.create_instance were handled using their serialized names. For example, when using camel case for an excluded field entity_id, data.create_instance(entityId="1") would work but data.create_instance(entity_id="A") wouldn't. That was not ideal so the erroneous fix was to ignore serialized field names altogether when dealing with DTOData. Here is an example of why this would fail:

@dataclass
class User):
    entityId: str
    firstName: str

@post(
    dto=DataclassDTO[Annotated[User, DTOConfig(exclude={"entity_id"}, rename_strategy="camel")]],
)
def handler(data: DTOData[User]) -> User:
    result = data.create_instance(entity_id="123")
    return result

Since. #2065 ignored serialized names, the above example will fail to deserialize b'{firstName: "J"}'. The fix: only override serialization names when using data.create_instance, otherwise, when dealing with DTOData we should use the serialization name (to successfully deserialize)

Close Issue(s)

@abdulhaq-e abdulhaq-e requested review from a team as code owners August 10, 2023 11:00
@github-actions
Copy link

Documentation preview will be available shortly at https://litestar-org.github.io/litestar-docs-preview/2144

@Goldziher Goldziher merged commit 031df7f into litestar-org:main Aug 10, 2023
22 checks passed
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.

None yet

2 participants