Skip to content

refactor: 문의 관련 첨부파일 public 으로 변경 #337

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
chominju02 wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

refactor: 문의 관련 첨부파일 public 으로 변경 #337

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 4 additions & 4 deletions src/main/java/life/mosu/mosuserver/application/inquiry/InquiryAnswerAttachmentService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,21 +64,21 @@ public void updateAttachment(

private InquiryDetailResponse.AttachmentResponse createAttachResponse(
InquiryAnswerAttachmentEntity attachment) {
String preSignedUrl = s3Service.getPreSignedUrl(attachment.getS3Key());
String publicUrl = s3Service.getPublicUrl(attachment.getS3Key());

return new InquiryDetailResponse.AttachmentResponse(
attachment.getFileName(),
preSignedUrl
publicUrl
);
Comment on lines +67 to 72
Copy link

@gemini-code-assist gemini-code-assist bot Aug 12, 2025

Choose a reason for hiding this comment

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

medium

S3Service에 파일의 공개 여부에 따라 적절한 URL(public 또는 pre-signed)을 반환하는 getUrl(File file) 메서드가 있습니다. getPublicUrl을 직접 호출하는 대신 s3Service.getUrl(attachment)를 사용하면 향후 가시성 정책이 변경되더라도 이 코드를 수정할 필요가 없어 유지보수성이 향상됩니다.

Suggested change
String publicUrl = s3Service.getPublicUrl(attachment.getS3Key());
return new InquiryDetailResponse.AttachmentResponse(
attachment.getFileName(),
preSignedUrl
publicUrl
);
String url = s3Service.getUrl(attachment);
return new InquiryDetailResponse.AttachmentResponse(
attachment.getFileName(),
url
);

Comment on lines +67 to 72
Copy link

@coderabbitai coderabbitai bot Aug 12, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Public URL switch: ensure legacy attachments won’t break

Same concern as for inquiry attachments. Existing inquiry-answer attachments stored as PRIVATE will fail if we now return public URLs. Plan for either (a) data migration to PUBLIC or (b) a temporary conditional URL strategy based on entity visibility.

To reduce risk of dead code and ensure consistency, please also verify whether createAttachResponse(...) is used anywhere:

I can provide a visibility-aware S3Service.getUrl(visibility, key) shim if you want a transitional solution.

Also applies to: 77-83

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Check for lingering pre-signed URL usage"
rg -n "getPreSignedUrl\\("

echo
echo "Is createAttachResponse referenced?"
rg -n "createAttachResponse\\("

Length of output: 986

Handle PRIVATE inquiry-answer attachments to prevent broken URLs

Verified that the private createAttachResponse method in InquiryAnswerAttachmentService is only called within that class (no dead-code risk). You still need to handle legacy PRIVATE attachments or migrate them to PUBLIC to avoid broken links.

• Files to update:
– src/main/java/life/mosu/mosuserver/application/inquiry/InquiryAnswerAttachmentService.java (around lines 67–72 & 77–83)
– src/main/java/life/mosu/mosuserver/infra/persistence/s3/S3Service.java

• Suggested changes:

  1. Add a visibility-aware helper in your S3Service:
public String getUrl(Visibility visibility, String s3Key) {
    return visibility == Visibility.PUBLIC
        ? getPublicUrl(s3Key)
        : getPreSignedUrl(s3Key);
}
  1. In InquiryAnswerAttachmentService#createAttachResponse, switch to the new helper (or inline conditional):
-   String publicUrl = s3Service.getPublicUrl(attachment.getS3Key());
+   String url = s3Service.getUrl(attachment.getVisibility(), attachment.getS3Key());
    return new InquiryDetailResponse.AttachmentResponse(
            attachment.getFileName(),
-           publicUrl
+           url
    );

Alternatively, if you prefer not to introduce a shim, inline the conditional:

String url = attachment.getVisibility() == Visibility.PUBLIC
    ? s3Service.getPublicUrl(key)
    : s3Service.getPreSignedUrl(key);

This ensures PRIVATE attachments continue working until you migrate them to PUBLIC.

🤖 Prompt for AI Agents 
In
src/main/java/life/mosu/mosuserver/application/inquiry/InquiryAnswerAttachmentService.java
around lines 67–72 and 77–83 and in
src/main/java/life/mosu/mosuserver/infra/persistence/s3/S3Service.java, handle
PRIVATE attachments so URLs aren’t broken: add a visibility-aware helper in
S3Service (getUrl or similar) that returns a public URL for PUBLIC visibility
and a presigned URL for PRIVATE, then update
InquiryAnswerAttachmentService#createAttachResponse to call that helper (or
inline a conditional) using attachment.getVisibility() to choose between
getPublicUrl and getPreSignedUrl so legacy PRIVATE items continue to work until
migrated.

}

