Skip to content
Merged

Oo #49

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,13 @@ All endpoints accept **HTTP GET** requests. The API uses a single entry point (`
| `/api.php?get=revids` | GET | MDWiki revision IDs for titles |
| `/api.php?get=settings` | GET | System configuration settings |
| `/api.php?get=statics_by_category` | GET | Count of existing articles grouped by language for a category |
| `/api.php?get=status` | GET | Page publication counts by month (optional filters) |
| `/api.php?get=titles` | GET | Page titles with assessments, refs, views, QIDs, words |
| `/api.php?get=top_lang_of_users` | GET | Top language per user (by page count) |
| `/api.php?get=top_langs` | GET | Top languages by targets, words, views |
| `/api.php?get=top_users` | GET | Top users by targets, words, views |
| `/api.php?get=translate_type` | GET | Translation types (lead/full) |
| `/api.php?get=user_access` | GET | Access keys with usernames |
| `/api.php?get=user_lang_status` | GET | User status by language (redirects to user_status) |
| `/api.php?get=get_lang_years` | GET | User status by language (redirects to user_status) |

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🟡 This description is inaccurate for get_lang_years.

It's copied from the old user_lang_status alias, but get_lang_years no longer redirects — it's a real endpoint returning the distinct years present in pages, optionally filtered by lang. Something like:

| `/api.php?get=lang_years` | GET | Distinct years present in pages (optional lang filter) |

Also consider the endpoint name itself: every other endpoint is a bare noun (langs, pages, views, …), so get_lang_years reads like a function call. lang_years would match the house style — and this row is also out of alphabetical order, since it was left where user_lang_status used to sit.

| `/api.php?get=user_status` | GET | User status by language with year/select options |
| `/api.php?get=user_views2` | GET | Page views filtered by a specific user (alias) |
| `/api.php?get=user_views` | GET | Page views filtered by a specific user |
Expand Down
3 changes: 1 addition & 2 deletions _works_files/tree.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,11 @@ src/
│ ├── request.php
│ ├── select_helps.php
│ ├── sql.php
│ └── status.php
├── api.php
├── bootstrap.php
├── index.php
├── openapi.json
├── README.md
└── request.php

```
```
157 changes: 90 additions & 67 deletions docs/new-structure-oc.md
Original file line number Diff line number Diff line change
Expand Up @@ -176,17 +176,20 @@ td_api/
**Purpose**: Contains the core business logic, independent of any external frameworks or infrastructure.

**Key Components**:
- **Entities**: Objects with identity (Page, User, Translation)
- **Value Objects**: Immutable objects without identity (Language, Qid)
- **Repository Interfaces**: Contracts defining data access requirements
- **Domain Services**: Complex business operations that don't fit in entities

- **Entities**: Objects with identity (Page, User, Translation)
- **Value Objects**: Immutable objects without identity (Language, Qid)
- **Repository Interfaces**: Contracts defining data access requirements
- **Domain Services**: Complex business operations that don't fit in entities

**Benefits**:
- Business logic is isolated and testable without database
- Changes to infrastructure don't affect business rules
- Clear boundaries make the codebase easier to understand

- Business logic is isolated and testable without database
- Changes to infrastructure don't affect business rules
- Clear boundaries make the codebase easier to understand

**Migration Example**:

```php
// Current approach (app/request.php)
switch ($get) {
Expand All @@ -209,39 +212,44 @@ class GetPagesAction {
**Purpose**: Orchestrates use cases by coordinating domain objects. No business rules here, only coordination.

**Key Components**:
- **Actions/Commands**: Handle write operations
- **Queries**: Handle read operations (CQRS pattern)
- **DTOs**: Data structures for crossing layer boundaries
- **Validators**: Input validation and sanitization

- **Actions/Commands**: Handle write operations
- **Queries**: Handle read operations (CQRS pattern)
- **DTOs**: Data structures for crossing layer boundaries
- **Validators**: Input validation and sanitization

**Benefits**:
- Clear entry points for each use case
- Request validation separated from business logic
- Easier to add new features without modifying existing code

- Clear entry points for each use case
- Request validation separated from business logic
- Easier to add new features without modifying existing code

### 3. Infrastructure Layer (`src/Infrastructure/`)

**Purpose**: Contains all technical details and external concerns.

**Key Components**:
- **Database**: Concrete repository implementations, connection management
- **Cache**: APCu and other caching implementations
- **Http**: Controllers, middleware, request/response handling
- **External**: Third-party API integrations

- **Database**: Concrete repository implementations, connection management
- **Cache**: APCu and other caching implementations
- **Http**: Controllers, middleware, request/response handling
- **External**: Third-party API integrations

**Benefits**:
- Technical details are isolated
- Easy to swap implementations (e.g., APCu → Redis)
- Database schema changes only affect repository implementations

- Technical details are isolated
- Easy to swap implementations (e.g., APCu → Redis)
- Database schema changes only affect repository implementations

### 4. Shared Kernel (`src/Shared/`)

**Purpose**: Common utilities used across all layers.

**Migration Path**:
- Move `app/helps.php` functions to `src/Shared/Sanitizers.php`
- Refactor into static utility classes with clear responsibilities
- Maintain backward compatibility with gradual migration

- Move `app/helps.php` functions to `src/Shared/Sanitizers.php`
- Refactor into static utility classes with clear responsibilities
- Maintain backward compatibility with gradual migration

---

Expand All @@ -252,6 +260,7 @@ class GetPagesAction {
**Current Risk**: All files in the web root are accessible via HTTP, including `app/sql.php` which contains database credentials.

**Solution**:

```
# Apache/Nginx configuration
DocumentRoot /var/www/td_api/public
Expand All @@ -260,19 +269,22 @@ DocumentRoot /var/www/td_api/public
```

**Benefits**:
- Source code files are not directly accessible
- Configuration files protected from HTTP access
- Prevents accidental exposure of sensitive data

- Source code files are not directly accessible
- Configuration files protected from HTTP access
- Prevents accidental exposure of sensitive data

### 2. Configuration Management

**Current Approach**:

```php
// Hardcoded in sql.php
$ts_mycnf = parse_ini_file($this->home_dir . "/confs/db.ini");
```

**Proposed Approach**:

```php
// config/database.php
return [
Expand All @@ -292,20 +304,23 @@ return [
```

**Benefits**:
- Environment-specific configuration via `.env` files
- No hardcoded credentials in source code
- Easy to manage different environments (dev, staging, production)

- Environment-specific configuration via `.env` files
- No hardcoded credentials in source code
- Easy to manage different environments (dev, staging, production)

### 3. Autoloading and Namespaces

**Current**: Manual includes in `app/bootstrap.php`

```php
include_once __DIR__ . '/helps.php';
include_once __DIR__ . '/sql.php';
// ... 12 more includes
```

**Proposed**: PSR-4 autoloading via Composer

```json
{
"autoload": {
Expand All @@ -317,15 +332,17 @@ include_once __DIR__ . '/sql.php';
```

**Benefits**:
- No manual include/require statements
- Class loading on-demand
- Namespace-based organization

- No manual include/require statements
- Class loading on-demand
- Namespace-based organization

### 4. Testing Infrastructure

**Current**: Frontend test files (`test/`, `test2/`) mixed with source

**Proposed**: Comprehensive test suite

```
tests/
├── Unit/ # Fast, isolated tests for domain logic
Expand All @@ -334,16 +351,18 @@ tests/
```

**Benefits**:
- Regression prevention
- Confidence when refactoring
- Documentation via tests
- PHPUnit integration for CI/CD

- Regression prevention
- Confidence when refactoring
- Documentation via tests
- PHPUnit integration for CI/CD

### 5. API Documentation

**Current**: `openapi.html` at root, `openapi.json` separate

**Proposed**: Organized documentation

```
docs/
├── api/ # API usage guides
Expand Down Expand Up @@ -407,21 +426,20 @@ resources/schemas/ # OpenAPI JSON specs

## File Mapping: Current → Proposed

| Current Location | Proposed Location | Rationale |
|-----------------|-------------------|-----------|
| `app/request.php` | `src/Infrastructure/Http/Controllers/ApiController.php` | HTTP handling belongs in infrastructure |
| `app/sql.php` | `src/Infrastructure/Database/` | Database access is infrastructure concern |
| `app/helps.php` | `src/Shared/Sanitizers.php`, `src/Application/Validators/` | Split into shared utilities and validators |
| `app/QueryBuilder.php` | `src/Infrastructure/Database/QueryBuilders/` | Specific builders for each entity |
| `app/langs/*` | `src/Domain/ValueObjects/Language.php`, `resources/lang/` | Language as value object + translations |
| `app/subs/*` | `src/Infrastructure/Database/QueryBuilders/` | Query building logic |
| `app/leaderboard.php` | `src/Application/Actions/GetLeaderboardAction.php` | Use case in application layer |
| `app/status.php` | `src/Application/Queries/GetTranslationStatusQuery.php` | CQRS query |
| `endpoint_params.json` | `config/endpoints/params.php` | PHP config for better caching |
| `index.php`, `api.php` | `public/index.php` | Single entry point with routing |
| `openapi.html` | `public/swagger-ui/index.html` | Organized documentation |
| `test/`, `test2/` | `tests/Functional/`, `public/assets/` | Separate test suite from frontend assets |
| `x/` | `bin/`, `var/temp/` | Scripts in bin/, temp files in var/ |
| Current Location | Proposed Location | Rationale |
| ---------------------- | ---------------------------------------------------------- | ------------------------------------------ |
| `app/request.php` | `src/Infrastructure/Http/Controllers/ApiController.php` | HTTP handling belongs in infrastructure |
| `app/sql.php` | `src/Infrastructure/Database/` | Database access is infrastructure concern |
| `app/helps.php` | `src/Shared/Sanitizers.php`, `src/Application/Validators/` | Split into shared utilities and validators |
| `app/QueryBuilder.php` | `src/Infrastructure/Database/QueryBuilders/` | Specific builders for each entity |
| `app/langs/*` | `src/Domain/ValueObjects/Language.php`, `resources/lang/` | Language as value object + translations |
| `app/subs/*` | `src/Infrastructure/Database/QueryBuilders/` | Query building logic |
| `app/leaderboard.php` | `src/Application/Actions/GetLeaderboardAction.php` | Use case in application layer |
| `endpoint_params.json` | `config/endpoints/params.php` | PHP config for better caching |
| `index.php`, `api.php` | `public/index.php` | Single entry point with routing |
| `openapi.html` | `public/swagger-ui/index.html` | Organized documentation |
| `test/`, `test2/` | `tests/Functional/`, `public/assets/` | Separate test suite from frontend assets |
| `x/` | `bin/`, `var/temp/` | Scripts in bin/, temp files in var/ |

---

Expand Down Expand Up @@ -491,29 +509,34 @@ server {
## Benefits Summary

### Maintainability
- **Clear separation of concerns**: Each layer has a single responsibility
- **Easy to navigate**: Developers know exactly where to find code
- **Reduced cognitive load**: Smaller, focused classes instead of large files

- **Clear separation of concerns**: Each layer has a single responsibility
- **Easy to navigate**: Developers know exactly where to find code
- **Reduced cognitive load**: Smaller, focused classes instead of large files

### Testability
- **Domain layer**: Unit testable without database
- **Infrastructure layer**: Mockable interfaces for testing
- **Integration tests**: Test database queries in isolation

- **Domain layer**: Unit testable without database
- **Infrastructure layer**: Mockable interfaces for testing
- **Integration tests**: Test database queries in isolation

### Scalability
- **Horizontal scaling**: Stateless design allows multiple servers
- **Caching strategy**: Centralized cache layer easy to optimize
- **Database optimization**: Repository pattern allows query optimization without affecting business logic

- **Horizontal scaling**: Stateless design allows multiple servers
- **Caching strategy**: Centralized cache layer easy to optimize
- **Database optimization**: Repository pattern allows query optimization without affecting business logic

### Security
- **Defense in depth**: Web root isolation prevents source code exposure
- **Input validation**: Centralized validation layer
- **Environment isolation**: Configuration separated from code

- **Defense in depth**: Web root isolation prevents source code exposure
- **Input validation**: Centralized validation layer
- **Environment isolation**: Configuration separated from code

### Developer Experience
- **IDE support**: PSR-4 autoloading enables better autocomplete
- **Type safety**: Gradual introduction of type hints
- **Documentation**: Clear architecture boundaries serve as documentation

- **IDE support**: PSR-4 autoloading enables better autocomplete
- **Type safety**: Gradual introduction of type hints
- **Documentation**: Clear architecture boundaries serve as documentation

---

Expand Down
4 changes: 3 additions & 1 deletion src/app/APIController.php
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,9 @@ public function handleRequest(): void
} catch (Throwable $e) {
error_log('[API] ' . $e->getMessage());
http_response_code(500);
$this->emit($this->builder->errorOnly('internal error'));
$result = $this->builder->errorOnly('internal error');
$result["e"] = $e->getMessage();

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

🔴 Security — raw exception messages are now echoed to API clients.

$e->getMessage() for a PDOException / DatabaseException typically contains the failing SQL and schema details (e.g. Unknown column 'p.campaign' in 'where clause'), and other throwables leak file/class/method internals. For a public API this is information disclosure, and it also contradicts the deliberate errorOnly('internal error') user-facing message.

The codebase already has a gate for exactly this — ResponseBuilder::build() only exposes query when Environment::isLocalhost(). Reuse it:

$result = $this->builder->errorOnly('internal error');
if (Environment::isLocalhost()) {
    $result['e'] = $e->getMessage();
}
$this->emit($result);

(Side note: this PR also removed the Logger::error() doc-comment stating internal details never reach the user — worth preserving that intent.)

$this->emit($result);
}
}

Expand Down
1 change: 1 addition & 0 deletions src/app/Database/Database.php
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ public function fetchQuery(string $sqlQuery, ?array $params = null): array
return $stmt->fetchAll(PDO::FETCH_ASSOC);
} catch (PDOException $e) {
Logger::error('SQL Error in fetchQuery: ' . $e->getMessage() . ' | Query: ' . $sqlQuery);
Logger::debug('SQL Error in fetchQuery: Query: ' . $sqlQuery);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Nit: this Logger::debug(...) line duplicates the Logger::error(...) two lines above it, which already logs the message and the full query unconditionally. This one logs a strict subset (query only) and only in development — it adds nothing, so it can go.

throw new DatabaseException('Query failed', 0, $e);
}
}
Expand Down
35 changes: 35 additions & 0 deletions src/app/Endpoints/EndpointContext.php
Original file line number Diff line number Diff line change
@@ -1,15 +1,19 @@
<?php
// src/app/Endpoints/EndpointContext.php
declare(strict_types=1);

namespace App\Endpoints;

use App\Query\Ordering;
use App\Query\FilterBuilder;
use App\Query\SelectBuilder;
use App\Http\Request;
use App\Query\InputSanitizer;


final class EndpointContext
{
private const PATTERN = '/^[A-Za-z0-9- ]+$/';
public readonly array $params;
public readonly array $columns;
public readonly string $select;
Expand All @@ -23,8 +27,10 @@ public function __construct(
public readonly Request $request,
) {
$this->params = $data['params'] ?? [];

$this->columns = $data['columns'] ?? [];
$this->select = SelectBuilder::build($this->params, $this->columns, $request);

$this->distinct = $request->enabled('distinct') ? 'DISTINCT ' : '';
$this->group = $request->get('group');
$this->order = $request->get('order');
Expand All @@ -40,4 +46,33 @@ public function applyGroup(string $sql): string
{
return Ordering::group($sql, $this->data, $this->group);
}

/** @return array{0: string, 1: array} [sql, params] */
public function applyCampaignCategory(
string $sql,
array $params
): array {

// Apply campaign/category filters
$campaign = InputSanitizer::match($this->request->get('campaign') ?? '', self::PATTERN);
$category = InputSanitizer::match(
$this->request->get('category') ?? $this->request->get('cat') ?? '',
self::PATTERN
);

$glue = FilterBuilder::glue($sql);

if (FilterBuilder::isValid($category)) {
$sql .= "$glue p.cat = ?";
$params[] = $category;
} elseif (FilterBuilder::isValid($campaign)) {
$sql .= "$glue ca.campaign = ?";
$params[] = $campaign;
}
return [$sql, $params];
}
public function isValid(string $value): bool
{
return FilterBuilder::isValid($value);
}
}
Loading
Loading