feat(content-pipeline): implement content pipeline and creator publishing system #252
Conversation
undead2146
changed the title
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…tor publishing system - Standardize content discovery, resolution, and acquisition across multiple sources (GitHub, ModDB, CNCLabs, AODMaps). - Introduce Creator Publishing system with self-hosted JSON catalogs and subscriptions. - Redesign Downloads browser with publisher-specific filters and sidebar navigation. - Implement Playwright-based scraping for robust web discovery. - Extensive documentation and architectural flowcharts for the new system.
undead2146
force-pushed
the
ui/downloads
branch
from
d10c082 to
e662cd1
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@greptile Review this PR regarding the overal infrastructure and implementation. |
| private readonly ILogger<PublisherSubscriptionStore> _logger; | ||
| private readonly IConfigurationProviderService _configurationProvider; | ||
| private readonly string _subscriptionsFilePath; | ||
| private readonly SemaphoreSlim _fileLock = new(1, 1); |
There was a problem hiding this comment.
style: instance-level _fileLock SemaphoreSlim is never disposed
The _fileLock semaphore is created as an instance field but PublisherSubscriptionStore doesn't implement IDisposable or IAsyncDisposable. This causes a resource leak. Since the class is registered as a singleton (ContentPipelineModule.cs:127), the lock will live for the application lifetime, but it's still best practice to implement disposal.
| private readonly SemaphoreSlim _fileLock = new(1, 1); | |
| private readonly SemaphoreSlim _fileLock = new(1, 1); |
Add IAsyncDisposable to the class and dispose the lock:
public class PublisherSubscriptionStore : IPublisherSubscriptionStore, IAsyncDisposable
{
// ... existing code ...
public async ValueTask DisposeAsync()
{
_fileLock.Dispose();
GC.SuppressFinalize(this);
}
}Prompt To Fix With AI
This is a comment left during a code review.
Path: GenHub/GenHub/Features/Content/Services/Catalog/PublisherSubscriptionStore.cs
Line: 27:27
Comment:
**style:** instance-level `_fileLock` SemaphoreSlim is never disposed
The `_fileLock` semaphore is created as an instance field but `PublisherSubscriptionStore` doesn't implement `IDisposable` or `IAsyncDisposable`. This causes a resource leak. Since the class is registered as a singleton (ContentPipelineModule.cs:127), the lock will live for the application lifetime, but it's still best practice to implement disposal.
```suggestion
private readonly SemaphoreSlim _fileLock = new(1, 1);
```
Add `IAsyncDisposable` to the class and dispose the lock:
```csharp
public class PublisherSubscriptionStore : IPublisherSubscriptionStore, IAsyncDisposable
{
// ... existing code ...
public async ValueTask DisposeAsync()
{
_fileLock.Dispose();
GC.SuppressFinalize(this);
}
}
```
How can I resolve this? If you propose a fix, please make it concise.
| } | ||
|
|
||
| /// <inheritdoc /> | ||
| public string ParserId => "ModDB"; |
There was a problem hiding this comment.
hardcoded "ModDB" string should use ModDBConstants.PublisherType or define a parser ID constant
Per constants.md style guide, string literals should be defined in dedicated constants classes.
Context Used: Context from dashboard - Use dedicated constants classes instead of hardcoding constants string, integers or variables in ser... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: GenHub/GenHub/Features/Content/Services/Parsers/ModDBPageParser.cs
Line: 610:610
Comment:
hardcoded `"ModDB"` string should use `ModDBConstants.PublisherType` or define a parser ID constant
Per constants.md style guide, string literals should be defined in dedicated constants classes.
**Context Used:** Context from `dashboard` - Use dedicated constants classes instead of hardcoding constants string, integers or variables in ser... ([source](https://app.greptile.com/review/custom-context?memory=53453b3b-b708-4856-b1b0-0cbc8bfe5330))
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.1 participant
Overview
This PR introduces a architectural overhaul and feature expansion for GenHub's content management system. Implementing a Universal Content Pipeline and a Creator Publishing System, transforming how content is discovered, managed, and installed.
Features
1. Universal Content Pipeline
A standardized 3-tier architecture for consistent content handling:
IContentOrchestrator.2. Creator Publishing System
Empowers creators to take control of their content delivery:
3. Redesigned Downloads Browser
A premium browsing experience for discovery:
4. Advanced Web Discovery
🛠 Technical Details
IContentDiscoverer,IContentResolver,IContentDeliverer,IPublisherManifestFactory.📚 Documentation
New documentation available:
Greptile Overview
Greptile Summary
This PR delivers a major architectural transformation introducing a Universal Content Pipeline and Creator Publishing System for GenHub. The implementation standardizes content discovery, resolution, and delivery across multiple sources (GitHub, ModDB, CNC Labs, AOD Maps, Generals Online) through a 3-tier architecture with orchestration, provider-specific facades, and pluggable components.
Key Changes
IContentDiscoverer,IContentResolver,IContentDelivererinterfaces with source-specific implementations for 7+ providersModDBPageParserextracting files, videos, images, articles, reviews, and comments from ModDB pagesVersionSelectorsupporting Latest/Stable/Specific policies for content releasesIssues Found
Resource disposal issues flagged in previous review threads remain unaddressed:
_browserLockdisposal inPlaywrightService(singleton but incorrect pattern)ModDBDiscoverernever disposedHttpClientinContentDetailViewModelnever disposed_fileLockinPublisherSubscriptionStorenot disposedDebug markers
[TEMP]present inModDBResolver,ModDBManifestFactory,CNCLabsMapResolver.Minor: hardcoded
"ModDB"string inModDBPageParser.cs:610should use constant.No dedicated tests for new
PublisherCatalog,VersionSelector, orModDBPageParserclasses.Architecture Quality
The content pipeline architecture is well-designed with clear separation of concerns. The 3-tier pattern (Orchestration → Providers → Components) provides excellent extensibility. Documentation is comprehensive with flowcharts and detailed guides. The creator publishing system enables decentralized content distribution which aligns with the project's community-driven nature.
Confidence Score: 3/5
PlaywrightService.cs,ModDBDiscoverer.cs,ContentDetailViewModel.cs, andPublisherSubscriptionStore.csfor resource disposal issuesImportant Files Changed
_browserLock(already flagged)_fileLocksemaphore not disposed (already flagged)Sequence Diagram
sequenceDiagram participant User participant UI as DownloadsBrowserViewModel participant Orch as ContentOrchestrator participant Disc as IContentDiscoverer<br/>(ModDB/GitHub/etc) participant PW as PlaywrightService participant Res as IContentResolver participant Parser as ModDBPageParser participant MF as ManifestFactory participant Pool as ContentManifestPool participant DL as DownloadService User->>UI: Select Publisher + Apply Filters UI->>Disc: DiscoverAsync(ContentSearchQuery) alt ModDB with WAF Disc->>PW: FetchAndParseAsync(url) PW->>PW: Launch Chromium Browser PW-->>Disc: IDocument (AngleSharp) end Disc->>Parser: ParseAsync(url, html) Parser-->>Disc: ParsedWebPage (files, images, articles) Disc-->>UI: ContentDiscoveryResult (SearchResults) User->>UI: Click Download on Item UI->>Res: ResolveAsync(SearchResult) Res->>Parser: ParseAsync(detailUrl) Parser-->>Res: ParsedWebPage with file metadata Res->>MF: CreateManifestAsync(ParsedWebPage) MF-->>Res: ContentManifest Res-->>UI: OperationResult<ContentManifest> UI->>DL: DownloadFileAsync(url, progress) DL-->>UI: Download Progress Updates UI->>Pool: AddManifestAsync(manifest, sourceDir) Pool->>Pool: Store in CAS + Create Reference Pool-->>UI: Success UI-->>User: Download CompleteContext used (3)
dashboard- What: All compiler warnings and linter warnings across the entire codebase must be resolved before m... (source)dashboard- Use dedicated constants classes instead of hardcoding constants string, integers or variables in ser... (source)dashboard- Coding style used in the application which PullRequests and coding style has to be applied to. (source)