private InquiryDetailResponse.AttachmentDetailResponse createAttachDetailResponse(
InquiryAnswerAttachmentEntity attachment) {
String preSignedUrl = s3Service.getPreSignedUrl(attachment.getS3Key());
String publicUrl = s3Service.getPublicUrl(attachment.getS3Key());

return new InquiryDetailResponse.AttachmentDetailResponse(
attachment.getFileName(),
preSignedUrl,
publicUrl,
attachment.getS3Key()
);
Comment on lines +77 to 83
Copy link

@gemini-code-assist gemini-code-assist bot Aug 12, 2025

Choose a reason for hiding this comment

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

medium

createAttachResponse 메서드와 마찬가지로, s3Service.getUrl(attachment)를 사용하면 코드의 유연성과 유지보수성을 높일 수 있습니다. 파일의 visibility 속성에 따라 URL 생성을 S3Service에 위임하는 것이 좋습니다.

Suggested change
String publicUrl = s3Service.getPublicUrl(attachment.getS3Key());
return new InquiryDetailResponse.AttachmentDetailResponse(
attachment.getFileName(),
preSignedUrl,
publicUrl,
attachment.getS3Key()
);
String url = s3Service.getUrl(attachment);
return new InquiryDetailResponse.AttachmentDetailResponse(
attachment.getFileName(),
url,
attachment.getS3Key()
);

}
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/life/mosu/mosuserver/application/inquiry/InquiryAttachmentService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,11 @@ public List<InquiryDetailResponse.AttachmentDetailResponse> toAttachmentResponse

