ispras / dedoc

Dedoc is a library (service) for automate documents parsing and bringing to a uniform format. It automatically extracts content, logical structure, tables, and meta information from textual electronic documents. (Parse document; Document content extraction; Logical structure extraction; PDF parser; Scanned document parser; DOCX parser; HTML parser
Apache License 2.0
152 stars 18 forks source link

TLDR-517 attachments_dir #370

Closed raxtemur closed 10 months ago

NastyBoget commented 10 months ago

Предлагаю еще сделать тесты после теста unit_tests.test_module_attachment_extractor.TestAttachmentsExtractor.test_attachments_dir

Еще просьба переименовать тест test_attachments_dir в test_manager_attachments_dir и вместо docx файла в нем использовать файл, котрый я в чатике пришлю (добавить его в data/mhtml). Нам нужно протестить ридеры, которые выделяют аттачи внутри себя (см. комментарий ниже), поэтому в тесте test_manager_attachments_dir предлагаю потестить на файлах:

NastyBoget commented 10 months ago

Еще заметка: без некоторых правок тест test_manager_attachments_dir у тебя не пройдет. Некоторые ридеры извлекают аттачи не через AttachmentsExtractor, а внутри себя:

В этих ридерах нужно получать attachments_dir отдельно

Еще я заметила интересную штуку на строке tests/unit_tests/test_module_attachment_extractor.py:108

document = ArchiveReader(config=config).read(path=file_path, parameters={"with_attachment": True})

Переименуй with_attachment в with_attachments плиз. Этот параметр ни на что не влияет (внезапно), потому что ArchiveReader всегда извлекает вложения. Хз норм это или нет, создам на это отдельную задачу на проверку

raxtemur commented 10 months ago

Блин, хренов mhtml reader is specific:

  1. It saves attached files not in the temp_dir, but in temp_dir/FILE_HASH. I'm kind of not sure if it is better to keep it as it is and change pipeline for test? Мы так починим TLDR-522?
  2. Файлы так лежат потому что могут быть имена одинаковые? А что если имена разные, а файлы одинаковые(=> хеши одинаковые => одинаковые папки в которых они лежат => там лежит не один файл?)
NastyBoget commented 10 months ago

Блин, хренов mhtml reader is specific:

1. It saves attached files not in the temp_dir, but in temp_dir/FILE_HASH. I'm kind of not sure if it is better to keep it as it is and change pipeline for test? Мы так починим [TLDR-522](https://jira.intra.ispras.ru/browse/TLDR-522)?

2. Файлы так лежат потому что могут быть имена одинаковые? А что если имена разные, а файлы одинаковые(=> хеши одинаковые => одинаковые папки в которых они лежат => там лежит не один файл?)

Да, это сделано из-за файлов с одинаковыми именами. И если во вложениях два одинаковых файла с разными именами - норм поведение сохранить оба файла. Ключевой момент - tmp_file_path у аттачей должен указывать на реально существующий файл в файловой системе, это прям надо проверять в тестах.

Насчет директорий MhtmlReader - реально поведение не очень, давай лучше сделаем так:

tmp_file_name = save_data_to_unique_file(directory=tmpdir, filename=filename, binary_data=payload)
attachments.append(AttachedFile(original_name=filename, tmp_file_path=os.path.join(tmpdir, tmp_file_name), uid=f"attach_{uuid.uuid1()}", need_content_analysis=need_content_analysis))

Это будет намного лучше, во всех attachments_extractors так сделано, и в EmailReader тоже. Еще, прошу проверить, везде ли так сделано (где извлекаются вложения).

raxtemur commented 10 months ago

Пока копал откопал в attachments_handler.py TODO на https://jira.intra.ispras.ru/browse/TLDR-185 Точно актуальная задача?

NastyBoget commented 10 months ago

Пока копал откопал в attachments_handler.py TODO на https://jira.intra.ispras.ru/browse/TLDR-185 Точно актуальная задача?

не факт, но ее точно тебе сейчас делать не стоит