Skip to content

Document Apache HTTP implementations - #528

Merged
jonathanpeppers merged 7 commits into
mainfrom
jonathanpeppers-document-http-impl-apis
Aug 18, 2026
Merged

Document Apache HTTP implementations#528
jonathanpeppers merged 7 commits into
mainfrom
jonathanpeppers-document-http-impl-apis

Conversation

@jonathanpeppers

Copy link
Copy Markdown
Member

Closes#301

Sources

Validation

  • Parsed all 13 changed XML files with System.Xml.XmlDocument.
  • Confirmed no To be added. placeholders remain in Org.Apache.Http.Impl or its namespace XML.
  • Ran git diff --check.

Refs #301
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

CopilotAI left a comment

Copy link
Copy Markdown

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 updates the XML documentation for the Org.Apache.Http.Impl namespace to replace To be added. placeholders with meaningful summaries/parameter/return descriptions, aligning the generated API docs with the underlying Apache HTTP implementation types exposed in Mono.Android.

Changes:

  • Replaced placeholder summaries across core connection/strategy/factory types in Org.Apache.Http.Impl.
  • Added parameter and return-value documentation for key methods/properties (e.g., socket binding, buffer creation, metrics lookups).
  • Added a namespace-level summary for Org.Apache.Http.Impl.

Reviewed changes

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

Show a summary per file
FileDescription
docs/xml/Org.Apache.Http.Impl/SocketHttpServerConnection.xmlAdds class/member docs for socket-backed HTTP server connections.
docs/xml/Org.Apache.Http.Impl/SocketHttpClientConnection.xmlAdds class/member docs for socket-backed HTTP client connections.
docs/xml/Org.Apache.Http.Impl/NoConnectionReuseStrategy.xmlDocuments the “never reuse” connection reuse strategy (one doc accuracy fix requested).
docs/xml/Org.Apache.Http.Impl/HttpConnectionMetricsImpl.xmlDocuments connection metrics collection and metric accessors.
docs/xml/Org.Apache.Http.Impl/EnglishReasonPhraseCatalog.xmlDocuments English HTTP status reason phrase lookup behavior.
docs/xml/Org.Apache.Http.Impl/DefaultHttpServerConnection.xmlDocuments default server connection implementation details.
docs/xml/Org.Apache.Http.Impl/DefaultHttpClientConnection.xmlDocuments default client connection implementation details.
docs/xml/Org.Apache.Http.Impl/DefaultHttpResponseFactory.xmlDocuments default HTTP response creation APIs.
docs/xml/Org.Apache.Http.Impl/DefaultHttpRequestFactory.xmlDocuments default HTTP request creation APIs.
docs/xml/Org.Apache.Http.Impl/DefaultConnectionReuseStrategy.xmlDocuments default connection reuse decision logic entry points.
docs/xml/Org.Apache.Http.Impl/AbstractHttpServerConnection.xmlDocuments base server connection behavior and extensibility points.
docs/xml/Org.Apache.Http.Impl/AbstractHttpClientConnection.xmlDocuments base client connection behavior and extensibility points.
docs/xml/ns-Org.Apache.Http.Impl.xmlAdds a namespace-level summary for Org.Apache.Http.Impl.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment threaddocs/xml/Org.Apache.Http.Impl/NoConnectionReuseStrategy.xml Outdated
Refs #301
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@jonathanpeppers

Copy link
Copy Markdown
MemberAuthor

@dalexsoto review

1 similar comment
@jonathanpeppers

Copy link
Copy Markdown
MemberAuthor

@dalexsoto review

@dalexsotodalexsoto left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The prior KeepAlive finding is fixed, but three implementation-contract groups remain inaccurate:

  • EnglishReasonPhraseCatalog.GetReason ignores locale and requires status 100-599; DetermineLocale ignores context and always returns the default locale.
  • DefaultHttpRequestFactory supports only GET/POST/PUT/HEAD/OPTIONS/DELETE/TRACE, throws for methods such as PATCH/CONNECT, and only POST/PUT are entity-enclosing.
  • Connection metrics, sockets, addresses, ports, and timeouts can be null or -1 before initialization/binding; the current docs present them as always available.

