how-to-fix-use-case-validation.md

How to improve use case validation

Some unstructured thoughts and assumptions:

• lots of duplication, which is not great
• validating parts of a request always works the same way, i.e. no two validators would validate the same field (e.g. an id, edit metadata, …) on a use case request differently
• the added level of abstraction between use case validation and generic reusable validators in Application\Validation is useful, e.g. for post patch validation
• sometimes validators deserialize things, and we want to keep the result (ugh)

Option 1: the naive DRY-up

• simply take the parts that are redundant and extract them into their own classes
• use case validator unit tests would only check that each request part gets stuck into the right validator
• no longer need to test for the exact exception and error message that is thrown, which is nice

Before:

$validationError = $this->itemIdValidator->validate( $request->getItemId() );
if ( $validationError ) {
  throw new UseCaseError(
    UseCaseError::INVALID_ITEM_ID,
    "Not a valid item ID: {$validationError->getContext()[ItemIdValidator::CONTEXT_VALUE]}"
  );
}

After:

$this->itemIdValidator->assertValidItemId( $request->getItemId() );

Thoughts:

• works fine
• still requires quite a lot of repetitive code, especially in tests

Option 2: combining validators

• add interfaces to use case request objects to hint which parts they contain

class ReplaceItemStatementRequest 
  extends ItemIdRequest, StatementIdRequest, StatementSerializationRequest, EditRequest {
  // ...
}

• now we can pass the whole request into a validator and it knows how to get the field that it needs to validate
• use case validators are now made up of lists of validators

class CombinedValidator {
  // ...
  public function assertValidRequest( $request ): void {
    foreach ( $this->validators as $validator ) {
      $validator->assertValidRequest( $request );
    }
  }
}

• can be instantiated within the use case validator so that we still have one specific validator per use case and don't do loosey-goosey CombinedValidator hints
• use a factory, either with methods per validator, or some constant mapping if we're feeling fancy

class ReplaceItemStatementValidator {
  // ...
  public function assertValidRequest( $request ): void {
    $this->validatorFactory->createCombinedValidator(
      ValidatorFactory::ITEM_ID_VALIDATOR,
      ValidatorFactory::STATEMENT_ID_VALIDATOR,
      ValidatorFactory::STATEMENT_SERIALIZATION_VALIDATOR,
      ValidatorFactory::EDIT_METADATA_VALIDATOR
    )->assertValidRequest( $request );
  }
}

Thoughts:

• nice side effect: since validators now take whole requests as input, they can remember the object and skip those they validated before
• nicer IMO and less code than option 1
• would we even need tests for these kinds of validators? it's just a list of strings (constants)
• how to deal with getValidatedStatement()? 😭 could have a special CombinedValidator that has access to the inner statement validator

Option 3: one validator to rule them all

• realization while thinking about option 2: there is a 1 to 1 mapping between interface and validator
• we have a limited number of input types, so we can have each validator "take a look" at the request and decide whether they want to validate it
• add method to each validator to check whether they're suited for the request

class ItemIdValidator {
  public function isValidatorForRequest( $request ): bool {
    return $request instanceof ItemIdRequest;
  }
  
  public function assertValidRequest( ItemIdRequest $request ) { /* ... */ }
}

• 1 validator to be used by all use cases

class RequestValidator {
  public function assertValidRequest( $request ): void {
    foreach ( $this->allValidators as $validator ) {
      if ( $validator->isValidatorForRequest( $request ) ) {
        $validator->assertValidRequest( $request );
      }
    }
  }
}

Thoughts:

• feels a bit too clever, but can't think of a reason why it wouldn't work
• special use cases can still implement their own / things on top
• how to deal with getValidatedStatement()? the almighty validator already has access to the statement validator. can expose it through another method, and even pass along the request object as a sanity check getValidatedStatement( StatementSerializationRequest $request ): Statement
• same as above: can remember request objects