Skip to content

Add ETag Support to HackMD API Client #31

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

Merged
merged 4 commits into from
May 27, 2025
Merged

Conversation

bagnier
Copy link
Contributor

@bagnier bagnier commented Mar 30, 2025

Add ETag Support to HackMD API Client

Summary

This PR implements ETag support for HackMD API client operations, enabling conditional requests and response caching.

Technical Details

  • Added ETag support to GET, POST, and PATCH operations for user notes
  • ETags are now accessible in responses for getNote, createNote, updateNote, and updateNoteContent
  • Implemented proper handling of 304 Not Modified responses for conditional GET requests

Limitations

  • The HackMD API server does not currently support If-Match headers in PATCH requests, limiting conditional updates
  • Current implementation focuses on user notes operations - the most commonly used endpoints.

Implementation Approach

  • Made ETag usage optional to preserve backward compatibility
  • Included ETags automatically in responses when available
  • Required explicit opt-in for conditional requests via the options parameter
  • Focused implementation on user notes operations only (not team notes)
  • Comprehensive test suite verifies all ETag functionality scenarios

Security

  • Updated dependencies to fix vulnerabilities

bagnier added 4 commits March 28, 2025 20:32
      - Remove test.only to run all tests
      - Create dedicated client instance with retry disabled for rate limit tests
      - Fix Jest exit issues with proper cleanup
      - Remove debug console.log statements
  - Update axios from 0.25.0 to 1.8.4
  - Update msw from 1.0.1 to 2.7.3
  - Refactor mock server implementation to use MSW v2 API
  - Update API client to use new axios types
  - Fix type issues in headers handling
  - Add etag field to RequestOptions type
  - Implement conditional requests with If-None-Match header
  - Add proper handling of 304 Not Modified responses
  - Extract etag from response headers
  - Include etag in response data when requested
  - Add comprehensive test coverage for etag functionality
  * Add etag support to createNote method
  * Add etag support to updateNoteContent method
  * Add etag support to updateNote method
  * Organize tests by API method
  * Improve test naming for clarity
@Yukaii Yukaii requested review from Yukaii and Copilot May 27, 2025 07:25
Copy link

@Copilot 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 adds optional ETag support to the HackMD API client, enabling conditional requests and response caching for user note operations. Key changes include updating the API methods to optionally include ETag headers and accept 304 responses, modifying test mocks to use the new msw http API, and upgrading dependencies to support these changes.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
nodejs/tests/mock/index.ts Updated mock server setup to use new msw http API for GET handlers.
nodejs/tests/mock/handlers.ts Converted REST handlers to use the new msw http API.
nodejs/tests/etag.spec.ts Added comprehensive tests validating ETag functionality.
nodejs/tests/api.spec.ts Updated API tests including error handling and cleanup adjustments.
nodejs/src/index.ts Modified API class to support ETag functionality with conditional GET and header handling.
nodejs/package.json Upgraded msw and axios dependencies.
Comments suppressed due to low confidence (2)

nodejs/src/index.ts:150

  • [nitpick] The configuration in getNote that allows a 304 status via validateStatus is appropriate for ETag support; ensure that this does not inadvertently accept other non-successful responses that should be handled differently.
const config = options.etag ? { headers: { 'If-None-Match': options.etag }, validateStatus: (status: number) => (status >= 200 && status < 300) || status === 304 } : undefined

nodejs/src/index.ts:54

  • Using config.headers.set may cause runtime errors if the headers object does not support the set method. Consider verifying that Axios' header implementation in v1.8.4 supports this pattern or revert to direct object assignment (e.g. config.headers['Authorization'] = ...).
config.headers.set('Authorization', `Bearer ${accessToken}`)

@Yukaii
Copy link
Member

Yukaii commented May 27, 2025

Thanks for your contirbution, @bagnier !

@Yukaii Yukaii merged commit 75dcf4d into hackmdio:develop May 27, 2025
1 check 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