Please document these concrete behaviors throughout the affected client/server and metrics XML.

Refs #301
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 70c8c744-f36b-4f2a-9993-b1acdb352c84

@jonathanpeppersjonathanpeppers left a comment

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Addressed all three contract groups from the AOSP implementation: the reason-catalog range and ignored locale, response-factory default locale and ignored context, request-factory method support and entity-enclosing behavior, and nullable/-1 pre-initialization connection values.

@jonathanpeppers

Copy link
Copy Markdown
MemberAuthor

@dalexsoto review

@dalexsotodalexsoto left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The locale, request-method, metrics, and concrete socket fixes are correct, but the abstract client and server SocketTimeout properties still omit the full sentinel contract. Please document positive milliseconds, 0 as disabled, and -1 as undefined consistently on both abstract APIs: AbstractHttpClientConnection.xml and AbstractHttpServerConnection.xml.

Refs #301
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a8faf899-f391-481e-97f9-55e0e3e2e54b
@jonathanpeppers

Copy link
Copy Markdown
MemberAuthor

Addressed the remaining SocketTimeout feedback in 2858857: both abstract client and server properties now document positive millisecond values, 0 as disabled, and -1 as undefined. The 13 changed XML files pass the mdoc ECMA validator, the updated text appears in both generated HTML pages, all retain CRLF endings, and git diff --check passes.

@dalexsotodalexsoto left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The SocketTimeout docs still conflate getter sentinels with setter-valid values. Getting may return -1 when unavailable or undefined, but the socket-backed setters reject negative values; setting accepts nonnegative milliseconds, with 0 meaning an infinite/disabled timeout. Please separate getter and setter behavior on both abstract properties and both concrete socket implementations.

Refs #301
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a8faf899-f391-481e-97f9-55e0e3e2e54b
@jonathanpeppers

Copy link
Copy Markdown
MemberAuthor

Addressed the getter/setter distinction in bbe5f0c across both abstract properties and both socket-backed implementations. The docs now state that getters may return -1 when unavailable or undefined, while setters accept only nonnegative milliseconds, use 0 for an infinite/disabled timeout, and reject negative values. The XML passes mdoc validation, the text appears in all four generated HTML pages, CRLF is preserved, and git diff --check passes.

@dalexsotodalexsoto left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

SocketTimeout getter sentinels and setter-valid values are now correctly distinguished across all four APIs.

Resolve the namespace documentation conflict by preserving the more complete upstream summary and attribution.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a8faf899-f391-481e-97f9-55e0e3e2e54b
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: a8faf899-f391-481e-97f9-55e0e3e2e54b
@jonathanpeppers

Copy link
Copy Markdown
MemberAuthor

Resolved the main merge conflict in 053bba9/987f57e3 by preserving the newer upstream Org.Apache.Http.Impl namespace summary and attribution in full. That namespace file now matches main exactly and has dropped out of the PR diff; the PR remains limited to the 12 intended implementation-type XML files. The affected XML passes mdoc validation, all four reviewed timeout pages still generate correctly, CRLF is preserved, and git diff --check passes.

@dalexsotodalexsoto left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The conflict resolution preserves current main documentation while retaining all reviewed implementation contracts and timeout fixes.

@jonathanpeppers
jonathanpeppers merged commit c97ee94 into mainAug 18, 2026
3 checks passed
@jonathanpeppers
jonathanpeppers deleted the jonathanpeppers-document-http-impl-apis branch August 18, 2026 17:51
Sign up for freeto 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.

Add XML documentation for Org.Apache.Http.Impl

3 participants

@jonathanpeppers@dalexsoto