Skip to content

Anti-Patterns

When to Use

Avoiding common mistakes that lead to maintenance problems, security issues, or performance degradation.

Decision

Anti-Pattern Why It's Wrong Correct Approach
\Drupal::service() in plugins Breaks testability, static dependency Inject via constructor
No default_name implementation Media items show "Unsaved media" Always implement default_name
Skipping caching API rate limits, slow pages Cache with tags
Weak URL validation SSRF attacks, security breach Whitelist domains, validate scheme
Synchronous thumbnail downloads Page timeouts, poor UX Queue-based generation

Pattern

WRONG - Static service calls: 

class CustomApi extends MediaSourceBase {
  protected function fetchFromApi($id) {
    $client = \Drupal::httpClient(); // ❌ Static dependency
    return $client->get("https://api.example.com/{$id}");
  }
}

RIGHT - Dependency injection: 

class CustomApi extends MediaSourceBase {
  protected ClientInterface $httpClient;

  public function __construct(..., ClientInterface $http_client) {
    parent::__construct(...);
    $this->httpClient = $http_client; // ✅ Injected dependency
  }

  protected function fetchFromApi($id) {
    return $this->httpClient->get("https://api.example.com/{$id}");
  }
}

WRONG - No caching: 

public function getMetadata(MediaInterface $media, $attribute_name): mixed {
  $data = $this->httpClient->get($url)->getBody(); // ❌ Every call hits API
  return json_decode($data, TRUE)[$attribute_name];
}

RIGHT - Cached responses: 

public function getMetadata(MediaInterface $media, $attribute_name): mixed {
  $cache_key = "module:meta:{$id}:{$attribute_name}";
  if ($cached = $this->cache->get($cache_key)) {
    return $cached->data; // ✅ Cached result
  }

  $data = $this->httpClient->get($url)->getBody();
  $value = json_decode($data, TRUE)[$attribute_name];
  $this->cache->set($cache_key, $value, time() + 3600);
  return $value;
}

WRONG - Weak validation: 

if (strpos($url, 'example.com') !== FALSE) { // ❌ Accepts http://evil.com/example.com
  return $this->httpClient->get($url);
}

RIGHT - Strong validation: 

if (!UrlHelper::isValid($url, TRUE)) return NULL;
$host = parse_url($url, PHP_URL_HOST);
if ($host !== 'api.example.com') return NULL; // ✅ Exact domain match
if (parse_url($url, PHP_URL_SCHEME) !== 'https') return NULL; // ✅ HTTPS only

Common Mistakes

  • Not implementing default_name → Users see "Unsaved media", poor UX

  • WHY: Media entity uses default_name for automatic naming; without it, entity label is empty until manually set

  • No error handling on HTTP requests → White screens when API down

  • WHY: Network requests fail unpredictably; exceptions bubble up and crash pages unless caught

  • Caching without tags → Stale content, can't invalidate selectively

  • WHY: Cache tags enable targeted invalidation; without them, must flush entire cache bin to update one item

  • Hardcoding API credentials → Security breach when code leaked

  • WHY: Code is visible in Git history, deployment logs, and to all developers; credentials must be in config or key storage

  • Not calling parent methods → Missing core functionality

  • WHY: Parent classes provide essential features (OEmbed thumbnails, File validation); not calling parent loses these

See Also