-
Notifications
You must be signed in to change notification settings - Fork 1
Fix 서버 파일/메인이미지 폴더 분리 #374
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.
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
base: develop
Are you sure you want to change the base?
Conversation
| @Column(unique = true) | ||
| val filename: String, | ||
|
|
||
| val folder: String? = null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[마이너] folder -> directory 등으로 이름 변경하는 것도 괜찮을거 같습니다..! (folder는 주로 윈도우에서만 쓰는 느낌이라서..?)
| throw WrongMethodTypeException("해당하는 엔티티가 없습니다") | ||
| } | ||
| } | ||
| contentEntity.mainImage = mainImage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이거 더 이상 필요없어진거 같아 제거하면 좋을거 같습니다.
| @DeleteMapping("/{*filepath}") | ||
| fun deleteFile( | ||
| @PathVariable filepath: String, | ||
| request: HttpServletRequest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
request 추가한 이유가 있을까요? 보기에는 필요 없어 보여서요..!
| val mainImage: MainImageEntity? | ||
| var mainImage: MainImageEntity? | ||
|
|
||
| fun getMainImageFolder(): String |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[마이너] getter 보다는 다른 네이밍으로 수정되면 더 좋을 거 같습니다..! (실제 field는 아니니까요..?)
| interface AttachmentAttachable { | ||
| val attachments: List<AttachmentEntity> | ||
|
|
||
| fun getAttachmentFolder(): String |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
| private val endpointProperties: EndpointProperties, | ||
| private val eventPublisher: ApplicationEventPublisher | ||
| ) : AttachmentService { | ||
| override fun uploadAttachmentInLabEntity(labEntity: LabEntity, requestAttachment: MultipartFile): AttachmentDto { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
흠 얘도 추후에 추상화가 되면 좋겠군요..
| attachment.councilFile = contentEntity | ||
| } | ||
|
|
||
| else -> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 메서드도 main image처럼 추후 간단하게 할 수 있다면 좋겠네용...
| @Column(unique = true) | ||
| val filename: String, | ||
|
|
||
| val folder: String? = null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이렇게 수정되면 <folder, filename> -> unique로 수정되는게 맞겠네요..!
| var councilFile: CouncilFileEntity? = null | ||
| ) : BaseTimeEntity() | ||
| ) : BaseTimeEntity() { | ||
| fun filePath(): String { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이건 수정해주세요보다는 방향성을 고민해보면 좋을 거 같아서 코멘트 남깁니다.
현재 각 entity에서 구현한 attachable 인터페이스는 결국 패턴이 "{attachment,mainImage}/{domain}"의 형태로 고정되는거 같고,
각 도메인의 entity가 '어떤 경로에 저장되어야 한다'라는 정보까지 들고 있을 필요가 있을지는 고민되요.
위에 문제를 벗어나기 위해서 각 entity에 method를 정의한다기보다는
(fileType, attachable) -> String 의 식으로 resource/common 패키지 하위에 method를 추가하는 것도 한가지 방법일 거 같아요.
다만 어떻게 하든 장단점은 있어보이네요..!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
원래 attachable -> String 형식으로 메소드를 AttachmentService에 만들었는데, 저장 위치는 attachable이라면 가져야 할 속성이라 생각해 이렇게 수정했습니다.
다시 생각해보니 저장 위치를 하나의 함수에서 관리해줘야 한눈에 보기 편할 것 같기도 합니다.
수정해보겠습니다.
| private fun attachmentDirectoryOf(contentEntityType: AttachmentAttachable): String { | ||
| return "attachment/" + when (contentEntityType) { | ||
| is NewsEntity -> "news" | ||
| is NoticeEntity -> "notice" | ||
| is SeminarEntity -> "seminar" | ||
| is AboutEntity -> "about" | ||
| is AcademicsEntity -> "academics" | ||
| is CouncilFileEntity -> "council" | ||
| else -> "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
attachment와 mainImage를 한번에 처리하는 것보다 이 구조가 더 직관적이라 생각해 이렇게 만들었습니다
|
수정이랑 로컬에서 테스트 완료했습니다! |
서버에서 첨부파일/메인이미지가 저장되는 폴더 분리
한줄 요약
게시물의 타입별로 첨부파일이나 메인이미지가 저장되는 폴더를 분리
상세 설명
attachmentEntity와 mainImageEntity에 nullable folder 필드를 추가해서 기존 파일도 일단은 별도 마이그레이션 없이 그대로 사용할 수 있도록 했습니다.
app/files/attachment(또는 mainImage)/"폴더이름"/"파일이름" 위치에 저장됩니다
attachment의 하위 폴더 : news, notice, seminar, about, academics, council, lab
mainIamge의 하위 폴더 : news, seminar, about, professor, staff, research, recruit, council
폴더 이름은 각 엔티티에서 함수로 반환하도록 했습니다AttachmentService/MainImageService내부 함수에서 Entity 종류에 따라 디렉토리 이름 지정하는 방식으로 변경했습니다
TODO
로컬에서 실행 시 의도하지 않은 빈 폴더(app/attachment와 app/mainImage)가 생성되는 문제를 아직 해결하지 못했습니다