Skip to content

♻️ refactor: replace custom DeepL cURL client with official PHP SDK#4546

Merged
mauretto78 merged 1 commit intodevelopfrom
deepl-glossary-fix
Apr 30, 2026
Merged

♻️ refactor: replace custom DeepL cURL client with official PHP SDK#4546
mauretto78 merged 1 commit intodevelopfrom
deepl-glossary-fix

Conversation

@mauretto78
Copy link
Copy Markdown
Contributor

@mauretto78 mauretto78 commented Apr 29, 2026

Summary

Replaced the custom cURL-based DeepLApiClient with the official deeplcom/deepl-php SDK. The custom implementation had issues with API authentication and glossary creation (HTTP 400 "Access denied"). The official SDK handles authentication, request formatting, retries, and API versioning correctly out of the box.

Type

  • refactor — restructure without behavior change

Changes

File Change
composer.json Added deeplcom/deepl-php: ^1.18 dependency
composer.lock Updated lock file with new dependency
lib/Utils/Engines/DeepL/DeepLApiClient.php Rewrote to wrap official DeepL Translator SDK; same public API (returns arrays) preserved for backward compatibility

Testing

  • Manual testing performed (describe below) Glossary creation, listing, retrieval, deletion, and entry retrieval all delegate to the official SDK which handles auth headers, API versioning (v2), and request formatting.

AI Disclosure

  • No AI tools were used in this PR
  • AI tools were used — details below GitHub Copilot (claude-sonnet-4-20250514)

## Summary
Replaced the custom cURL-based DeepLApiClient with the official
`deeplcom/deepl-php` SDK. The custom implementation had issues with
API authentication and glossary creation (HTTP 400 "Access denied").
The official SDK handles authentication, request formatting, retries,
and API versioning correctly out of the box.
## Type
- [x] `refactor` — restructure without behavior change
## Changes
| File | Change |
|------|--------|
| composer.json | Added `deeplcom/deepl-php: ^1.18` dependency |
| composer.lock | Updated lock file with new dependency |
| lib/Utils/Engines/DeepL/DeepLApiClient.php | Rewrote to wrap official DeepL Translator SDK; same public API (returns arrays) preserved for backward compatibility |
## Testing
- [ ] Manual testing performed (describe below)
Glossary creation, listing, retrieval, deletion, and entry retrieval
all delegate to the official SDK which handles auth headers, API
versioning (v2), and request formatting.
## AI Disclosure
- [ ] No AI tools were used in this PR
- [x] AI tools were used — details below
GitHub Copilot (claude-sonnet-4-20250514)
Copilot AI review requested due to automatic review settings April 29, 2026 16:31
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors the DeepL integration by replacing the custom cURL-based client with the official deeplcom/deepl-php SDK while preserving the existing array-based response shapes for callers.

Changes:

  • Add deeplcom/deepl-php dependency via Composer.
  • Rewrite DeepLApiClient to delegate translation + glossary operations to the official SDK and map SDK objects into the legacy array format.
  • Add unit tests covering DeepLApiClient translation and glossary behaviors; clean up related PHPStan baseline entries.

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
composer.json Adds the official DeepL PHP SDK dependency.
composer.lock Locks new SDK package and transitive dependencies.
lib/Utils/Engines/DeepL/DeepLApiClient.php Replaces custom HTTP implementation with SDK wrapper + array mapping.
phpstan-baseline.neon Removes baseline entries that were specific to the previous implementation.
tests/unit/Utils/Engines/DeepL/DeepLApiClientTest.php Adds unit coverage for the new SDK-backed client behavior.

