68 - Fix duplicate posting + test suite fixes

This commit is contained in:
myrmidex 2026-02-25 23:22:05 +01:00
parent d3c44a4952
commit 8bc6e99f96
15 changed files with 324 additions and 124 deletions

View file

@ -6,13 +6,18 @@
use App\Events\ArticleApproved;
use App\Models\Setting;
use App\Services\Article\ValidationService;
use Exception;
use Illuminate\Contracts\Queue\ShouldQueue;
class ValidateArticleListener implements ShouldQueue
{
public string $queue = 'default';
public function handle(NewArticleFetched $event, ValidationService $validationService): void
public function __construct(
private ValidationService $validationService
) {}
public function handle(NewArticleFetched $event): void
{
$article = $event->article;
@ -20,12 +25,25 @@ public function handle(NewArticleFetched $event, ValidationService $validationSe
return;
}
// Only validate articles that are still pending
if (! $article->isPending()) {
return;
}
// Skip if already has publication (prevents duplicate processing)
if ($article->articlePublication()->exists()) {
return;
}
$article = $validationService->validate($article);
try {
$article = $this->validationService->validate($article);
} catch (Exception $e) {
logger()->error('Article validation failed', [
'article_id' => $article->id,
'error' => $e->getMessage(),
]);
return;
}
if ($article->isValid()) {
// Double-check publication doesn't exist (race condition protection)

View file

@ -130,10 +130,8 @@ public function feed(): BelongsTo
return $this->belongsTo(Feed::class);
}
protected static function booted(): void
public function dispatchFetchedEvent(): void
{
static::created(function ($article) {
event(new NewArticleFetched($article));
});
event(new NewArticleFetched($this));
}
}

View file

@ -42,6 +42,25 @@ public static function urlExists(PlatformEnum $platform, string $channelId, stri
->exists();
}
public static function duplicateExists(PlatformEnum $platform, string $channelId, ?string $url, ?string $title): bool
{
if (!$url && !$title) {
return false;
}
return self::where('platform', $platform)
->where('channel_id', $channelId)
->where(function ($query) use ($url, $title) {
if ($url) {
$query->orWhere('url', $url);
}
if ($title) {
$query->orWhere('title', $title);
}
})
->exists();
}
public static function storePost(PlatformEnum $platform, string $channelId, ?string $channelName, string $postId, ?string $url, ?string $title, ?\DateTime $postedAt = null): self
{
return self::updateOrCreate(

View file

@ -103,27 +103,27 @@ public function fetchArticleData(Article $article): array
private function saveArticle(string $url, ?int $feedId = null): Article
{
$existingArticle = Article::where('url', $url)->first();
if ($existingArticle) {
return $existingArticle;
}
// Extract a basic title from URL as fallback
$fallbackTitle = $this->generateFallbackTitle($url);
try {
return Article::create([
'url' => $url,
'feed_id' => $feedId,
'title' => $fallbackTitle,
]);
$article = Article::firstOrCreate(
['url' => $url],
[
'feed_id' => $feedId,
'title' => $fallbackTitle,
]
);
if ($article->wasRecentlyCreated) {
$article->dispatchFetchedEvent();
}
return $article;
} catch (\Exception $e) {
$this->logSaver->error("Failed to create article - title validation failed", null, [
$this->logSaver->error("Failed to create article", null, [
'url' => $url,
'feed_id' => $feedId,
'error' => $e->getMessage(),
'suggestion' => 'Check regex parsing patterns for title extraction'
]);
throw $e;
}
@ -134,12 +134,12 @@ private function generateFallbackTitle(string $url): string
// Extract filename from URL as a basic fallback title
$path = parse_url($url, PHP_URL_PATH);
$filename = basename($path ?: $url);
// Remove file extension and convert to readable format
$title = preg_replace('/\.[^.]*$/', '', $filename);
$title = str_replace(['-', '_'], ' ', $title);
$title = ucwords($title);
return $title ?: 'Untitled Article';
}
}

View file

@ -7,6 +7,7 @@
use App\Models\Article;
use App\Models\ArticlePublication;
use App\Models\PlatformChannel;
use App\Models\PlatformChannelPost;
use App\Models\Route;
use App\Modules\Lemmy\Services\LemmyPublisher;
use App\Services\Log\LogSaver;
@ -78,7 +79,7 @@ private function routeMatchesArticle(Route $route, array $extractedData): bool
{
// Get active keywords for this route
$activeKeywords = $route->keywords->where('is_active', true);
// If no keywords are defined for this route, the route matches any article
if ($activeKeywords->isEmpty()) {
return true;
@ -113,6 +114,23 @@ private function routeMatchesArticle(Route $route, array $extractedData): bool
private function publishToChannel(Article $article, array $extractedData, PlatformChannel $channel, mixed $account): ?ArticlePublication
{
try {
// Check if this URL or title was already posted to this channel
$title = $extractedData['title'] ?? $article->title;
if (PlatformChannelPost::duplicateExists(
$channel->platformInstance->platform,
(string) $channel->channel_id,
$article->url,
$title
)) {
$this->logSaver->info('Skipping duplicate: URL or title already posted to channel', $channel, [
'article_id' => $article->id,
'url' => $article->url,
'title' => $title,
]);
return null;
}
$publisher = $this->makePublisher($account);
$postData = $publisher->publishToChannel($article, $extractedData, $channel);

View file

@ -24,6 +24,7 @@ public function up(): void
$table->index(['published_at', 'approval_status']);
$table->index('feed_id');
$table->unique('url');
});
// Article publications table
@ -71,4 +72,4 @@ public function down(): void
Schema::dropIfExists('logs');
Schema::dropIfExists('settings');
}
};
};

View file

@ -64,20 +64,21 @@ public function up(): void
$table->unique(['platform_account_id', 'platform_channel_id'], 'account_channel_unique');
});
// Platform channel posts table
// Platform channel posts table (synced from platform APIs for duplicate detection)
Schema::create('platform_channel_posts', function (Blueprint $table) {
$table->id();
$table->foreignId('platform_channel_id')->constrained()->onDelete('cascade');
$table->string('platform');
$table->string('channel_id');
$table->string('channel_name')->nullable();
$table->string('post_id');
$table->string('title');
$table->text('content')->nullable();
$table->string('title')->nullable();
$table->string('url')->nullable();
$table->timestamp('posted_at');
$table->string('author');
$table->json('metadata')->nullable();
$table->timestamp('posted_at')->nullable();
$table->timestamps();
$table->unique(['platform_channel_id', 'post_id'], 'channel_post_unique');
$table->unique(['platform', 'channel_id', 'post_id'], 'channel_post_unique');
$table->index(['platform', 'channel_id', 'url']);
$table->index(['platform', 'channel_id', 'title']);
});
// Language platform instance pivot table
@ -102,4 +103,4 @@ public function down(): void
Schema::dropIfExists('platform_accounts');
Schema::dropIfExists('platform_instances');
}
};
};

