OCA / storage

GNU Affero General Public License v3.0
70 stars 159 forks source link

[16.0] fs_attachment: fix meaningful name rewriting #312

Closed len-foss closed 9 months ago

len-foss commented 9 months ago

This fix the name rewriting in case the storage is "directory name optimized".

It adds a test to ensure that the name is consistently used between write and read.

Closes https://github.com/OCA/storage/pull/285

OCA-git-bot commented 9 months ago

Hi @lmignon, some modules you are maintaining are being modified, check this out!

len-foss commented 9 months ago

I'm very confused, as the newly added test breaks on the write, and only so if other tests from the same file (test_fs_attachment) are run. The way it breaks is also very strange:

FileNotFoundError: [Errno 2] No such file or directory: '/github/home/.local/share/Odoo/filestore/odoo/storage//tmp/tmpq6abj5qo/github/home/.local/share/Odoo/filestore/odoo/storage//te/st/test.txt'

Let's take a bit more time to understand where the issue is coming from, unless you already have some pointer?

lmignon commented 9 months ago

I'm very confused, as the newly added test breaks on the write, and only so if other tests from the same file (test_fs_attachment) are run. The way it breaks is also very strange:

FileNotFoundError: [Errno 2] No such file or directory: '/github/home/.local/share/Odoo/filestore/odoo/storage//tmp/tmpq6abj5qo/github/home/.local/share/Odoo/filestore/odoo/storage//te/st/test.txt'

Let's take a bit more time to understand where the issue is coming from, unless you already have some pointer?

The tests are failing due to the env cleanup at transaction rollback after each test. Indeed, the fs.storage backend inherit from the server.env.mixin. Therefore some fields are pseudo computed fields and need to be reset before running each test. To fix your issue, you need to update your created fs.storage record into the setup method....


def setUp(self)::
    super().setUp()
    self.backend_optimized = write(
            {
                "protocol": "file",
                "code": "tmp_opt",
                "directory_path": temp_dir,
                "optimizes_directory_path": True,
            }
        )
len-foss commented 9 months ago

Oh, thank you very much! I see, that also explains why I had to create the other backend, reusing the other made the tests break unexplainably in a similar fashion.

We should be good now then :-)

len-foss commented 9 months ago

All right. I have added this to the test and a bit more explanations to make it easier to check the assumptions.

lmignon commented 9 months ago

/ocabot merge patch

OCA-git-bot commented 9 months ago

What a great day to merge this nice PR. Let's do it! Prepared branch 16.0-ocabot-merge-pr-312-by-lmignon-bump-patch, awaiting test results.

OCA-git-bot commented 9 months ago

Congratulations, your PR was merged at ad8a4d3563b278ef7e254d7932245f0f63817be0. Thanks a lot for contributing to OCA. ❤️