[REFACTOR] Kingfisher 기능을 대체하기 위한 ImageLoader 개발 #97

dongglehada · 2025-04-01T11:28:46Z

📌 이슈

Kingfisher 제거 후 자체 모듈 개발의 건 #92

✅ 작업 사항

🚀 테스트 방식

메모리 및 디스크 에서 일정 시간 후 삭제되는 것은 확인 완료하였습니다.
기존 킹피셔를 사용해 이미지를 로드 하는 코드를 일부 변경 적용하여 대부분의 이미지를 로드할 때 해당 모듈이 사용되고 있습니다.

👀 ETC (추후 개발해야 할 것, 참고자료 등) ->

이미지 리사이징시 사이즈에 대한 기준을 찾아 보았으나 괜찮은 기준을 가진 자료를 찾아보지 못한 것 같습니다.. ㅠ 그래서 해당 리사이징 사이즈에 대한 기준을 어떻게 할지 의견을 주실 수 있을까요? 그와 동일한 이유로 스토리지 별 객체들의 만료 시간에 대한 의견도 부탁드립니다!

…tor/#92-Replacing-Kingfisher-with-a-custom-image-caching-and-resizing-module

0Hooni

수고하셨습니다!! 몇가지 질문이 좀 있어서 Approve는 다음 리뷰로 넘기겠습니다☺️

이미지 캐싱을 Kingfisher에 의존하지 않기 위한 매니저를 만드는것과, 이를 사용하고 읽기좋게 짜신 노력이 보여서 좋았습니다👍🏻

추가로 아마 캐시 저장에 제한을 걸 때 지금과 같이 주기적으로 처리해 주거나 데이터의 만료를 정해주는 방식 말고도 디바이스 용량의 n%로 거는 방식도 있는것으로 알고 있습니다! 이것도 같이 고민해보면 좋을것 같아요.

코멘트 외적으로 궁금한것이 있는데 같이 생각해봐주시면 감사하겠습니다☺️

이러한 캐시 매니저들은 어느 레이어에 있어야 되는걸까요? 이건 저도 항상 고민인것 같아서 준영님의 의견이 궁금합니다.

0Hooni · 2025-04-01T13:50:36Z

Poppool/Poppool/Infrastructure/ImageLoader/DiskStorage.swift

+    private func startCacheCleanup() {
+        DispatchQueue.global(qos: .background).async { [weak self] in
+            guard let self = self else { return }
+
+            let cleanTimer = Timer.scheduledTimer(withTimeInterval: 300, repeats: true) { _ in
+                let files = (try? self.fileManager.contentsOfDirectory(at: self.cacheDirectory, includingPropertiesForKeys: nil)) ?? []
+
+                for file in files {
+                    if file.pathExtension == "metadata",
+                       let metadataData = try? Data(contentsOf: file),
+                       let metadata = try? JSONSerialization.jsonObject(with: metadataData) as? [String: TimeInterval],
+                       let expirationTime = metadata["expiration"] {
+
+                        // 만료 시간이 지나면 이미지와 메타데이터 삭제
+                        if Date().timeIntervalSince1970 > expirationTime {
+                            let imageFileURL = file.deletingPathExtension() // 메타데이터와 동일한 이름의 이미지 파일
+                            do {
+                                try self.fileManager.removeItem(at: imageFileURL)
+                                try self.fileManager.removeItem(at: file) // 메타데이터 삭제
+                            } catch {
+                                print("Failed to delete expired cache: \(error)")
+                            }
+                        }
+                    }
+                }
+            }
+            // 백그라운드에서 실행되는 타이머를 메인 루프에 추가
+            RunLoop.current.add(cleanTimer, forMode: .common)
+            RunLoop.current.run() // 백그라운드 스레드에서 타이머를 계속 실행하기 위해 RunLoop를 유지
+        }
+    }


캐시 클리닝을 주기적으로 처리하는 이유가 있을까요?

디스크 스토리지에 저장하는 경우에는 일반적으로 반영구적인 데이터를 캐싱하기 위해 사용되는 공간으로 알고 있는데, 주기적으로 삭제가 되면 디스크 캐싱이 갖는 이점(메모리 대비 큰 용량)을 잘 못 활용할 수 있다 생각합니다.

프린트문 보다는 로거로 대체해주시면 더 좋을것 같습니다!
마찬가지로 영훈님 의견과 마찬가지로 좀더 부가적으로 의견을 덧대자면
주기적인 처리보단 특정 한계치를 설정하면 이후부터 삭제되는 방향으로 고려하셔도 좋을듯합니다!

타이머를 사용하여 주기적으로 확인할 필요가 없을 것 같기는 합니다만 특정 시점에 캐시를 정리하는 과정은 필요하다고 생각을 합니다 사용되지 않는 데이터를 가지고 있게되는 문제도 있을 것 같기도 하고 이미지가 수정되었음에도 동일한 URL을 가지고 있다면 수정사항을 반영할 수 없을 것 같아서 캐시를 정리하는 기능 자체는 필요할 것 같다고 생각합니다

0Hooni · 2025-04-01T13:55:53Z

Poppool/Poppool/Infrastructure/ImageLoader/DiskStorage.swift

+            do {
+                try metadataData.write(to: metadataURL)
+            } catch {
+                print("Error writing metadata: \(error)")


Print가 아마 추후 디버깅에 사용하시려고 남기신것 같은데 이 부분 Print대신 사용할 로그 방식에 대해 의논해서 로그로 대체하면 좋겠습니다!

로그 양식같은것도 고민해봐야겠네요

로그를 어떻게 남길까 고민하다가 일단 print문을 남겼는데 의논해 보면 좋을 것 같습니다 :>

Poppool/Poppool/Presentation/Extension/UIImageView+.swift

0Hooni · 2025-04-01T14:00:18Z

Poppool/Poppool/Infrastructure/ImageLoader/MemoryStorage.swift

+            let cleanTimer = Timer.scheduledTimer(withTimeInterval: 60, repeats: true) { _ in
+                for key in self.cachedKeys {
+                    let nsKey = key as NSString
+                    if let cachedData = self.cache.object(forKey: nsKey), cachedData.isExpired() {
+                        self.cache.removeObject(forKey: nsKey)
+                        self.cachedKeys.remove(key)
+                    }
+                }
+            }


디스크는 인터벌이 5분이고 메모리는 1분인데 혹시 기반이 되는 근거가 있었을까요?

인터벌 및 이미지 사이즈의 기준은 일단 임의로 설정해 둔 값입니다! 기능 구현의 목적을 둔 초안이라 고려하지 않고 작성하였어요 ㅠ

0Hooni · 2025-04-01T14:02:27Z

Poppool/Poppool/Infrastructure/ImageLoader/ImageLoader.swift

+    var memoryCacheExpiration: TimeInterval = 300
+    var diskCacheExpiration: TimeInterval = 86_400


시간은 자체적으로 한번 지정한 뒤 바뀌지 않을것으로 생각되는데 맞을까요?

맞다면 var → let으로 하시면 좋을것 같습니다!

모듈로 만들었을 때 AppDelegate에서
ImageLoader.shared.configure.memoryCacheExpiration = 360 이렇게 사용하려고 var로 선언하였습니다.! 영훈님 생각에는 let으로 선언하는게 좋을까요?

0Hooni · 2025-04-01T14:05:09Z

Poppool/Poppool/Infrastructure/ImageLoader/ImageLoader.swift

+    var size: CGSize {
+        switch self {
+        case .low:
+            return CGSize(width: 100, height: 100)
+        case .middle:
+            return CGSize(width: 200, height: 200)
+        case .high:
+            return CGSize(width: 400, height: 400)
+        case .origin:
+            return CGSize(width: 1000, height: 1000)
+        }
+    }


혹시 4단계로 분류된 이미지 사이즈의 기준이 뭐였었나요?

dongglehada · 2025-04-04T09:53:20Z

@0Hooni 저는 해당 모듈은 프로젝트와 분리하여 라이브러리로 만들어서 관리해 보는 것도 좋을 것 같습니다 이후 presentation layer에서 사용해보면 괜찮지 않을까 생각해요!

…tor/#92-Replacing-Kingfisher-with-a-custom-image-caching-and-resizing-module # Conflicts: # Poppool/Poppool.xcodeproj/project.pbxproj # Poppool/Poppool.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved # Poppool/Poppool/Presentation/Map/Common/MapPopupCardView/PopupCardCell.swift # Poppool/Poppool/Presentation/Scene/Home/Main/View/HomeCardSection/HomeCardSectionCell.swift # Poppool/Poppool/Presentation/Scene/Home/Main/View/HomePopularCardSection/HomePopularCardSectionCell.swift

dongglehada · 2025-04-07T15:06:10Z

@0Hooni , @zzangzzangguy 리뷰 한 번 부탁드립니다! 그리고 ETC 내용 참고 부탁드리겠습니다!

zzangzzangguy · 2025-04-02T08:28:34Z

Poppool/Poppool/Infrastructure/ImageLoader/DiskStorage.swift

+    private func startCacheCleanup() {
+        DispatchQueue.global(qos: .background).async { [weak self] in
+            guard let self = self else { return }
+
+            let cleanTimer = Timer.scheduledTimer(withTimeInterval: 300, repeats: true) { _ in
+                let files = (try? self.fileManager.contentsOfDirectory(at: self.cacheDirectory, includingPropertiesForKeys: nil)) ?? []
+
+                for file in files {
+                    if file.pathExtension == "metadata",
+                       let metadataData = try? Data(contentsOf: file),
+                       let metadata = try? JSONSerialization.jsonObject(with: metadataData) as? [String: TimeInterval],
+                       let expirationTime = metadata["expiration"] {
+
+                        // 만료 시간이 지나면 이미지와 메타데이터 삭제
+                        if Date().timeIntervalSince1970 > expirationTime {
+                            let imageFileURL = file.deletingPathExtension() // 메타데이터와 동일한 이름의 이미지 파일
+                            do {
+                                try self.fileManager.removeItem(at: imageFileURL)
+                                try self.fileManager.removeItem(at: file) // 메타데이터 삭제
+                            } catch {
+                                print("Failed to delete expired cache: \(error)")
+                            }
+                        }
+                    }
+                }
+            }
+            // 백그라운드에서 실행되는 타이머를 메인 루프에 추가
+            RunLoop.current.add(cleanTimer, forMode: .common)
+            RunLoop.current.run() // 백그라운드 스레드에서 타이머를 계속 실행하기 위해 RunLoop를 유지
+        }
+    }


프린트문 보다는 로거로 대체해주시면 더 좋을것 같습니다!
마찬가지로 영훈님 의견과 마찬가지로 좀더 부가적으로 의견을 덧대자면
주기적인 처리보단 특정 한계치를 설정하면 이후부터 삭제되는 방향으로 고려하셔도 좋을듯합니다!

0Hooni

LGTM!!

사실 킹피셔를 사용하는곳이 몇곳 없었네요 ☺️

dongglehada added 6 commits March 31, 2025 14:33

feat/#92: URL을 사용한 이미지 다운로드 기능 초기 구현

3e50213

Merge branch 'dev' of https://github.com/PopPool/iOS-renew into refac…

ae607ab

…tor/#92-Replacing-Kingfisher-with-a-custom-image-caching-and-resizing-module

feat/#92: NSCache 적용

c3ad17a

feat/#92: 메모리 캐시 스토리지 분리 및 자동 캐시 데이터 제거 로직 적용 및 문서화

42d4a95

feat/#92: DiskStorage 적용

395729f

feat/#92: 이미지 리사이징 적용

b529e24

dongglehada added the 🔄 refactor 프로덕션 코드 리팩토링, 파일 삭제, 네이밍 수정 및 폴더링 label Apr 1, 2025

dongglehada requested review from 0Hooni and zzangzzangguy April 1, 2025 11:28

dongglehada self-assigned this Apr 1, 2025

dongglehada linked an issue Apr 1, 2025 that may be closed by this pull request

Kingfisher 제거 후 자체 모듈 개발의 건 #92

Closed

0Hooni reviewed Apr 1, 2025

View reviewed changes

0Hooni marked this pull request as draft April 4, 2025 10:08

0Hooni changed the title ~~[Refactor] Kingfisher 기능을 대체하기 위한 ImageLoader 개발~~ Kingfisher 기능을 대체하기 위한 ImageLoader 개발 Apr 4, 2025

0Hooni changed the title ~~Kingfisher 기능을 대체하기 위한 ImageLoader 개발~~ [REFACTOR] Kingfisher 기능을 대체하기 위한 ImageLoader 개발 Apr 4, 2025

dongglehada added 2 commits April 6, 2025 18:03

refactor/#92: Kingfisher 제거 및 관련 코드 제거, 대치

6db82d3

fix/#92: DiskStorage TImer 삭제

d80a512

dongglehada marked this pull request as ready for review April 6, 2025 09:25

dongglehada marked this pull request as draft April 6, 2025 09:48

dongglehada and others added 4 commits April 7, 2025 23:27

refactor/#92: import Kingfisher 제거 및 빌드 가능하게 코드 일부 수정

72b9de6

refactor/#92: import Kingfisher 제거

8b2674e

style/#92: Apply SwiftLint autocorrect

ec1848b

dongglehada marked this pull request as ready for review April 7, 2025 15:05

0Hooni self-requested a review April 8, 2025 00:11

zzangzzangguy approved these changes Apr 8, 2025

View reviewed changes

0Hooni approved these changes Apr 8, 2025

View reviewed changes

dongglehada merged commit 20e3247 into dev Apr 8, 2025

dongglehada deleted the refactor/#92-Replacing-Kingfisher-with-a-custom-image-caching-and-resizing-module branch April 8, 2025 06:33

		var memoryCacheExpiration: TimeInterval = 300
		var diskCacheExpiration: TimeInterval = 86_400

[REFACTOR] Kingfisher 기능을 대체하기 위한 ImageLoader 개발 #97

[REFACTOR] Kingfisher 기능을 대체하기 위한 ImageLoader 개발 #97

Uh oh!

Conversation

dongglehada commented Apr 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📌 이슈

✅ 작업 사항

🚀 테스트 방식

👀 ETC (추후 개발해야 할 것, 참고자료 등) ->

Uh oh!

0Hooni left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dongglehada Apr 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dongglehada commented Apr 4, 2025

Uh oh!

dongglehada commented Apr 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0Hooni left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dongglehada commented Apr 1, 2025 •

edited

Loading

dongglehada Apr 4, 2025 •

edited

Loading