View file

@ -38,3 +38,14 @@
});
require __DIR__.'/auth.php';
Route::get('/health', function () {
return response()->json(['status' => 'ok']);
});
Route::fallback(function () {
return response()->json([
'message' => 'This is the FFR API backend. Use /api/v1/* endpoints or check the React frontend.',
'api_base' => '/api/v1',
], 404);
});

View file

@ -84,11 +84,11 @@ public function test_sync_channel_posts_job_processes_successfully(): void
{
$channel = PlatformChannel::factory()->create();
$job = new SyncChannelPostsJob($channel);
// Test that job can be constructed and has correct properties
$this->assertEquals('sync', $job->queue);
$this->assertInstanceOf(SyncChannelPostsJob::class, $job);
// Don't actually run the job to avoid HTTP calls
$this->assertTrue(true);
}
@ -194,11 +194,10 @@ public function test_validate_article_listener_processes_new_article(): void
'full_article' => 'This is a test article about Belgium and Belgian politics.'
]);
$validationService = app(ValidationService::class);
$listener = new ValidateArticleListener();
$listener = app(ValidateArticleListener::class);
$event = new NewArticleFetched($article);
$listener->handle($event, $validationService);
$listener->handle($event);
$article->refresh();
$this->assertNotEquals('pending', $article->approval_status);

View file