private InquiryDetailResponse.AttachmentDetailResponse createAttachDetailResponse(
InquiryAttachmentJpaEntity attachment) {
String preSignedUrl = s3Service.getPreSignedUrl(attachment.getS3Key());
String publicUrl = s3Service.getPublicUrl(attachment.getS3Key());

return new InquiryDetailResponse.AttachmentDetailResponse(
attachment.getFileName(),
preSignedUrl,
publicUrl,
attachment.getS3Key()
);
Comment on lines +63 to 69
Copy link

@gemini-code-assist gemini-code-assist bot Aug 12, 2025

Choose a reason for hiding this comment

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

medium

S3Service에 파일의 공개 여부에 따라 적절한 URL(public 또는 pre-signed)을 반환하는 getUrl(File file) 메서드가 있습니다. getPublicUrl을 직접 호출하는 대신 s3Service.getUrl(attachment)를 사용하면 향후 가시성 정책이 변경되더라도 이 코드를 수정할 필요가 없어 유지보수성이 향상됩니다.

Suggested change
String publicUrl = s3Service.getPublicUrl(attachment.getS3Key());
return new InquiryDetailResponse.AttachmentDetailResponse(
attachment.getFileName(),
preSignedUrl,
publicUrl,
attachment.getS3Key()
);
String url = s3Service.getUrl(attachment);
return new InquiryDetailResponse.AttachmentDetailResponse(
attachment.getFileName(),
url,
attachment.getS3Key()
);

Comment on lines +63 to 69
Copy link

@coderabbitai coderabbitai bot Aug 12, 2025

Choose a reason for hiding this comment

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

💡 Verification agent

🧩 Analysis chain

Switch to public URL: confirm migration/backward-compat for existing private objects

Changing from pre-signed to public URLs works for newly created PUBLIC objects, but existing inquiry attachments previously stored as PRIVATE will 403 if not migrated. Please either:

  • run a data migration to update object ACL/metadata and DB visibility to PUBLIC for legacy inquiry attachments, or
  • temporarily serve URLs conditionally by the attachment’s stored visibility (PUBLIC → public URL, PRIVATE → pre-signed), until migration completes.

You can quickly check whether the attachment entities carry a visibility field we can branch on, and whether any lingering usages of getPreSignedUrl() remain:

If helpful, I can draft a one-off migration plan (DB update + S3 ACL/object metadata update via prefix) and a temporary dual-path resolver in S3Service.

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Find visibility on inquiry attachment entities"
rg -n -A 5 -B 2 $'class InquiryAttachmentJpaEntity|visibility|getVisibility\\('

echo
echo "Find any remaining usages of pre-signed URL API"
rg -n "getPreSignedUrl\\("

Length of output: 27648

Handle private inquiry attachments by branching on visibility or migrating
The InquiryAttachmentService currently always calls s3Service.getPublicUrl(...), which will 403 for any legacy attachments stored as PRIVATE. Since the domain model indeed persists a visibility field (e.g. InquiryAttachmentJpaEntity extends File with visibility and isPublic()), you should either:

  • Branch within InquiryAttachmentService
    Update src/main/java/life/mosu/mosuserver/application/inquiry/InquiryAttachmentService.java (around lines 63–69) to: 
    boolean isPublic = attachment.isPublic();
String url = isPublic
    ? s3Service.getPublicUrl(attachment.getS3Key())
    : s3Service.getPreSignedUrl(attachment.getS3Key());

return new InquiryDetailResponse.AttachmentDetailResponse(
    attachment.getFileName(),
    url,
    attachment.getS3Key()
);
  • Or perform a one-off migration
    1. Run a DB script to set visibility = 'PUBLIC' for all existing inquiry_attachment rows.
    2. Update S3 object ACLs (and metadata, if needed) under the inquiry/ prefix to be public.

Either approach prevents 403s for legacy private files.

🤖 Prompt for AI Agents 
In
src/main/java/life/mosu/mosuserver/application/inquiry/InquiryAttachmentService.java
around lines 63–69, the service always calls s3Service.getPublicUrl(...) which
will 403 for attachments persisted as PRIVATE; fix by branching on the
attachment visibility: call attachment.isPublic() (or check the visibility
field) and when true use s3Service.getPublicUrl(attachment.getS3Key()),
otherwise use s3Service.getPreSignedUrl(attachment.getS3Key()), then return the
AttachmentDetailResponse with the chosen URL and existing fileName and s3Key;
alternatively, if you prefer a migration approach, run a one‑time DB script to
set visibility='PUBLIC' for existing inquiry_attachment rows and update S3
object ACLs under the inquiry/ prefix so existing getPublicUrl calls continue to
work.

}
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/life/mosu/mosuserver/domain/file/Folder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ public enum Folder {
NOTICE("notice", PUBLIC),

TEMP("temp", PRIVATE),
INQUIRY("inquiry", PRIVATE),
INQUIRY_ANSWER("inquiryAnswer", PRIVATE),
INQUIRY("inquiry", PUBLIC),
INQUIRY_ANSWER("inquiryAnswer", PUBLIC),
ADMISSION_TICKET_IMAGE("admissionTicket/images", PRIVATE),
ADMISSION_TICKET_PDF("admissionTicket/pdfs", PRIVATE);

Expand Down
6 changes: 3 additions & 3 deletions src/main/java/life/mosu/mosuserver/presentation/common/FileRequest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@ public InquiryAttachmentJpaEntity toInquiryAttachmentEntity(Long inquiryId) {
return InquiryAttachmentJpaEntity.builder()
.fileName(fileName)
.s3Key(s3Key)
.visibility(Visibility.PRIVATE)
.visibility(Visibility.PUBLIC)
.inquiryId(inquiryId)
.build();
}

public NoticeAttachmentJpaEntity toNoticeAttachmentEntity(Long noticeId) {
return NoticeAttachmentJpaEntity.builder()
.fileName(fileName)
Expand All @@ -50,7 +50,7 @@ public InquiryAnswerAttachmentEntity toInquiryAnswerAttachmentEntity(Long inquir
return InquiryAnswerAttachmentEntity.builder()
.fileName(fileName)
.s3Key(s3Key)
.visibility(Visibility.PRIVATE)
.visibility(Visibility.PUBLIC)
.inquiryAnswerId(inquiryAnswerId)
.build();
}
Expand Down