HRINT-4835-2 - wrong images are displayed on certain posts#141
Conversation
d68ad9a to
4a7fc61
Compare
There was a problem hiding this comment.
Pull request overview
This PR fixes incorrect images being served for certain legacy posts by making image document IDs unique per image path (rather than colliding on filename-only IDs), so posts that reference identically-named thumbnails in different folders resolve to the correct attachment.
Changes:
- Update image lookup to load
PostImagedocuments by full relative path (images/<relative/path>) instead of filename-only. - Update the DEBUG-only migration to generate
PostImagedoc IDs from the image’s relative path underwwwroot/images.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
RaccoonBlog.Web/Controllers/ImagesController.cs |
Switches image doc-id construction to include the full image path to avoid filename collisions. |
RaccoonBlog.Web/Areas/Admin/Controllers/PostsController.cs |
Updates the debug migration to store images under path-based doc IDs (relative to wwwroot/images). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (string.IsNullOrEmpty(imagePath)) | ||
| return NotFound(); | ||
| var docId = "images/" + fileName; | ||
|
|
||
| var docId = "images/" + imagePath.ToLowerInvariant(); |
There was a problem hiding this comment.
This is PR2, so we need to remember PR1. In PR1, we added ImageUrlRewriter. The imagePath comes from . ImageUrlRewriter always uses clean forward slashes /. So bad path with \ cannot happen here.
@garayx or do you think we should do what Copilot suggests?
| [HttpGet("admin/posts/migrate-images")] | ||
| [AllowAnonymous] | ||
| [AllowAnonymous] | ||
| public IActionResult MigrateOldImages([FromServices] IWebHostEnvironment env) |
There was a problem hiding this comment.
This endpoint is #if DEBUG only and never compiled into Release builds.
| var batchFiles = allFiles.Skip(i).Take(batchSize).ToList(); | ||
| var openStreams = new List<FileStream>(); | ||
| var openStreams = new List<FileStream>(); | ||
|
|
||
| using (var fileSession = DocumentStore.OpenSession()) | ||
| { |
4a7fc61 to
c371264
Compare
Issue:
https://issues.hibernatingrhinos.com/issue/HRINT-4835/ayende.com-Old-posts-Images-issues
Verified the collision fix. Took posts that reference
image_thumb_1.pngfrom different folders — with the old code all would resolve to the same flat docimages/image_thumb_1.png, showing the wrong image. With the fix each post gets its own unique image via full-path doc ID.Opened each post and confirmed the image matches the content, then verified the image URL directly.
Code-Review-SharpArchitectur.MultiTenant_FF2F/image_thumb_1.pngThe-wages-of-sin-Inverse-leaky-abstracti_2259/image_thumb_1.pngThe-wages-of-sin-Over-architecture-in-th_124F2/image_thumb_1.pngThe-wages-of-sin-Re-creating-the_127AA/image_thumb_1.pngArchitecting-in-the-pit-of-doom_12C75/image_thumb_1.pngMore-on-the-joy-of-support_E77D/image_thumb_1.pngWindowsLiveWriter/Googlevs.Bing_1054F/image_thumb_1.pngWindows-Live-Writer/Google-vs.-Bing_E768/image_thumb_1.pngWindowsLiveWriter/DesigningadocumentdatabaseAggregation_84C9/image_thumb_1.pngWindowsLiveWriter/Googlevs.BingMaps_BFC/image_thumb_1.png