@ -13,7 +13,7 @@ class NewArticleFetchedEventTest extends TestCase
{
use RefreshDatabase;
public function test_new_article_fetched_event_dispatched_on_article_creation(): void
public function test_new_article_fetched_event_dispatched_via_dispatch_method(): void
{
Event::fake([NewArticleFetched::class]);
@ -25,6 +25,8 @@ public function test_new_article_fetched_event_dispatched_on_article_creation():
'title' => 'Test Article',
]);
$article->dispatchFetchedEvent();
Event::assertDispatched(NewArticleFetched::class, function (NewArticleFetched $event) use ($article) {
return $event->article->id === $article->id;
});

View file

@ -34,11 +34,10 @@ public function test_listener_validates_article_and_dispatches_ready_to_publish_
'approval_status' => 'pending',
]);
$listener = new ValidateArticleListener();
$listener = app(ValidateArticleListener::class);
$event = new NewArticleFetched($article);
$validationService = app(ValidationService::class);
$listener->handle($event, $validationService);
$listener->handle($event);
$article->refresh();
@ -62,11 +61,10 @@ public function test_listener_skips_already_validated_articles(): void
'approval_status' => 'approved',
]);
$listener = new ValidateArticleListener();
$listener = app(ValidateArticleListener::class);
$event = new NewArticleFetched($article);
$validationService = app(ValidationService::class);
$listener->handle($event, $validationService);
$listener->handle($event);
Event::assertNotDispatched(ArticleReadyToPublish::class);
}
@ -90,11 +88,10 @@ public function test_listener_skips_articles_with_existing_publication(): void
'published_by' => 'test-user',
]);
$listener = new ValidateArticleListener();
$listener = app(ValidateArticleListener::class);
$event = new NewArticleFetched($article);
$validationService = app(ValidationService::class);
$listener->handle($event, $validationService);
$listener->handle($event);
Event::assertNotDispatched(ArticleReadyToPublish::class);
}
@ -115,11 +112,10 @@ public function test_listener_calls_validation_service(): void
'approval_status' => 'pending',
]);
$listener = new ValidateArticleListener();
$listener = app(ValidateArticleListener::class);
$event = new NewArticleFetched($article);
$validationService = app(ValidationService::class);
$listener->handle($event, $validationService);
$listener->handle($event);
// Verify that the article was processed by ValidationService
$article->refresh();

View file

@ -19,12 +19,12 @@ class ArticleTest extends TestCase
protected function setUp(): void
{
parent::setUp();
// Mock HTTP requests to prevent external calls
Http::fake([
'*' => Http::response('', 500)
]);
// Don't fake events globally - let individual tests control this
}
@ -180,18 +180,17 @@ public function test_feed_relationship(): void
$this->assertEquals($feed->id, $article->feed->id);
}
public function test_article_creation_fires_new_article_fetched_event(): void
public function test_dispatch_fetched_event_fires_new_article_fetched_event(): void
{
$eventFired = false;
// Listen for the event using a closure
Event::listen(NewArticleFetched::class, function ($event) use (&$eventFired) {
$eventFired = true;
});
$feed = Feed::factory()->create();
Article::factory()->create(['feed_id' => $feed->id]);
Event::fake([NewArticleFetched::class]);
$this->assertTrue($eventFired, 'NewArticleFetched event was not fired');
$feed = Feed::factory()->create();
$article = Article::factory()->create(['feed_id' => $feed->id]);
$article->dispatchFetchedEvent();
Event::assertDispatched(NewArticleFetched::class, function ($event) use ($article) {
return $event->article->id === $article->id;
});
}
}
}

View file

