Skip to content

Make custom generators writable and restore the random-string prefix - #6

Merged
imanimanyara merged 1 commit into
masterfrom
fix/custom-generators-and-random-string-prefix
Oct 6, 2026
Merged

imanimanyara merged 1 commit into
masterfrom
fix/custom-generators-and-random-string-prefix

Conversation

@imanimanyara

Copy link
Copy Markdown
Member

Summary

Two defects, plus three latent ones of the same kind found on the way.

Custom generators could not be written as documented. docs/tools/generators.md said to extend SkuGenerator, which is final. The shipped generators stay final; the docs now describe the two seams that work:

  • extend the non-final Generators\Services\Generator base (its getSourceString(), makeValue(), exists() and generate() are protected hooks, and uniqueness keeps working), or
  • implement SkuGeneratorInterface / OrderNumberGeneratorInterface / TicketNumberGeneratorInterface with a one-argument (Model $model) constructor and render().

Both examples are test fixtures configured through generator.<name>.generator and used by a model with the trait. A test fails if the documented example stops matching its fixture (proven by editing the doc and watching it go red).

Making the second seam real needed two fixes: the observer cast the generator to a string, which threw for any generator without __toString(), so it now calls render(); and the provider now rejects a configured class that does not implement GeneratorInterface with an InvalidOptionException naming the config key, instead of a TypeError from the observer. The shipped generators now implement the per-type interface they are resolved through.

Helpers::makeRandomString() read an undefined $prefix. Commit 5a0461b (2022) lifted the body out of the Str macros, where $prefix was a closure parameter, and left the parameter behind. empty() of an undefined variable raises nothing, so the branch was dead. It is now ?string $prefix = null, threaded through Identifiers::sku() / ticketNumber() and a new per-generator prefix config key (default null; for order numbers it replaces ORD). No existing caller passed a prefix, so no existing output changes.

Latent, fixed: Configs::$prefix had no default, so getPrefix() threw before setPrefix(); InvalidOptionException::invalidArgument() dropped the 500 code its caller passed, and render() type-hinted the Request facade.

Behaviour changes

  • Output is identical for every call that existed before, and for a published config without prefix.
  • The observer uses render(), not __toString().
  • A misconfigured generator class throws InvalidOptionException instead of TypeError.
  • InvalidOptionException from Configs::__get carries code 500 instead of 0.

The Str macros still ignore their $prefix argument for SKUs and ticket numbers, as 0.1.0 did. That is left for a separate decision.

Tests

66 tests, 158 assertions, green locally. The new tests failed on the old code: 3 errors and 7 failures, covering the helper prefix, the interface-only generator, the uninitialised prefix, the per-type interfaces, the config prefix and the docs fixture.

The generator docs told users to extend SkuGenerator, which is final, so
the documented example could not compile. Document the two seams that
work, extending the non-final Generator base or implementing the
interface, and run both as test fixtures; a test fails if the docs stop
matching. The observer now calls render() rather than casting to string,
so an interface-only generator works, the provider rejects a configured
class that is not a generator with the config key named, and the shipped
generators implement the per-type interface they are resolved through.

Helpers::makeRandomString read a $prefix it never declared: the body was
lifted out of the 2022 Str macros, where $prefix was a closure parameter.
Add it as an optional third parameter, thread it through Identifiers and
a new per-generator `prefix` config key, and default Configs::$prefix to
null so getPrefix() no longer throws before setPrefix().
Copilot AI balanced review requested due to automatic review settings October 6, 2026 11:55

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@imanimanyara
imanimanyara merged commit c07d36a into master Oct 6, 2026
6 checks 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