Skip to content

Refactor SDK: entity contracts, new entities, and docs - #17

Merged
ghultink merged 6 commits into
masterfrom
feature/refactor-sdk-entities-and-contracts
Mar 20, 2026
Merged

Refactor SDK: entity contracts, new entities, and docs#17
ghultink merged 6 commits into
masterfrom
feature/refactor-sdk-entities-and-contracts

Conversation

@martijnbots

@martijnbots martijnbots commented Mar 6, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Introduce CRUD contracts (Creatable, Deletable, Gettable, Listable, Updatable) as interfaces, replacing the monolithic Entity base class
  • Add new entities: Playlist, Playout, and Subtitle with full CRUD support
  • Remove MediaClipList (consolidated into Playlist), update Channel and MediaClip with contract-based CRUD
  • Rewrite README with comprehensive usage documentation, quick start guide, and examples
  • Update license year, improve error handling in Response and HTTPRequestException
  • Add unit tests for new entities (PlaylistTest, PlayoutTest, SubtitleTest)

Test plan

  • Run composer test to verify all unit tests pass
  • Verify new entities (Playlist, Playout, Subtitle) work against SAPI
  • Verify existing MediaClip and Channel functionality is not broken
  • Check README examples are accurate

🤖 Generated with Claude Code

Martijn Bots and others added 4 commits March 6, 2026 09:55
…ntation

Introduce CRUD contracts (Creatable, Deletable, Gettable, Listable, Updatable)
as interfaces to replace monolithic Entity base class. Add Playlist, Playout,
and Subtitle entities. Remove MediaClipList (consolidated into MediaClip).
Update Channel and MediaClip with full CRUD support. Rewrite README with
comprehensive usage documentation.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…r edge cases

Cover the remaining untested paths to reach 100% class coverage:
- Test getResponseBody() on HTTPRequestException (via assertIsOk)
- Test uploadProgressGenerator maxIterations exceeded
- Test getAbsoluteVideoPathAsync directly
- Test executeUploadAsync missing required keys validation

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Cover the last uncovered line: the UnexpectedValueException thrown when
the mediaclip response does not contain a 'src' field.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- create-release-branch: manual dispatch to create release/v*.* branches
- create-release: manual dispatch from release branch to create GitHub Release
  with auto-incrementing patch version

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

@ghultink ghultink left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good overall — clean contract design, consistent new entities, good security improvement with LIBXML_NONET.

One bug to fix: removing the CRUD methods from the Entity base class introduces an infinite recursion risk. For entities that don't implement CRUD contracts (e.g. Thumbnail), calling $sdk->thumbnail->get(1) will cause AutoAsyncToSyncCaller::__call to check method_exists($this, 'getAsync') → false → falls through to call_user_func_array([$this, 'get'], ...) → triggers __call again → stack overflow.

Previously the base class threw NotImplementedException. Consider adding a guard in __call (e.g. check for recursion or throw a clean error when the async method doesn't exist or re-adding minimal fallbacks to the base class.
EOF
)

@ghultink ghultink left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Additional issue: the Playout entity maps to /sapi/player, but player and playout are separate SAPI entities. This should use /sapi/playout instead.

Martijn Bots and others added 2 commits March 10, 2026 15:26
…date dependencies

- Fix AutoAsyncToSyncCaller to throw BadMethodCallException for undefined
  methods instead of recursing, while preserving access to protected methods
- Fix Playout entity to use /sapi/playout instead of /sapi/player
- Update composer dependencies to resolve PHP 8.4 deprecation warnings

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Cover the previously untested code path where __call throws
BadMethodCallException for methods that don't exist and have no
async counterpart. Fixes 100% class coverage requirement.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@ghultink
ghultink merged commit ec56809 into master Mar 20, 2026
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.

2 participants