Deduplicate the per-draft constraint classes #991

Activity

1. DannyvdSluijs commented on Oct 26, 2026

   

   DannyvdSluijs

   on Oct 26, 2026

   Collaborator

   More actions

   Hi @Roman3349 thanks for the extensive report. I'm aware there is duplication between the drafts, which is intentional or at least the result of the Draft 3 and Draft 4 being intertwined. I've chosen explicitly not sharing code between draft elements unless the draft refers to a complex format which was captured in an RFC. You're absolutely right on the downside that during fixes or improvements we need to consider multiple classes/paths.

   For now I would still think keeping the duplication and not having to do a mayor version bump is the way to go. As I would rather give priority to getting the drafts complete (you can check the badges in the readme on the progress).

   Beside the organisational issues there is no impact on timings/performance right?
2. DannyvdSluijs

   added

   Awaiting feedback

   on Oct 26, 2026
3. Roman3349 commented on Oct 28, 2026

   

   Roman3349

   on Oct 28, 2026

   ContributorAuthor

   More actions

   Hi @DannyvdSluijs,

   that makes sense, and I agree that completing the drafts comes first.

   No, there's no runtime impact. Validation runs the same code either way, and the copies only cost a bit of extra opcache memory and autoloading when one process validates schemas of several drafts. It's purely a maintenance issue. For the record, the proposal wouldn't need a major version bump either, because the per-draft classes would stay as thin subclasses.

   For illustration, the thin subclasses from the proposal would look like this:

   src/JsonSchema/Constraints/Drafts/
   ├── Common/
   │   ├── MinLengthConstraint.php     # the implementation, moved from Draft06/
   │   └── ...
   ├── Draft06/
   │   ├── MinLengthConstraint.php     # extends Common\MinLengthConstraint
   │   ├── FormatConstraint.php        # stays a full per-draft implementation
   │   └── Factory.php                 # unchanged, still maps to Draft06\* classes
   ├── Draft07/
   │   ├── MinLengthConstraint.php     # extends Common\MinLengthConstraint
   │   ├── FormatConstraint.php        # stays a full per-draft implementation
   │   └── Factory.php                 # unchanged, still maps to Draft07\* classes
   └── Draft2019/
       ├── MinLengthConstraint.php     # extends Common\MinLengthConstraint
       ├── FormatConstraint.php        # stays a full per-draft implementation
       └── Factory.php                 # unchanged, still maps to Draft2019\* classes
   

   <?php
   
   declare(strict_types=1);
   
   namespace JsonSchema\Constraints\Drafts\Common;
   
   use JsonSchema\ConstraintError;
   use JsonSchema\Constraints\ConstraintInterface;
   use JsonSchema\Constraints\Factory;
   use JsonSchema\Entity\ErrorBagProxy;
   use JsonSchema\Entity\JsonPointer;
   
   class MinLengthConstraint implements ConstraintInterface
   {
       use ErrorBagProxy;
   
       public function __construct(?Factory $factory = null)
       {
           $this->initialiseErrorBag($factory ?: new Factory());
       }
   
       public function check(&$value, $schema = null, ?JsonPointer $path = null, $i = null): void
       {
           // the body of today's Draft07\MinLengthConstraint::check(), unchanged
       }
   }

   <?php
   
   declare(strict_types=1);
   
   namespace JsonSchema\Constraints\Drafts\Draft07;
   
   class MinLengthConstraint extends \JsonSchema\Constraints\Drafts\Common\MinLengthConstraint
   {
   }

   The per-draft class names, the factories' constraintMap and Factory::setConstraintClass() keep working as before, which is why no major version bump is needed. A fix to minLength then goes into Common once.

   Strict mode support for Draft-04 doesn't depend on the deduplication either. It could be done as one more copied set later, if it's welcome. I'm fine with closing this issue or leaving it open for later, whichever you prefer.