Comment on lines +130 to +134
if ($entriesFormat === 'csv' && is_array($data['entries'])) {
// Convert 2D array of [source, target] pairs to CSV string
$csvLines = array_map(fn($row) => implode(',', $row), $data['entries']);
$csvContent = implode("\n", $csvLines);
$glossaryInfo = $this->translator->createGlossaryFromCsv($name, $sourceLang, $targetLang, $csvContent);
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

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

createGlossary() builds CSV payloads via implode(',', $row) / implode("\n", ...), which does not escape commas, quotes, or newlines inside terms. This can corrupt the CSV sent to DeepL and cause incorrect glossary entries or 400 responses for perfectly valid CSV content. Use proper CSV serialization (e.g., fputcsv() to an in-memory stream with , delimiter and " enclosure, and normalize newlines), and validate each row has exactly 2 scalar fields before encoding.

Copilot uses AI. Check for mistakes.
Comment on lines +124 to +142
try {
$name = $data['name'];
$sourceLang = $data['source_lang'];
$targetLang = $data['target_lang'];
$entriesFormat = $data['entries_format'] ?? 'tsv';

if ($entriesFormat === 'csv' && is_array($data['entries'])) {
// Convert 2D array of [source, target] pairs to CSV string
$csvLines = array_map(fn($row) => implode(',', $row), $data['entries']);
$csvContent = implode("\n", $csvLines);
$glossaryInfo = $this->translator->createGlossaryFromCsv($name, $sourceLang, $targetLang, $csvContent);
} elseif ($entriesFormat === 'tsv') {
$entries = is_string($data['entries'])
? GlossaryEntries::fromTsv($data['entries'])
: GlossaryEntries::fromEntries($data['entries']);
$glossaryInfo = $this->translator->createGlossary($name, $sourceLang, $targetLang, $entries);
} else {
// Fallback: send entries as CSV string
$csvContent = is_array($data['entries']) ? implode("\n", array_map(fn($row) => implode(',', $row), $data['entries'])) : $data['entries'];
Copy link

Copilot AI Apr 29, 2026

Choose a reason for hiding this comment

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

createGlossary() reads $data['name'], $data['source_lang'], $data['target_lang'], and $data['entries'] without checking they exist or have the expected types. If this method is called from any path other than the current controller, it will raise PHP warnings / type errors rather than a controlled DeepLApiException. Consider validating required keys up front (and that entries is string|array) and throwing an InvalidArgumentException/DeepLApiException with a clear message when the payload is invalid.

Suggested change
try {
$name = $data['name'];
$sourceLang = $data['source_lang'];
$targetLang = $data['target_lang'];
$entriesFormat = $data['entries_format'] ?? 'tsv';
if ($entriesFormat === 'csv' && is_array($data['entries'])) {
// Convert 2D array of [source, target] pairs to CSV string
$csvLines = array_map(fn($row) => implode(',', $row), $data['entries']);
$csvContent = implode("\n", $csvLines);
$glossaryInfo = $this->translator->createGlossaryFromCsv($name, $sourceLang, $targetLang, $csvContent);
} elseif ($entriesFormat === 'tsv') {
$entries = is_string($data['entries'])
? GlossaryEntries::fromTsv($data['entries'])
: GlossaryEntries::fromEntries($data['entries']);
$glossaryInfo = $this->translator->createGlossary($name, $sourceLang, $targetLang, $entries);
} else {
// Fallback: send entries as CSV string
$csvContent = is_array($data['entries']) ? implode("\n", array_map(fn($row) => implode(',', $row), $data['entries'])) : $data['entries'];
if (!array_key_exists('name', $data) || !is_string($data['name'])) {
throw new DeepLApiException('Invalid glossary payload: "name" is required and must be a string.');
}
if (!array_key_exists('source_lang', $data) || !is_string($data['source_lang'])) {
throw new DeepLApiException('Invalid glossary payload: "source_lang" is required and must be a string.');
}
if (!array_key_exists('target_lang', $data) || !is_string($data['target_lang'])) {
throw new DeepLApiException('Invalid glossary payload: "target_lang" is required and must be a string.');
}
if (!array_key_exists('entries', $data) || (!is_string($data['entries']) && !is_array($data['entries']))) {
throw new DeepLApiException('Invalid glossary payload: "entries" is required and must be a string or array.');
}
if (array_key_exists('entries_format', $data) && !is_string($data['entries_format'])) {
throw new DeepLApiException('Invalid glossary payload: "entries_format" must be a string when provided.');
}
try {
$name = $data['name'];
$sourceLang = $data['source_lang'];
$targetLang = $data['target_lang'];
$entries = $data['entries'];
$entriesFormat = $data['entries_format'] ?? 'tsv';
if ($entriesFormat === 'csv' && is_array($entries)) {
// Convert 2D array of [source, target] pairs to CSV string
$csvLines = array_map(fn($row) => implode(',', $row), $entries);
$csvContent = implode("\n", $csvLines);
$glossaryInfo = $this->translator->createGlossaryFromCsv($name, $sourceLang, $targetLang, $csvContent);
} elseif ($entriesFormat === 'tsv') {
$glossaryEntries = is_string($entries)
? GlossaryEntries::fromTsv($entries)
: GlossaryEntries::fromEntries($entries);
$glossaryInfo = $this->translator->createGlossary($name, $sourceLang, $targetLang, $glossaryEntries);
} else {
// Fallback: send entries as CSV string
$csvContent = is_array($entries) ? implode("\n", array_map(fn($row) => implode(',', $row), $entries)) : $entries;

Copilot uses AI. Check for mistakes.
@github-actions
Copy link
Copy Markdown

🧪 Test-Guard Report

✅ PASS — All changed source files have adequate test coverage.

Coverage Analysis: ❌ FAIL

No changed source files found in coverage report (threshold: 80%)

File Verdict Reason
lib/Utils/Engines/DeepL/DeepLApiClient.php ❌ fail not in coverage report

Test File Matching: ✅ PASS

File matching: 1 pass

File Verdict Reason
lib/Utils/Engines/DeepL/DeepLApiClient.php ✅ pass Test file modified in PR: tests/unit/Utils/Engines/DeepL/DeepLApiClientTest.php

Per-File Evaluation: ✅ PASS

Evaluated 1 files: 1 via AI (1 batch), 0 via shortcuts.

File Verdict Reason
lib/Utils/Engines/DeepL/DeepLApiClient.php ✅ pass Tests cover all new methods, error paths, and options including glossary formats.

Result: ✅ PASS

@mauretto78 mauretto78 merged commit 3ab8697 into develop Apr 30, 2026
23 of 25 checks passed
@mauretto78 mauretto78 deleted the deepl-glossary-fix branch April 30, 2026 08:59
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.

3 participants