Merge lp:~jderose/dmedia/filestore-tmp-api into lp:dmedia
- filestore-tmp-api
- Merge into trunk
Proposed by
Jason Gerard DeRose
Status: | Merged |
---|---|
Approved by: | Jason Gerard DeRose |
Approved revision: | 170 |
Merged at revision: | 156 |
Proposed branch: | lp:~jderose/dmedia/filestore-tmp-api |
Merge into: | lp:dmedia |
Diff against target: |
582 lines (+250/-66) 4 files modified
dmedia/downloader.py (+2/-2) dmedia/filestore.py (+119/-42) dmedia/tests/test_downloader.py (+1/-1) dmedia/tests/test_filestore.py (+128/-21) |
To merge this branch: | bzr merge lp:~jderose/dmedia/filestore-tmp-api |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
James Raymond | Approve | ||
dmedia Dev | Pending | ||
Review via email: mp+49495@code.launchpad.net |
Commit message
Description of the change
Splits FileStore.
FileStore.
FileStore.
To fit these naming conventions, I also renamed:
FileStore.reltemp() => FileStore.reltmp()
FileStore.temp() => FileStore.tmp()
Meaning to do that for a while, so good excuse.
Probably easier to read the docstrings here - http://
To post a comment you must log in.
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'dmedia/downloader.py' | |||
2 | --- dmedia/downloader.py 2011-02-11 16:55:18 +0000 | |||
3 | +++ dmedia/downloader.py 2011-02-12 11:14:54 +0000 | |||
4 | @@ -172,12 +172,12 @@ | |||
5 | 172 | self.ext = ext | 172 | self.ext = ext |
6 | 173 | 173 | ||
7 | 174 | def get_tmp(self): | 174 | def get_tmp(self): |
9 | 175 | tmp = self.fs.temp(self.chash, self.ext, create=True) | 175 | tmp = self.fs.tmp(self.chash, self.ext, create=True) |
10 | 176 | log.debug('Writting file to %r', tmp) | 176 | log.debug('Writting file to %r', tmp) |
11 | 177 | return tmp | 177 | return tmp |
12 | 178 | 178 | ||
13 | 179 | def finalize(self): | 179 | def finalize(self): |
15 | 180 | dst = self.fs.finalize_transfer(self.chash, self.ext) | 180 | dst = self.fs.tmp_verify_move(self.chash, self.ext) |
16 | 181 | log.debug('Canonical name is %r', dst) | 181 | log.debug('Canonical name is %r', dst) |
17 | 182 | return dst | 182 | return dst |
18 | 183 | 183 | ||
19 | 184 | 184 | ||
20 | === modified file 'dmedia/filestore.py' | |||
21 | --- dmedia/filestore.py 2011-02-11 17:22:08 +0000 | |||
22 | +++ dmedia/filestore.py 2011-02-12 11:14:54 +0000 | |||
23 | @@ -39,6 +39,7 @@ | |||
24 | 39 | 39 | ||
25 | 40 | import os | 40 | import os |
26 | 41 | from os import path | 41 | from os import path |
27 | 42 | import stat | ||
28 | 42 | import tempfile | 43 | import tempfile |
29 | 43 | from hashlib import sha1 as HASH | 44 | from hashlib import sha1 as HASH |
30 | 44 | from base64 import b32encode, b32decode | 45 | from base64 import b32encode, b32decode |
31 | @@ -278,8 +279,6 @@ | |||
32 | 278 | if not chunk: | 279 | if not chunk: |
33 | 279 | break | 280 | break |
34 | 280 | self.thread.join() | 281 | self.thread.join() |
35 | 281 | if self.dst_fp is not None: | ||
36 | 282 | os.fchmod(self.dst_fp.fileno(), 0o444) | ||
37 | 283 | return b32encode(self.h.digest()) | 282 | return b32encode(self.h.digest()) |
38 | 284 | 283 | ||
39 | 285 | 284 | ||
40 | @@ -450,18 +449,18 @@ | |||
41 | 450 | return (dname, fname) | 449 | return (dname, fname) |
42 | 451 | 450 | ||
43 | 452 | @staticmethod | 451 | @staticmethod |
45 | 453 | def reltemp(chash, ext=None): | 452 | def reltmp(chash, ext=None): |
46 | 454 | """ | 453 | """ |
47 | 455 | Relative path of temporary file with *chash*, ending with *ext*. | 454 | Relative path of temporary file with *chash*, ending with *ext*. |
48 | 456 | 455 | ||
49 | 457 | For example: | 456 | For example: |
50 | 458 | 457 | ||
52 | 459 | >>> FileStore.reltemp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW') | 458 | >>> FileStore.reltmp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW') |
53 | 460 | ('transfers', 'NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW') | 459 | ('transfers', 'NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW') |
54 | 461 | 460 | ||
55 | 462 | Or with the file extension *ext*: | 461 | Or with the file extension *ext*: |
56 | 463 | 462 | ||
58 | 464 | >>> FileStore.reltemp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW', ext='mov') | 463 | >>> FileStore.reltmp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW', ext='mov') |
59 | 465 | ('transfers', 'NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW.mov') | 464 | ('transfers', 'NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW.mov') |
60 | 466 | 465 | ||
61 | 467 | Also see `FileStore.relpath()`. | 466 | Also see `FileStore.relpath()`. |
62 | @@ -472,6 +471,9 @@ | |||
63 | 472 | return (TRANSFERS_DIR, chash) | 471 | return (TRANSFERS_DIR, chash) |
64 | 473 | 472 | ||
65 | 474 | def check_path(self, pathname): | 473 | def check_path(self, pathname): |
66 | 474 | """ | ||
67 | 475 | Verify that *pathname* in inside this filestore base directory. | ||
68 | 476 | """ | ||
69 | 475 | abspath = path.abspath(pathname) | 477 | abspath = path.abspath(pathname) |
70 | 476 | if abspath.startswith(self.base + os.sep): | 478 | if abspath.startswith(self.base + os.sep): |
71 | 477 | return abspath | 479 | return abspath |
72 | @@ -574,7 +576,7 @@ | |||
73 | 574 | self.create_parent(filename) | 576 | self.create_parent(filename) |
74 | 575 | return filename | 577 | return filename |
75 | 576 | 578 | ||
77 | 577 | def temp(self, chash, ext=None, create=False): | 579 | def tmp(self, chash, ext=None, create=False): |
78 | 578 | """ | 580 | """ |
79 | 579 | Returns path of temporary file with *chash*, ending with *ext*. | 581 | Returns path of temporary file with *chash*, ending with *ext*. |
80 | 580 | 582 | ||
81 | @@ -582,26 +584,26 @@ | |||
82 | 582 | in which case the content-hash is already known. For example: | 584 | in which case the content-hash is already known. For example: |
83 | 583 | 585 | ||
84 | 584 | >>> fs = FileStore() | 586 | >>> fs = FileStore() |
86 | 585 | >>> fs.temp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW') #doctest: +ELLIPSIS | 587 | >>> fs.tmp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW') #doctest: +ELLIPSIS |
87 | 586 | '/tmp/store.../transfers/NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW' | 588 | '/tmp/store.../transfers/NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW' |
88 | 587 | 589 | ||
89 | 588 | 590 | ||
90 | 589 | Or with a file extension: | 591 | Or with a file extension: |
91 | 590 | 592 | ||
93 | 591 | >>> fs.temp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW', 'txt') #doctest: +ELLIPSIS | 593 | >>> fs.tmp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW', 'txt') #doctest: +ELLIPSIS |
94 | 592 | '/tmp/store.../transfers/NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW.txt' | 594 | '/tmp/store.../transfers/NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW.txt' |
95 | 593 | 595 | ||
96 | 594 | 596 | ||
97 | 595 | If called with ``create=True``, the parent directory is created with | 597 | If called with ``create=True``, the parent directory is created with |
98 | 596 | `FileStore.create_parent()`. | 598 | `FileStore.create_parent()`. |
99 | 597 | """ | 599 | """ |
101 | 598 | filename = self.join(*self.reltemp(chash, ext)) | 600 | filename = self.join(*self.reltmp(chash, ext)) |
102 | 599 | if create: | 601 | if create: |
103 | 600 | self.create_parent(filename) | 602 | self.create_parent(filename) |
104 | 601 | return filename | 603 | return filename |
105 | 602 | 604 | ||
106 | 603 | def allocate_for_transfer(self, size, chash, ext=None): | 605 | def allocate_for_transfer(self, size, chash, ext=None): |
108 | 604 | filename = self.temp(chash, ext, create=True) | 606 | filename = self.tmp(chash, ext, create=True) |
109 | 605 | fallocate(size, filename) | 607 | fallocate(size, filename) |
110 | 606 | try: | 608 | try: |
111 | 607 | fp = open(filename, 'r+b') | 609 | fp = open(filename, 'r+b') |
112 | @@ -620,14 +622,99 @@ | |||
113 | 620 | fallocate(size, filename) | 622 | fallocate(size, filename) |
114 | 621 | return open(filename, 'r+b') | 623 | return open(filename, 'r+b') |
115 | 622 | 624 | ||
119 | 623 | def finalize_transfer(self, chash, ext=None): | 625 | def tmp_move(self, tmp_fp, chash, ext=None): |
120 | 624 | """ | 626 | """ |
121 | 625 | Move canonically named temporary file to its final canonical location. | 627 | Move temporary file into its canonical location. |
122 | 628 | |||
123 | 629 | This method will securely and atomically move a temporary file into its | ||
124 | 630 | canonical location. | ||
125 | 631 | |||
126 | 632 | For example: | ||
127 | 633 | |||
128 | 634 | >>> fs = FileStore() | ||
129 | 635 | >>> tmp_fp = open(fs.join('foo.mov'), 'wb') | ||
130 | 636 | >>> chash = 'ZR765XWSF6S7JQHLUI4GCG5BHGPE252O' | ||
131 | 637 | >>> fs.tmp_move(tmp_fp, chash, 'mov') #doctest: +ELLIPSIS | ||
132 | 638 | '/tmp/store.../ZR/765XWSF6S7JQHLUI4GCG5BHGPE252O.mov' | ||
133 | 639 | |||
134 | 640 | |||
135 | 641 | Note, however, that this method does *not* verify the content hash of | ||
136 | 642 | the temporary file! This is by design as many operations will compute | ||
137 | 643 | the content hash as they write to the temporary file. Other operations | ||
138 | 644 | should use `FileStore.tmp_verify_move()` to verify and move in one step. | ||
139 | 645 | |||
140 | 646 | Regardless, the full content hash should have been verified prior to | ||
141 | 647 | calling this method. To ensure the content is not modified, operations | ||
142 | 648 | must take these steps: | ||
143 | 649 | |||
144 | 650 | 1. Open *tmp_fp* and keep it open, thereby retaining a lock on the | ||
145 | 651 | file | ||
146 | 652 | |||
147 | 653 | 2. Compute the full content hash, which can be done as content is | ||
148 | 654 | written to *tmp_fp* (open in mode ``'r+b'`` to resume a transfer, | ||
149 | 655 | but hash of previously transfered leaves must still be verified) | ||
150 | 656 | |||
151 | 657 | 3. With *tmp_fp* still open, move the temporary file into its | ||
152 | 658 | canonical location using this method. | ||
153 | 659 | |||
154 | 660 | As a simple locking mechanism, this method takes an open ``file`` rather | ||
155 | 661 | than a filename, thereby preventing the file from being modified during | ||
156 | 662 | the move. A ``ValueError`` is raised if *tmp_fp* is already closed. | ||
157 | 663 | |||
158 | 664 | For portability reasons, this method requires that *tmp_fp* be opened in | ||
159 | 665 | a binary mode: ``'rb'``, ``'wb'``, or ``'r+b'``. A ``ValueError`` is | ||
160 | 666 | raised if opened in any other mode. | ||
161 | 667 | |||
162 | 668 | For security reasons, this method will only move a temporary file | ||
163 | 669 | located within the ``FileStore.base`` directory or a subdirectory | ||
164 | 670 | thereof. If an attempt is made to move a file from outside the store, | ||
165 | 671 | `FileStoreTraversal` is raised. See `FileStore.check_path()`. | ||
166 | 672 | |||
167 | 673 | Just prior to moving the file, a call to ``os.fchmod()`` is made to set | ||
168 | 674 | read-only permissions (0444). After the move, *tmp_fp* is closed. | ||
169 | 675 | |||
170 | 676 | If the canonical file already exists, `DuplicateFile` is raised. | ||
171 | 677 | |||
172 | 678 | The return value is the absolute path of the canonical file. | ||
173 | 679 | |||
174 | 680 | :param tmp_fp: a ``file`` instance created with ``open()`` | ||
175 | 681 | :param chash: base32-encoded content-hash | ||
176 | 682 | :param ext: normalized lowercase file extension, eg ``'mov'`` | ||
177 | 683 | """ | ||
178 | 684 | # Validate tmp_fp: | ||
179 | 685 | if not isinstance(tmp_fp, file): | ||
180 | 686 | raise TypeError( | ||
181 | 687 | TYPE_ERROR % ('tmp_fp', file, type(tmp_fp), tmp_fp) | ||
182 | 688 | ) | ||
183 | 689 | if tmp_fp.mode not in ('rb', 'wb', 'r+b'): | ||
184 | 690 | raise ValueError( | ||
185 | 691 | "tmp_fp: mode must be 'rb', 'wb', or 'r+b'; got %r" % tmp_fp.mode | ||
186 | 692 | ) | ||
187 | 693 | if tmp_fp.closed: | ||
188 | 694 | raise ValueError('tmp_fp is closed, must be open: %r' % tmp_fp.name) | ||
189 | 695 | self.check_path(tmp_fp.name) | ||
190 | 696 | |||
191 | 697 | # Get canonical name, check for duplicate: | ||
192 | 698 | dst = self.path(chash, ext, create=True) | ||
193 | 699 | if path.exists(dst): | ||
194 | 700 | raise DuplicateFile(chash=chash, src=tmp_fp.name, dst=dst) | ||
195 | 701 | |||
196 | 702 | # Set file to read-only (0444) and move into canonical location | ||
197 | 703 | os.fchmod(tmp_fp.fileno(), stat.S_IRUSR | stat.S_IRGRP | stat.S_IROTH) | ||
198 | 704 | os.rename(tmp_fp.name, dst) | ||
199 | 705 | tmp_fp.close() | ||
200 | 706 | |||
201 | 707 | # Return canonical filename: | ||
202 | 708 | return dst | ||
203 | 709 | |||
204 | 710 | def tmp_verify_move(self, chash, ext=None): | ||
205 | 711 | """ | ||
206 | 712 | Verify temporary file, then move into its canonical location. | ||
207 | 626 | 713 | ||
208 | 627 | This method will check the content hash of the canonically-named | 714 | This method will check the content hash of the canonically-named |
209 | 628 | temporary file with content hash *chash* and extension *ext*. If the | 715 | temporary file with content hash *chash* and extension *ext*. If the |
212 | 629 | content hash is correct, it will do an ``os.fchmod()`` to set read-only | 716 | content hash is correct, this method will then move the temporary file |
213 | 630 | permissions, and then rename the file into its canonical location. | 717 | into its canonical location using `FileStore.tmp_move()`. |
214 | 631 | 718 | ||
215 | 632 | If the content hash is incorrect, `IntegrityError` is raised. If the | 719 | If the content hash is incorrect, `IntegrityError` is raised. If the |
216 | 633 | canonical file already exists, `DuplicateFile` is raised. Lastly, if | 720 | canonical file already exists, `DuplicateFile` is raised. Lastly, if |
217 | @@ -639,7 +726,7 @@ | |||
218 | 639 | temporary file name, like this: | 726 | temporary file name, like this: |
219 | 640 | 727 | ||
220 | 641 | >>> fs = FileStore() | 728 | >>> fs = FileStore() |
222 | 642 | >>> tmp = fs.temp('ZR765XWSF6S7JQHLUI4GCG5BHGPE252O', 'mov', create=True) | 729 | >>> tmp = fs.tmp('ZR765XWSF6S7JQHLUI4GCG5BHGPE252O', 'mov', create=True) |
223 | 643 | >>> tmp #doctest: +ELLIPSIS | 730 | >>> tmp #doctest: +ELLIPSIS |
224 | 644 | '/tmp/store.../transfers/ZR765XWSF6S7JQHLUI4GCG5BHGPE252O.mov' | 731 | '/tmp/store.../transfers/ZR765XWSF6S7JQHLUI4GCG5BHGPE252O.mov' |
225 | 645 | 732 | ||
226 | @@ -662,33 +749,23 @@ | |||
227 | 662 | Finally, the downloader will move the temporary file into its canonical | 749 | Finally, the downloader will move the temporary file into its canonical |
228 | 663 | location: | 750 | location: |
229 | 664 | 751 | ||
231 | 665 | >>> dst = fs.finalize_transfer('ZR765XWSF6S7JQHLUI4GCG5BHGPE252O', 'mov') | 752 | >>> dst = fs.tmp_verify_move('ZR765XWSF6S7JQHLUI4GCG5BHGPE252O', 'mov') |
232 | 666 | >>> dst #doctest: +ELLIPSIS | 753 | >>> dst #doctest: +ELLIPSIS |
233 | 667 | '/tmp/store.../ZR/765XWSF6S7JQHLUI4GCG5BHGPE252O.mov' | 754 | '/tmp/store.../ZR/765XWSF6S7JQHLUI4GCG5BHGPE252O.mov' |
234 | 668 | 755 | ||
235 | 669 | 756 | ||
238 | 670 | Note above that this method returns the full path of the canonically | 757 | The return value is the absolute path of the canonical file. |
239 | 671 | named file. | 758 | |
240 | 759 | :param chash: base32-encoded content-hash | ||
241 | 760 | :param ext: normalized lowercase file extension, eg ``'mov'`` | ||
242 | 672 | """ | 761 | """ |
245 | 673 | # Open temporary file and check content hash: | 762 | tmp = self.tmp(chash, ext) |
244 | 674 | tmp = self.temp(chash, ext) | ||
246 | 675 | tmp_fp = open(tmp, 'rb') | 763 | tmp_fp = open(tmp, 'rb') |
247 | 676 | h = HashList(tmp_fp) | 764 | h = HashList(tmp_fp) |
248 | 677 | got = h.run() | 765 | got = h.run() |
249 | 678 | if got != chash: | 766 | if got != chash: |
250 | 679 | raise IntegrityError(got=got, expected=chash, filename=tmp_fp.name) | 767 | raise IntegrityError(got=got, expected=chash, filename=tmp_fp.name) |
263 | 680 | 768 | return self.tmp_move(tmp_fp, chash, ext) | |
252 | 681 | # Get canonical name, check for duplicate: | ||
253 | 682 | dst = self.path(chash, ext, create=True) | ||
254 | 683 | if path.exists(dst): | ||
255 | 684 | raise DuplicateFile(chash=chash, src=tmp_fp.name, dst=dst) | ||
256 | 685 | |||
257 | 686 | # Set file to read-only and rename into canonical location | ||
258 | 687 | os.fchmod(tmp_fp.fileno(), 0o444) | ||
259 | 688 | os.rename(tmp_fp.name, dst) | ||
260 | 689 | |||
261 | 690 | # Return canonical filename: | ||
262 | 691 | return dst | ||
264 | 692 | 769 | ||
265 | 693 | def import_file(self, src_fp, ext=None): | 770 | def import_file(self, src_fp, ext=None): |
266 | 694 | """ | 771 | """ |
267 | @@ -696,8 +773,8 @@ | |||
268 | 696 | 773 | ||
269 | 697 | The method will compute the content-hash of *src_fp* as it copies it to | 774 | The method will compute the content-hash of *src_fp* as it copies it to |
270 | 698 | a temporary file within this store. Once the copying is complete, the | 775 | a temporary file within this store. Once the copying is complete, the |
273 | 699 | file will be renamed to its canonical location in the store, thus | 776 | temporary file will be moved to its canonical location using |
274 | 700 | ensuring an atomic operation. | 777 | `FileStore.tmp_move()`. |
275 | 701 | 778 | ||
276 | 702 | A `DuplicatedFile` exception will be raised if the file already exists | 779 | A `DuplicatedFile` exception will be raised if the file already exists |
277 | 703 | in this store. | 780 | in this store. |
278 | @@ -708,15 +785,15 @@ | |||
279 | 708 | 785 | ||
280 | 709 | Note that *src_fp* must have been opened in ``'rb'`` mode. | 786 | Note that *src_fp* must have been opened in ``'rb'`` mode. |
281 | 710 | 787 | ||
284 | 711 | :param src_fp: A ``file`` instance created with ``open()`` | 788 | :param src_fp: a ``file`` instance created with ``open()`` |
285 | 712 | :param ext: The file's extension, e.g., ``'ogv'`` | 789 | :param ext: normalized lowercase file extension, eg ``'mov'`` |
286 | 713 | """ | 790 | """ |
287 | 714 | size = os.fstat(src_fp.fileno()).st_size | 791 | size = os.fstat(src_fp.fileno()).st_size |
289 | 715 | tmp_fp = self.allocate_for_import(size, ext=ext) | 792 | tmp_fp = self.allocate_for_import(size, ext) |
290 | 716 | h = HashList(src_fp, tmp_fp) | 793 | h = HashList(src_fp, tmp_fp) |
291 | 717 | chash = h.run() | 794 | chash = h.run() |
296 | 718 | dst = self.path(chash, ext, create=True) | 795 | try: |
297 | 719 | if path.exists(dst): | 796 | self.tmp_move(tmp_fp, chash, ext) |
298 | 720 | raise DuplicateFile(chash=chash, src=src_fp.name, dst=dst) | 797 | except DuplicateFile as e: |
299 | 721 | os.rename(tmp_fp.name, dst) | 798 | raise DuplicateFile(src=src_fp.name, dst=e.dst, chash=e.chash) |
300 | 722 | return (chash, h.leaves) | 799 | return (chash, h.leaves) |
301 | 723 | 800 | ||
302 | === modified file 'dmedia/tests/test_downloader.py' | |||
303 | --- dmedia/tests/test_downloader.py 2011-02-11 15:45:43 +0000 | |||
304 | +++ dmedia/tests/test_downloader.py 2011-02-12 11:14:54 +0000 | |||
305 | @@ -212,7 +212,7 @@ | |||
306 | 212 | self.assertFalse(path.exists(dst)) | 212 | self.assertFalse(path.exists(dst)) |
307 | 213 | 213 | ||
308 | 214 | # Test when transfers/ exists but file does not: | 214 | # Test when transfers/ exists but file does not: |
310 | 215 | self.assertEqual(fs.temp(mov_hash, 'mov', create=True), src) | 215 | self.assertEqual(fs.tmp(mov_hash, 'mov', create=True), src) |
311 | 216 | self.assertTrue(path.isdir(src_d)) | 216 | self.assertTrue(path.isdir(src_d)) |
312 | 217 | e = raises(IOError, inst.finalize) | 217 | e = raises(IOError, inst.finalize) |
313 | 218 | self.assertFalse(path.exists(src)) | 218 | self.assertFalse(path.exists(src)) |
314 | 219 | 219 | ||
315 | === modified file 'dmedia/tests/test_filestore.py' | |||
316 | --- dmedia/tests/test_filestore.py 2011-02-11 01:50:20 +0000 | |||
317 | +++ dmedia/tests/test_filestore.py 2011-02-12 11:14:54 +0000 | |||
318 | @@ -26,6 +26,7 @@ | |||
319 | 26 | 26 | ||
320 | 27 | import os | 27 | import os |
321 | 28 | from os import path | 28 | from os import path |
322 | 29 | import stat | ||
323 | 29 | from hashlib import sha1 | 30 | from hashlib import sha1 |
324 | 30 | from base64 import b32encode, b32decode | 31 | from base64 import b32encode, b32decode |
325 | 31 | import shutil | 32 | import shutil |
326 | @@ -530,24 +531,24 @@ | |||
327 | 530 | 'ext %r does not match pattern %r' % (bad, EXT_PAT) | 531 | 'ext %r does not match pattern %r' % (bad, EXT_PAT) |
328 | 531 | ) | 532 | ) |
329 | 532 | 533 | ||
331 | 533 | def test_reltemp(self): | 534 | def test_reltmp(self): |
332 | 534 | self.assertEqual( | 535 | self.assertEqual( |
334 | 535 | self.klass.reltemp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW'), | 536 | self.klass.reltmp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW'), |
335 | 536 | ('transfers', 'NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW') | 537 | ('transfers', 'NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW') |
336 | 537 | ) | 538 | ) |
337 | 538 | self.assertEqual( | 539 | self.assertEqual( |
339 | 539 | self.klass.reltemp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW', ext='ogv'), | 540 | self.klass.reltmp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW', ext='ogv'), |
340 | 540 | ('transfers', 'NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW.ogv') | 541 | ('transfers', 'NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW.ogv') |
341 | 541 | ) | 542 | ) |
342 | 542 | 543 | ||
343 | 543 | # Test to make sure hashes are getting checked with safe_b32(): | 544 | # Test to make sure hashes are getting checked with safe_b32(): |
344 | 544 | bad = 'NWBNVXVK5..GIOW7MYR4K3KA5K22W7NW' | 545 | bad = 'NWBNVXVK5..GIOW7MYR4K3KA5K22W7NW' |
346 | 545 | e = raises(ValueError, self.klass.reltemp, bad) | 546 | e = raises(ValueError, self.klass.reltmp, bad) |
347 | 546 | self.assertEqual( | 547 | self.assertEqual( |
348 | 547 | str(e), | 548 | str(e), |
349 | 548 | 'b32: cannot b32decode %r: Non-base32 digit found' % bad | 549 | 'b32: cannot b32decode %r: Non-base32 digit found' % bad |
350 | 549 | ) | 550 | ) |
352 | 550 | e = raises(ValueError, self.klass.reltemp, bad, ext='ogv') | 551 | e = raises(ValueError, self.klass.reltmp, bad, ext='ogv') |
353 | 551 | self.assertEqual( | 552 | self.assertEqual( |
354 | 552 | str(e), | 553 | str(e), |
355 | 553 | 'b32: cannot b32decode %r: Non-base32 digit found' % bad | 554 | 'b32: cannot b32decode %r: Non-base32 digit found' % bad |
356 | @@ -556,7 +557,7 @@ | |||
357 | 556 | # Test to make sure ext is getting checked with safe_ext(): | 557 | # Test to make sure ext is getting checked with safe_ext(): |
358 | 557 | chash = 'NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW' | 558 | chash = 'NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW' |
359 | 558 | bad = '/../../../.ssh/id_pub' | 559 | bad = '/../../../.ssh/id_pub' |
361 | 559 | e = raises(ValueError, self.klass.reltemp, chash, bad) | 560 | e = raises(ValueError, self.klass.reltmp, chash, bad) |
362 | 560 | self.assertEqual( | 561 | self.assertEqual( |
363 | 561 | str(e), | 562 | str(e), |
364 | 562 | 'ext %r does not match pattern %r' % (bad, EXT_PAT) | 563 | 'ext %r does not match pattern %r' % (bad, EXT_PAT) |
365 | @@ -750,27 +751,27 @@ | |||
366 | 750 | self.assertFalse(path.exists(f)) | 751 | self.assertFalse(path.exists(f)) |
367 | 751 | self.assertTrue(path.isdir(d)) | 752 | self.assertTrue(path.isdir(d)) |
368 | 752 | 753 | ||
370 | 753 | def test_temp(self): | 754 | def test_tmp(self): |
371 | 754 | tmp = TempDir() | 755 | tmp = TempDir() |
372 | 755 | inst = self.klass(tmp.path) | 756 | inst = self.klass(tmp.path) |
373 | 756 | 757 | ||
374 | 757 | self.assertEqual( | 758 | self.assertEqual( |
376 | 758 | inst.temp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW'), | 759 | inst.tmp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW'), |
377 | 759 | tmp.join('transfers', 'NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW') | 760 | tmp.join('transfers', 'NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW') |
378 | 760 | ) | 761 | ) |
379 | 761 | self.assertEqual( | 762 | self.assertEqual( |
381 | 762 | inst.temp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW', ext='ogv'), | 763 | inst.tmp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW', ext='ogv'), |
382 | 763 | tmp.join('transfers', 'NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW.ogv') | 764 | tmp.join('transfers', 'NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW.ogv') |
383 | 764 | ) | 765 | ) |
384 | 765 | 766 | ||
385 | 766 | # Test to make sure hashes are getting checked with safe_b32(): | 767 | # Test to make sure hashes are getting checked with safe_b32(): |
386 | 767 | bad = 'NWBNVXVK5..GIOW7MYR4K3KA5K22W7NW' | 768 | bad = 'NWBNVXVK5..GIOW7MYR4K3KA5K22W7NW' |
388 | 768 | e = raises(ValueError, inst.temp, bad) | 769 | e = raises(ValueError, inst.tmp, bad) |
389 | 769 | self.assertEqual( | 770 | self.assertEqual( |
390 | 770 | str(e), | 771 | str(e), |
391 | 771 | 'b32: cannot b32decode %r: Non-base32 digit found' % bad | 772 | 'b32: cannot b32decode %r: Non-base32 digit found' % bad |
392 | 772 | ) | 773 | ) |
394 | 773 | e = raises(ValueError, inst.temp, bad, ext='ogv') | 774 | e = raises(ValueError, inst.tmp, bad, ext='ogv') |
395 | 774 | self.assertEqual( | 775 | self.assertEqual( |
396 | 775 | str(e), | 776 | str(e), |
397 | 776 | 'b32: cannot b32decode %r: Non-base32 digit found' % bad | 777 | 'b32: cannot b32decode %r: Non-base32 digit found' % bad |
398 | @@ -779,7 +780,7 @@ | |||
399 | 779 | # Test to make sure ext is getting checked with safe_ext(): | 780 | # Test to make sure ext is getting checked with safe_ext(): |
400 | 780 | chash = 'NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW' | 781 | chash = 'NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW' |
401 | 781 | bad = '/../../../.ssh/id_pub' | 782 | bad = '/../../../.ssh/id_pub' |
403 | 782 | e = raises(ValueError, inst.temp, chash, bad) | 783 | e = raises(ValueError, inst.tmp, chash, bad) |
404 | 783 | self.assertEqual( | 784 | self.assertEqual( |
405 | 784 | str(e), | 785 | str(e), |
406 | 785 | 'ext %r does not match pattern %r' % (bad, EXT_PAT) | 786 | 'ext %r does not match pattern %r' % (bad, EXT_PAT) |
407 | @@ -794,13 +795,13 @@ | |||
408 | 794 | self.assertFalse(path.exists(f)) | 795 | self.assertFalse(path.exists(f)) |
409 | 795 | self.assertFalse(path.exists(d)) | 796 | self.assertFalse(path.exists(d)) |
410 | 796 | self.assertEqual( | 797 | self.assertEqual( |
412 | 797 | inst.temp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW'), | 798 | inst.tmp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW'), |
413 | 798 | f | 799 | f |
414 | 799 | ) | 800 | ) |
415 | 800 | self.assertFalse(path.exists(f)) | 801 | self.assertFalse(path.exists(f)) |
416 | 801 | self.assertFalse(path.exists(d)) | 802 | self.assertFalse(path.exists(d)) |
417 | 802 | self.assertEqual( | 803 | self.assertEqual( |
419 | 803 | inst.temp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW', create=True), | 804 | inst.tmp('NWBNVXVK5DQGIOW7MYR4K3KA5K22W7NW', create=True), |
420 | 804 | f | 805 | f |
421 | 805 | ) | 806 | ) |
422 | 806 | self.assertFalse(path.exists(f)) | 807 | self.assertFalse(path.exists(f)) |
423 | @@ -894,7 +895,113 @@ | |||
424 | 894 | self.assertEqual(path.dirname(fp.name), imports) | 895 | self.assertEqual(path.dirname(fp.name), imports) |
425 | 895 | self.assertTrue(fp.name.endswith('.mov')) | 896 | self.assertTrue(fp.name.endswith('.mov')) |
426 | 896 | 897 | ||
428 | 897 | def test_finalize_transfer(self): | 898 | def test_tmp_move(self): |
429 | 899 | tmp = TempDir() | ||
430 | 900 | base = tmp.join('.dmedia') | ||
431 | 901 | dst_d = tmp.join('.dmedia', mov_hash[:2]) | ||
432 | 902 | dst = tmp.join('.dmedia', mov_hash[:2], mov_hash[2:] + '.mov') | ||
433 | 903 | inst = self.klass(base) | ||
434 | 904 | |||
435 | 905 | # Test with wrong tmp_fp type | ||
436 | 906 | e = raises(TypeError, inst.tmp_move, 17, mov_hash, 'mov') | ||
437 | 907 | self.assertEqual(str(e), TYPE_ERROR % ('tmp_fp', file, int, 17)) | ||
438 | 908 | self.assertFalse(path.exists(dst_d)) | ||
439 | 909 | self.assertFalse(path.exists(dst)) | ||
440 | 910 | |||
441 | 911 | # Test with tmp_fp opened in wrong mode | ||
442 | 912 | bad = tmp.touch('.dmedia', 'bad1.mov') | ||
443 | 913 | for mode in ('r', 'w', 'r+'): | ||
444 | 914 | fp = open(bad, mode) | ||
445 | 915 | e = raises(ValueError, inst.tmp_move, fp, mov_hash, 'mov') | ||
446 | 916 | self.assertEqual( | ||
447 | 917 | str(e), | ||
448 | 918 | "tmp_fp: mode must be 'rb', 'wb', or 'r+b'; got %r" % mode | ||
449 | 919 | ) | ||
450 | 920 | self.assertFalse(path.exists(dst_d)) | ||
451 | 921 | self.assertFalse(path.exists(dst)) | ||
452 | 922 | self.assertFalse(fp.closed) | ||
453 | 923 | |||
454 | 924 | # Test when filename isn't contained in base (to ensure that | ||
455 | 925 | # FileStore.check_path() is used to validate tmp_fp.name) | ||
456 | 926 | bad = tmp.touch('.dmedia', '..', 'bad2.mov') | ||
457 | 927 | fp = open(bad, 'rb') | ||
458 | 928 | e = raises(FileStoreTraversal, inst.tmp_move, fp, mov_hash, 'mov') | ||
459 | 929 | self.assertEqual(e.pathname, bad) | ||
460 | 930 | self.assertEqual(e.abspath, tmp.join('bad2.mov')) | ||
461 | 931 | self.assertEqual(e.base, base) | ||
462 | 932 | self.assertFalse(path.exists(dst_d)) | ||
463 | 933 | self.assertFalse(path.exists(dst)) | ||
464 | 934 | self.assertFalse(fp.closed) | ||
465 | 935 | |||
466 | 936 | # Test that chash is validated with safe_b32() | ||
467 | 937 | good = tmp.write('yup', '.dmedia', 'imports', 'good.mov') | ||
468 | 938 | os.chmod(good, 0o660) | ||
469 | 939 | fp = open(good, 'rb') | ||
470 | 940 | b32 = 'NWBNVXVK5DQGIOW7MYR4K3KA' | ||
471 | 941 | e = raises(ValueError, inst.tmp_move, fp, b32, 'mov') | ||
472 | 942 | self.assertEqual( | ||
473 | 943 | str(e), | ||
474 | 944 | "len(b32) must be 32; got 24: 'NWBNVXVK5DQGIOW7MYR4K3KA'" | ||
475 | 945 | ) | ||
476 | 946 | self.assertFalse(path.exists(dst_d)) | ||
477 | 947 | self.assertFalse(path.exists(dst)) | ||
478 | 948 | self.assertFalse(fp.closed) | ||
479 | 949 | |||
480 | 950 | # Test that ext is validate with safe_ext() | ||
481 | 951 | bad_ext = '/etc/ssh' | ||
482 | 952 | e = raises(ValueError, inst.tmp_move, fp, mov_hash, bad_ext) | ||
483 | 953 | self.assertEqual( | ||
484 | 954 | str(e), | ||
485 | 955 | 'ext %r does not match pattern %r' % (bad_ext, EXT_PAT) | ||
486 | 956 | ) | ||
487 | 957 | self.assertFalse(path.exists(dst_d)) | ||
488 | 958 | self.assertFalse(path.exists(dst)) | ||
489 | 959 | self.assertFalse(fp.closed) | ||
490 | 960 | |||
491 | 961 | # Test when tmp_fp is closed | ||
492 | 962 | fp.close() | ||
493 | 963 | e = raises(ValueError, inst.tmp_move, fp, mov_hash, 'mov') | ||
494 | 964 | self.assertEqual( | ||
495 | 965 | str(e), | ||
496 | 966 | 'tmp_fp is closed, must be open: %r' % good | ||
497 | 967 | ) | ||
498 | 968 | self.assertFalse(path.exists(dst_d)) | ||
499 | 969 | self.assertFalse(path.exists(dst)) | ||
500 | 970 | self.assertTrue(fp.closed) | ||
501 | 971 | |||
502 | 972 | # Test when it's all good | ||
503 | 973 | fp = open(good, 'rb') | ||
504 | 974 | self.assertEqual(stat.S_IMODE(os.stat(good).st_mode), 0o660) | ||
505 | 975 | self.assertEqual(inst.tmp_move(fp, mov_hash, 'mov'), dst) | ||
506 | 976 | self.assertFalse(path.exists(good)) | ||
507 | 977 | self.assertTrue(path.isdir(dst_d)) | ||
508 | 978 | self.assertTrue(path.isfile(dst)) | ||
509 | 979 | self.assertEqual(stat.S_IMODE(os.stat(dst).st_mode), 0o444) | ||
510 | 980 | self.assertEqual(open(dst, 'rb').read(), 'yup') | ||
511 | 981 | self.assertTrue(fp.closed) | ||
512 | 982 | |||
513 | 983 | # Test when it's a duplicate | ||
514 | 984 | dup = tmp.write('wowza', '.dmedia', 'imports', 'dup') | ||
515 | 985 | os.chmod(dup, 0o660) | ||
516 | 986 | fp = open(dup, 'rb') | ||
517 | 987 | e = raises(DuplicateFile, inst.tmp_move, fp, mov_hash, 'mov') | ||
518 | 988 | self.assertEqual(e.chash, mov_hash) | ||
519 | 989 | self.assertEqual(e.src, dup) | ||
520 | 990 | self.assertEqual(e.dst, dst) | ||
521 | 991 | self.assertFalse(fp.closed) | ||
522 | 992 | |||
523 | 993 | # Make sure dup wasn't altered | ||
524 | 994 | self.assertTrue(path.isfile(dup)) | ||
525 | 995 | self.assertEqual(stat.S_IMODE(os.stat(dup).st_mode), 0o660) | ||
526 | 996 | self.assertEqual(open(dup, 'rb').read(), 'wowza') | ||
527 | 997 | |||
528 | 998 | # Make sure dst wasn't altered | ||
529 | 999 | self.assertTrue(path.isdir(dst_d)) | ||
530 | 1000 | self.assertTrue(path.isfile(dst)) | ||
531 | 1001 | self.assertEqual(stat.S_IMODE(os.stat(dst).st_mode), 0o444) | ||
532 | 1002 | self.assertEqual(open(dst, 'rb').read(), 'yup') | ||
533 | 1003 | |||
534 | 1004 | def test_tmp_verify_move(self): | ||
535 | 898 | tmp = TempDir() | 1005 | tmp = TempDir() |
536 | 899 | inst = self.klass(tmp.path) | 1006 | inst = self.klass(tmp.path) |
537 | 900 | 1007 | ||
538 | @@ -904,22 +1011,22 @@ | |||
539 | 904 | dst = tmp.join(mov_hash[:2], mov_hash[2:] + '.mov') | 1011 | dst = tmp.join(mov_hash[:2], mov_hash[2:] + '.mov') |
540 | 905 | 1012 | ||
541 | 906 | # Test when transfers/ dir doesn't exist: | 1013 | # Test when transfers/ dir doesn't exist: |
543 | 907 | e = raises(IOError, inst.finalize_transfer, mov_hash, 'mov') | 1014 | e = raises(IOError, inst.tmp_verify_move, mov_hash, 'mov') |
544 | 908 | self.assertFalse(path.exists(src_d)) | 1015 | self.assertFalse(path.exists(src_d)) |
545 | 909 | self.assertFalse(path.exists(dst_d)) | 1016 | self.assertFalse(path.exists(dst_d)) |
546 | 910 | self.assertFalse(path.exists(dst)) | 1017 | self.assertFalse(path.exists(dst)) |
547 | 911 | 1018 | ||
548 | 912 | # Test when transfers/ exists but file does not: | 1019 | # Test when transfers/ exists but file does not: |
550 | 913 | self.assertEqual(inst.temp(mov_hash, 'mov', create=True), src) | 1020 | self.assertEqual(inst.tmp(mov_hash, 'mov', create=True), src) |
551 | 914 | self.assertTrue(path.isdir(src_d)) | 1021 | self.assertTrue(path.isdir(src_d)) |
553 | 915 | e = raises(IOError, inst.finalize_transfer, mov_hash, 'mov') | 1022 | e = raises(IOError, inst.tmp_verify_move, mov_hash, 'mov') |
554 | 916 | self.assertFalse(path.exists(src)) | 1023 | self.assertFalse(path.exists(src)) |
555 | 917 | self.assertFalse(path.exists(dst_d)) | 1024 | self.assertFalse(path.exists(dst_d)) |
556 | 918 | self.assertFalse(path.exists(dst)) | 1025 | self.assertFalse(path.exists(dst)) |
557 | 919 | 1026 | ||
558 | 920 | # Test when file has wrong content hash and wrong size: | 1027 | # Test when file has wrong content hash and wrong size: |
559 | 921 | open(src, 'wb').write(open(sample_thm, 'rb').read()) | 1028 | open(src, 'wb').write(open(sample_thm, 'rb').read()) |
561 | 922 | e = raises(IntegrityError, inst.finalize_transfer, mov_hash, 'mov') | 1029 | e = raises(IntegrityError, inst.tmp_verify_move, mov_hash, 'mov') |
562 | 923 | self.assertEqual(e.got, thm_hash) | 1030 | self.assertEqual(e.got, thm_hash) |
563 | 924 | self.assertEqual(e.expected, mov_hash) | 1031 | self.assertEqual(e.expected, mov_hash) |
564 | 925 | self.assertEqual(e.filename, src) | 1032 | self.assertEqual(e.filename, src) |
565 | @@ -942,7 +1049,7 @@ | |||
566 | 942 | fp2.close() | 1049 | fp2.close() |
567 | 943 | self.assertEqual(path.getsize(sample_mov), path.getsize(src)) | 1050 | self.assertEqual(path.getsize(sample_mov), path.getsize(src)) |
568 | 944 | 1051 | ||
570 | 945 | e = raises(IntegrityError, inst.finalize_transfer, mov_hash, 'mov') | 1052 | e = raises(IntegrityError, inst.tmp_verify_move, mov_hash, 'mov') |
571 | 946 | self.assertEqual(e.got, 'UECTT7A7EIHZ2SGGBMMO5WTTSVU4SUWM') | 1053 | self.assertEqual(e.got, 'UECTT7A7EIHZ2SGGBMMO5WTTSVU4SUWM') |
572 | 947 | self.assertEqual(e.expected, mov_hash) | 1054 | self.assertEqual(e.expected, mov_hash) |
573 | 948 | self.assertEqual(e.filename, src) | 1055 | self.assertEqual(e.filename, src) |
574 | @@ -959,7 +1066,7 @@ | |||
575 | 959 | fp2.write(chunk) | 1066 | fp2.write(chunk) |
576 | 960 | fp1.close() | 1067 | fp1.close() |
577 | 961 | fp2.close() | 1068 | fp2.close() |
579 | 962 | self.assertEqual(inst.finalize_transfer(mov_hash, 'mov'), dst) | 1069 | self.assertEqual(inst.tmp_verify_move(mov_hash, 'mov'), dst) |
580 | 963 | self.assertTrue(path.isdir(src_d)) | 1070 | self.assertTrue(path.isdir(src_d)) |
581 | 964 | self.assertFalse(path.exists(src)) | 1071 | self.assertFalse(path.exists(src)) |
582 | 965 | self.assertTrue(path.isdir(dst_d)) | 1072 | self.assertTrue(path.isdir(dst_d)) |
Looks decent :)