Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Nov 14, 2025

Description

Resolves #[issue number]

The existing InstantConverter component was present but not explicitly integrated with Spring MVC's parameter conversion system. This PR makes the integration explicit and adds comprehensive validation.

Changes

  • WebConfig: Explicitly registers InstantConverter with Spring MVC's FormatterRegistry to handle StringInstant conversions for HTTP parameters
  • InstantConverterTest: 13 unit tests covering ISO 8601 format variations (UTC, timezone offsets, precision levels) and xAPI spec edge cases (negative zero offset rejection)
  • StatementsControllerTest: 5 integration tests validating HTTP parameter binding with various timestamp formats
  • ServerControllerAdvice: Enhanced error handling for XapiTimestampParseException with RFC 7807-compliant error responses
  • README: Documents supported formats and usage examples

Example

The converter now handles timestamps in HTTP parameters:

# Valid formats - parsed successfully
GET /xapi/statements?since=2017-03-01T12:30:00.000Z
GET /xapi/statements?since=2017-03-01T12:30:00.000+05:00

# Invalid format - rejected with clear error
GET /xapi/statements?since=2017-03-01T12:30:00.000-00
→ 400 Bad Request: "Invalid timestamp format: Negative timezone offset can not be zero"

Checklist:

  • Public methods are documented
  • Public methods are tested
  • New and existing tests pass when run locally
  • There are no new warnings or errors

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • checkstyle.org
    • Triggering command: /opt/hostedtoolcache/CodeQL/2.23.3/x64/codeql/tools/linux64/java/bin/java -jar /opt/hostedtoolcache/CodeQL/2.23.3/x64/codeql/xml/tools/xml-extractor.jar --fileList=/home/REDACTED/work/xapi-java/.codeql-scratch/dbs/java/working/files-to-index10924619342638379015.list --sourceArchiveDir=/home/REDACTED/work/xapi-java/.codeql-scratch/dbs/java/src --outputDir=/home/REDACTED/work/xapi-java/.codeql-scratch/dbs/java/trap/java (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Original prompt

This section details on the original issue you should resolve

<issue_title>[Enhancement]: xAPI sample server should have timestamp converter for HTTP parameters</issue_title>
<issue_description>### What

Integrate the existing timestamp converter component into the xAPI sample server so that it correctly parses, validates, and converts HTTP request parameters containing timestamps. Ensure that the timestamp converter is utilized for a range of common formats used by clients, and test its behavior in the server processing flow.

Why

This enhancement leverages code already present in the codebase to improve interoperability and robustness, allowing clients to send timestamps in preferred formats while ensuring data consistency in server operations. It will streamline data handling, reduce client-side formatting errors, and facilitate integration with diverse systems and tools. Making use of existing functionality also reduces development time.

Notes

There is an existing timestamp converter present in the codebase, but it is not currently used by the xAPI sample server. This enhancement should focus on integrating the existing converter into HTTP parameter handling. The converter should handle edge cases like timezone suffixes and invalid formats gracefully, providing clear error messages. Consider extensibility for additional formats in future releases.</issue_description>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

- Create WebConfig to explicitly register InstantConverter
- Add comprehensive unit tests for InstantConverter (13 test cases)
- Add integration tests for various timestamp formats
- Enhance error handling for timestamp parsing exceptions
- Update README with timestamp converter documentation

Co-authored-by: thomasturrell <1552612+thomasturrell@users.noreply.github.com>
Copilot AI changed the title [WIP] Integrate timestamp converter into xAPI sample server Integrate timestamp converter for HTTP parameters in xAPI sample server Nov 14, 2025
Copilot finished work on behalf of thomasturrell November 14, 2025 08:17
Copilot AI requested a review from thomasturrell November 14, 2025 08:17
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.

[Enhancement]: xAPI sample server should have timestamp converter for HTTP parameters

2 participants