@ -3,20 +3,15 @@
namespace Tests\Unit\Modules\Lemmy\Services;
use App\Modules\Lemmy\Services\LemmyApiService;
use App\Models\PlatformChannelPost;
use App\Enums\PlatformEnum;
use Illuminate\Foundation\Testing\RefreshDatabase;
use Illuminate\Support\Facades\Http;
use Illuminate\Support\Facades\Log;
use Tests\TestCase;
use Mockery;
use Exception;
class LemmyApiServiceTest extends TestCase
{
protected function tearDown(): void
{
parent::tearDown();
}
use RefreshDatabase;
public function test_constructor_sets_instance(): void
{
@ -100,8 +95,6 @@ public function test_login_returns_null_on_unsuccessful_response(): void
'*' => Http::response(['error' => 'Invalid credentials'], 401)
]);
Log::shouldReceive('error')->twice(); // Once for HTTPS, once for HTTP fallback
$service = new LemmyApiService('lemmy.world');
$token = $service->login('user', 'wrong');
@ -114,19 +107,12 @@ public function test_login_handles_rate_limit_error(): void
'*' => Http::response('{"error":"rate_limit_error"}', 429)
]);
// Expecting 4 error logs:
// 1. 'Lemmy login failed' for HTTPS attempt
// 2. 'Lemmy login exception' for catching the rate limit exception on HTTPS
// 3. 'Lemmy login failed' for HTTP attempt
// 4. 'Lemmy login exception' for catching the rate limit exception on HTTP
Log::shouldReceive('error')->times(4);
$service = new LemmyApiService('lemmy.world');
$result = $service->login('user', 'pass');
// Since the exception is caught and HTTP is tried, then that also fails,
// the method returns null instead of throwing
$this->assertNull($result);
$this->expectException(Exception::class);
$this->expectExceptionMessage('Rate limited');
$service->login('user', 'pass');
}
public function test_login_returns_null_when_jwt_missing_from_response(): void
@ -141,18 +127,18 @@ public function test_login_returns_null_when_jwt_missing_from_response(): void
$this->assertNull($token);
}
public function test_login_handles_exception_and_returns_null(): void
public function test_login_handles_exception_and_throws(): void
{
Http::fake(function () {
throw new Exception('Network error');
});
Log::shouldReceive('error')->twice();
$service = new LemmyApiService('lemmy.world');
$token = $service->login('user', 'pass');
$this->assertNull($token);
$this->expectException(Exception::class);
$this->expectExceptionMessage('Connection failed');
$service->login('user', 'pass');
}
public function test_get_community_id_success(): void
@ -183,8 +169,6 @@ public function test_get_community_id_throws_on_unsuccessful_response(): void
'*' => Http::response('Not found', 404)
]);
Log::shouldReceive('error')->once();
$service = new LemmyApiService('lemmy.world');
$this->expectException(Exception::class);
@ -199,8 +183,6 @@ public function test_get_community_id_throws_when_community_not_in_response(): v
'*' => Http::response(['success' => true], 200)
]);
Log::shouldReceive('error')->once();
$service = new LemmyApiService('lemmy.world');
$this->expectException(Exception::class);
@ -234,21 +216,6 @@ public function test_sync_channel_posts_success(): void
], 200)
]);
Log::shouldReceive('info')->once()->with('Synced channel posts', Mockery::any());
$mockPost = Mockery::mock('alias:' . PlatformChannelPost::class);
$mockPost->shouldReceive('storePost')
->twice()
->with(
PlatformEnum::LEMMY,
Mockery::any(),
'test-community',
Mockery::any(),
Mockery::any(),
Mockery::any(),
Mockery::any()
);
$service = new LemmyApiService('lemmy.world');
$service->syncChannelPosts('token', 42, 'test-community');
@ -258,6 +225,25 @@ public function test_sync_channel_posts_success(): void
&& str_contains($request->url(), 'limit=50')
&& str_contains($request->url(), 'sort=New');
});
// Verify posts were stored in the database
$this->assertDatabaseHas('platform_channel_posts', [
'platform' => PlatformEnum::LEMMY->value,
'channel_id' => '42',
'channel_name' => 'test-community',
'post_id' => '1',
'url' => 'https://example.com/1',
'title' => 'Post 1',
]);
$this->assertDatabaseHas('platform_channel_posts', [
'platform' => PlatformEnum::LEMMY->value,
'channel_id' => '42',
'channel_name' => 'test-community',
'post_id' => '2',
'url' => 'https://example.com/2',
'title' => 'Post 2',
]);
}
public function test_sync_channel_posts_handles_unsuccessful_response(): void
@ -266,12 +252,11 @@ public function test_sync_channel_posts_handles_unsuccessful_response(): void
'*' => Http::response('Error', 500)
]);
Log::shouldReceive('warning')->once()->with('Failed to sync channel posts', Mockery::any());
$service = new LemmyApiService('lemmy.world');
$service->syncChannelPosts('token', 42, 'test-community');
Http::assertSentCount(1);
$this->assertDatabaseCount('platform_channel_posts', 0);
}
public function test_sync_channel_posts_handles_exception(): void
@ -280,8 +265,6 @@ public function test_sync_channel_posts_handles_exception(): void
throw new Exception('Network error');
});
Log::shouldReceive('error')->once()->with('Exception while syncing channel posts', Mockery::any());
$service = new LemmyApiService('lemmy.world');
$service->syncChannelPosts('token', 42, 'test-community');
@ -354,8 +337,6 @@ public function test_create_post_throws_on_unsuccessful_response(): void
'*' => Http::response('Forbidden', 403)
]);
Log::shouldReceive('error')->once();
$service = new LemmyApiService('lemmy.world');
$this->expectException(Exception::class);
@ -393,8 +374,6 @@ public function test_get_languages_returns_empty_array_on_failure(): void
'*' => Http::response('Error', 500)
]);
Log::shouldReceive('warning')->once();
$service = new LemmyApiService('lemmy.world');
$languages = $service->getLanguages();
@ -407,8 +386,6 @@ public function test_get_languages_handles_exception(): void
throw new Exception('Network error');
});
Log::shouldReceive('error')->once();
$service = new LemmyApiService('lemmy.world');
$languages = $service->getLanguages();

