Skip to content

Conversation

@rfc2822
Copy link
Member

@rfc2822 rfc2822 commented Nov 24, 2025

Separate data classes and parser methods so that it's clear where tests have to be

@rfc2822 rfc2822 self-assigned this Nov 24, 2025
@rfc2822 rfc2822 added the refactoring Internal improvement of existing functions label Nov 24, 2025
@rfc2822 rfc2822 requested a review from Copilot November 24, 2025 12:53
@rfc2822 rfc2822 marked this pull request as ready for review November 24, 2025 12:54
Copilot finished reviewing on behalf of rfc2822 November 24, 2025 12:57
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the WebDAV XML parsing logic in the ktor package by separating parser methods from data classes. Previously, parsing logic was implemented as companion object methods on data classes (Response.parse, PropStat.parse). Now, dedicated parser classes (ResponseParser, PropStatParser, MultiStatusParser) handle the parsing logic, leaving data classes focused purely on data representation. This separation clarifies the code structure and makes it clearer where tests should be placed.

Key Changes:

  • Created three new parser classes: ResponseParser, PropStatParser, and MultiStatusParser
  • Removed companion objects with parsing logic from Response and PropStat data classes
  • Updated DavResource to use the new MultiStatusParser instead of the private parseMultiStatus method

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/main/kotlin/at/bitfire/dav4jvm/ktor/ResponseParser.kt New parser class for WebDAV response XML elements, extracted from Response companion object
src/main/kotlin/at/bitfire/dav4jvm/ktor/PropStatParser.kt New parser class for propstat XML elements, extracted from PropStat companion object
src/main/kotlin/at/bitfire/dav4jvm/ktor/MultiStatusParser.kt New parser class for multistatus XML responses, extracted from DavResource.parseMultiStatus
src/main/kotlin/at/bitfire/dav4jvm/ktor/Response.kt Removed companion object with parsing logic; kept as pure data class
src/main/kotlin/at/bitfire/dav4jvm/ktor/PropStat.kt Removed companion object with parsing logic; kept as pure data class
src/main/kotlin/at/bitfire/dav4jvm/ktor/DavResource.kt Updated to use MultiStatusParser and removed private parseMultiStatus method

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Move `PropStatParser` to object-based implementation
- Update `MultiStatusParser` and `ResponseParser` to use callback directly
- Remove redundant parameters from parsing functions
@rfc2822 rfc2822 force-pushed the separate-data-classes-and-parser branch from ccf65a2 to 8229a31 Compare November 24, 2025 13:02
@rfc2822 rfc2822 requested a review from Copilot November 24, 2025 13:02
Copilot finished reviewing on behalf of rfc2822 November 24, 2025 13:05
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@rfc2822 rfc2822 merged commit 23285c7 into main Nov 24, 2025
11 checks passed
@rfc2822 rfc2822 deleted the separate-data-classes-and-parser branch November 24, 2025 13:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactoring Internal improvement of existing functions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant