parser-core, comic-parser 패키지에 타입스크립트 적용 #86
Conversation
| @@ -224,6 +250,51 @@ class ComicParser extends Parser { | |||
| ); | |||
| return results; | |||
| } | |||
|
|
|||
| _secretKey: any; | |||
There was a problem hiding this comment.
스트림 파싱 방식 테스트 문서의 테스트 결과에 따라 퍼포먼스 문제를 해결하기 위해 CryptoProvider를 사용하지 않는 새로운 읽기 메소드를 추가 했어요.
CyptoProvider에 많은 기능이 포함되어있고 현재는 기존 기능 지원과 빠른 작업을 위해 수정대신 새로운 메소드를 추가하는 형태로 만들었는데요. 나머지 파서들도 작업이 끝나면 구조를 한번 정리해봐야 할 것 같아요.
|
코드 개선 후 다시 요청드릴게요 |
| /** | ||
| * Should execute encrypt or decrypt by condition if needed | ||
| */ | ||
| run(data: Buffer, filePath: string, purpose: string) { |
There was a problem hiding this comment.
이부분에 나와있진 않지만... BomCryptoProvider 구현을 보면 run의 반환값이 string|Buffer로 되어있는데 string타입이 base64 인코딩을 위한 것인가요?
There was a problem hiding this comment.
AesCryptor 인터페이스에 의한 타입 정의인데 기존 뷰어에서 WebCryptor로 옮겨가면서 실제 값은 Buffer만 있을거에요.
| | FileEntryObject<ZipFileInformation, IZipEntryPlus>; | ||
|
|
||
| /* eslint-disable no-param-reassign */ | ||
| export default async function readEntries( |
There was a problem hiding this comment.
CryptoProvider가 엔트리 파싱과 강하게 결합되어있는데요
압축에 대한 복호화는 CryptoProvider를 사용하더라도 페이지에 대한 복호화는 리소스 파싱 함수에서 같이 하는 것보다 context에서 암호화된 스트림을 받아서 복호화 파이프를 연결해서 반환하는 함수를 일괄로 추가해주는 것이 더 역할 분리가 될 것 같습니다.
There was a problem hiding this comment.
아래 세 가지를 고려하다 보니 readEntries와 강결합이 되어 있어요.
- zip이 암호화 되어있고 구성 엔트리는 암호화 되어있지 않은 경우
- zip은 암호화 되어있지 않고 구성 엔트리는 암호화 된 경우
- 둘 다 암호화 된 경우
파서쪽에서 다루는 context로 암호화된 스트림을 받아서 복호화와 파싱을 붙이기엔 EPUB과 같이 순서가 보장되지 않는 콘텐츠를 다루는 파서에서 복호화된 데이터를 메모리로 들고있도록 해야할텐데 어디까지 보고계신가요?
There was a problem hiding this comment.
파서에서 암호화된 데이터를 메모리로 들고 있고 사용할때 복호화가 필요하다면 복호화 파이프나 복호화를 걸어주면 된다고 생각해요.
파서테스트 결과보면 Buffer.concat을 사용하지 않는 경우 속도가 충분히 빠르게 나옵니다.
현재는 zip과 엔트리 동일한 cryptoProvider를 사용하는 것으로 보이는데
readEntries의 사용처 또는 readEntries에서 파싱결과 객체에 복호화 할 수 있는 메소드를 붙여주면 fromFile, fromZip, fromDirectory에서 cryptoProvider를 뺄 수 있을 것으로 보입니다.
| const result = { | ||
| ...zipCopy, | ||
| files: zip.files.map<IZipEntryPlus>((file) => { | ||
| const getFile: EntryBasicInformation["getFile"] = (options = {}) => { |
There was a problem hiding this comment.
fromZip의 getFile과 fromDirectory, fromFile의 getFile이 다르게 동작하는 것으로 보입니다.
fromDirectory, fromFile은 cryptoProvider가 입력되면 복호화된 결과를 반환하는데 fromZip은 암호화된 file을 반환하는걸로 보입니다.
fromDirectory, fromFile에서도 cryptoProvider를 제거하고 추후에 복호화를 붙일 수 있게 가공하는 것이 좋을 것 같습니다.
There was a problem hiding this comment.
아 zipUtil안에 복호화 코드가 있군요 🙏 �여기도 cryptoProvider를 빼는것이 좋을것같아요
작업내용
테스트 방법
yarn build이후 packages 폴더의 내용을 테스트 할 프로젝트의node_modules/@ridi/content-parser위치에 대체한 이후 프로젝트를 구동해서 동작에 이상이 없는지 확인합니다. books-desktop에서 코믹 뷰어에 대한 작동을 확인했습니다.이슈
수정 사항은 테스트는 모두 통과하지만 nyc에서 coverage summary가 Unknown%로 표기되는 문제가 있습니다. nyc가 관리되지 않은지 꽤 되어서 대안 패키지인 c8을 사용하는 것을 고려해볼 예정입니다.