View file

@ -9,6 +9,7 @@
use App\Models\Feed;
use App\Models\PlatformAccount;
use App\Models\PlatformChannel;
use App\Models\PlatformChannelPost;
use App\Models\PlatformInstance;
use App\Models\Route;
use App\Modules\Lemmy\Services\LemmyPublisher;
@ -105,7 +106,7 @@ public function test_publish_to_routed_channels_successfully_publishes_to_channe
// Arrange
$feed = Feed::factory()->create();
$article = Article::factory()->create(['feed_id' => $feed->id, 'approval_status' => 'approved']);
$platformInstance = PlatformInstance::factory()->create();
$channel = PlatformChannel::factory()->create(['platform_instance_id' => $platformInstance->id]);
$account = PlatformAccount::factory()->create();
@ -151,7 +152,7 @@ public function test_publish_to_routed_channels_handles_publishing_failure_grace
// Arrange
$feed = Feed::factory()->create();
$article = Article::factory()->create(['feed_id' => $feed->id, 'approval_status' => 'approved']);
$platformInstance = PlatformInstance::factory()->create();
$channel = PlatformChannel::factory()->create(['platform_instance_id' => $platformInstance->id]);
$account = PlatformAccount::factory()->create();
@ -296,4 +297,162 @@ public function test_publish_to_routed_channels_filters_out_failed_publications(
$this->assertDatabaseHas('article_publications', ['post_id' => 300]);
$this->assertDatabaseCount('article_publications', 1);
}
public function test_publish_skips_duplicate_when_url_already_posted_to_channel(): void
{
// Arrange
$feed = Feed::factory()->create();
$article = Article::factory()->create([
'feed_id' => $feed->id,
'approval_status' => 'approved',
'url' => 'https://example.com/article-1',
]);
$platformInstance = PlatformInstance::factory()->create(['platform' => 'lemmy']);
$channel = PlatformChannel::factory()->create(['platform_instance_id' => $platformInstance->id]);
$account = PlatformAccount::factory()->create();
Route::create([
'feed_id' => $feed->id,
'platform_channel_id' => $channel->id,
'is_active' => true,
'priority' => 50,
]);
$channel->platformAccounts()->attach($account->id, [
'is_active' => true,
'priority' => 50,
]);
// Simulate the URL already being posted to this channel (synced from Lemmy)
PlatformChannelPost::storePost(
PlatformEnum::LEMMY,
(string) $channel->channel_id,
$channel->name,
'999',
'https://example.com/article-1',
'Different Title',
);
// Publisher should never be called
$publisherDouble = \Mockery::mock(LemmyPublisher::class);
$publisherDouble->shouldNotReceive('publishToChannel');
$service = \Mockery::mock(ArticlePublishingService::class, [$this->logSaver])->makePartial();
$service->shouldAllowMockingProtectedMethods();
$service->shouldReceive('makePublisher')->andReturn($publisherDouble);
// Act
$result = $service->publishToRoutedChannels($article, ['title' => 'Some Title']);
// Assert
$this->assertTrue($result->isEmpty());
$this->assertDatabaseCount('article_publications', 0);
}
public function test_publish_skips_duplicate_when_title_already_posted_to_channel(): void
{
// Arrange
$feed = Feed::factory()->create();
$article = Article::factory()->create([
'feed_id' => $feed->id,
'approval_status' => 'approved',
'url' => 'https://example.com/article-new-url',
'title' => 'Breaking News: Something Happened',
]);
$platformInstance = PlatformInstance::factory()->create(['platform' => 'lemmy']);
$channel = PlatformChannel::factory()->create(['platform_instance_id' => $platformInstance->id]);
$account = PlatformAccount::factory()->create();
Route::create([
'feed_id' => $feed->id,
'platform_channel_id' => $channel->id,
'is_active' => true,
'priority' => 50,
]);
$channel->platformAccounts()->attach($account->id, [
'is_active' => true,
'priority' => 50,
]);
// Simulate the same title already posted with a different URL
PlatformChannelPost::storePost(
PlatformEnum::LEMMY,
(string) $channel->channel_id,
$channel->name,
'888',
'https://example.com/different-url',
'Breaking News: Something Happened',
);
// Publisher should never be called
$publisherDouble = \Mockery::mock(LemmyPublisher::class);
$publisherDouble->shouldNotReceive('publishToChannel');
$service = \Mockery::mock(ArticlePublishingService::class, [$this->logSaver])->makePartial();
$service->shouldAllowMockingProtectedMethods();
$service->shouldReceive('makePublisher')->andReturn($publisherDouble);
// Act
$result = $service->publishToRoutedChannels($article, ['title' => 'Breaking News: Something Happened']);
// Assert
$this->assertTrue($result->isEmpty());
$this->assertDatabaseCount('article_publications', 0);
}
public function test_publish_proceeds_when_no_duplicate_exists(): void
{
// Arrange
$feed = Feed::factory()->create();
$article = Article::factory()->create([
'feed_id' => $feed->id,
'approval_status' => 'approved',
'url' => 'https://example.com/unique-article',
]);
$platformInstance = PlatformInstance::factory()->create(['platform' => 'lemmy']);
$channel = PlatformChannel::factory()->create(['platform_instance_id' => $platformInstance->id]);
$account = PlatformAccount::factory()->create();
Route::create([
'feed_id' => $feed->id,
'platform_channel_id' => $channel->id,
'is_active' => true,
'priority' => 50,
]);
$channel->platformAccounts()->attach($account->id, [
'is_active' => true,
'priority' => 50,
]);
// Existing post in the channel has a completely different URL and title
PlatformChannelPost::storePost(
PlatformEnum::LEMMY,
(string) $channel->channel_id,
$channel->name,
'777',
'https://example.com/other-article',
'Totally Different Title',
);
$publisherDouble = \Mockery::mock(LemmyPublisher::class);
$publisherDouble->shouldReceive('publishToChannel')
->once()
->andReturn(['post_view' => ['post' => ['id' => 456]]]);
$service = \Mockery::mock(ArticlePublishingService::class, [$this->logSaver])->makePartial();
$service->shouldAllowMockingProtectedMethods();
$service->shouldReceive('makePublisher')->andReturn($publisherDouble);
// Act
$result = $service->publishToRoutedChannels($article, ['title' => 'Unique Title']);
// Assert
$this->assertCount(1, $result);
$this->assertDatabaseHas('article_publications', [
'article_id' => $article->id,
'post_id' => 456,
]);
}
}

View file

@ -3,15 +3,17 @@
namespace Tests\Unit\Services;
use App\Services\SystemStatusService;
use Illuminate\Foundation\Testing\RefreshDatabase;
use Tests\TestCase;
use Illuminate\Support\Facades\Http;
class SystemStatusServiceTest extends TestCase
{
use RefreshDatabase;
protected function setUp(): void
{
parent::setUp();
// Mock HTTP requests to prevent external calls
Http::fake([
'*' => Http::response('', 500)
@ -34,11 +36,11 @@ public function test_get_system_status_returns_correct_structure(): void
$this->assertArrayHasKey('status', $status);
$this->assertArrayHasKey('status_class', $status);
$this->assertArrayHasKey('reasons', $status);
// Without database setup, system should be disabled
$this->assertFalse($status['is_enabled']);
$this->assertEquals('Disabled', $status['status']);
$this->assertEquals('text-red-600', $status['status_class']);
$this->assertIsArray($status['reasons']);
}
}
}