Merge lp:~mandel/ubuntuone-client/add-virtual-watches into lp:ubuntuone-client
- add-virtual-watches
- Merge into trunk
Status: | Rejected |
---|---|
Rejected by: | dobey |
Proposed branch: | lp:~mandel/ubuntuone-client/add-virtual-watches |
Merge into: | lp:ubuntuone-client |
Diff against target: |
1545 lines (+636/-333) 3 files modified
tests/platform/windows/test_filesystem_notifications.py (+527/-255) ubuntuone/platform/windows/filesystem_notifications.py (+102/-75) ubuntuone/platform/windows/os_helper.py (+7/-3) |
To merge this branch: | bzr merge lp:~mandel/ubuntuone-client/add-virtual-watches |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Facundo Batista (community) | Needs Fixing | ||
Diego Sarmentero (community) | Approve | ||
Review via email: mp+88726@code.launchpad.net |
Commit message
Fixes lp:907511
This branch allows the Windows Watch implementation to behave more like the one found on pyinotify. The way this is achieved is the following:
* Moved _subdirs var to be a dictionary that store a map between a child path and if it is watched or not. The reason is that child_paths are relative to self._path which means that we store less info. The boolean state if we are watching the path, if not, we ignore all the events from that path.
* When we start watching all child events are added to _subdirs are not being watch until sd explicitly asks it.
* When adding a child we add it children to the _subdirs. The are not thread or run issues because the sd will add the watches always in the main twisted thread which is the one used to process the event.
* Moved _ignored_paths to be a set for performance.
* Remove the auto-add parameters and var since this are not expose to the rest of sd and they do not work on linux.
* Modify the tests to use a MementoWatch. The MementoWatch memorizes all the events that have been triggered by the ReadDirecotryCh
* Added a new filter that ensures that IN_MODIFY|IS_DIR events are not raised when the child is not watched.
Description of the change
Fixes lp:907511
This branch allows the Windows Watch implementation to behave more like the one found on pyinotify. The way this is achieved is the following:
* Moved _subdirs var to be a dictionary that store a map between a child path and if it is watched or not. The reason is that child_paths are relative to self._path which means that we store less info. The boolean state if we are watching the path, if not, we ignore all the events from that path.
* When we start watching all child events are added to _subdirs are not being watch until sd explicitly asks it.
* When adding a child we add it children to the _subdirs. The are not thread or run issues because the sd will add the watches always in the main twisted thread which is the one used to process the event.
* Moved _ignored_paths to be a set for performance.
* Remove the auto-add parameters and var since this are not expose to the rest of sd and they do not work on linux.
* Modify the tests to use a MementoWatch. The MementoWatch memorizes all the events that have been triggered by the ReadDirecotryCh
* Added a new filter that ensures that IN_MODIFY|IS_DIR events are not raised when the child is not watched.
Brian Curtin (brian.curtin) wrote : | # |
Manuel de la Peña (mandel) wrote : | # |
> Here's a mini-review. I don't have my dev environment setup and don't know a
> lot of the context, plus I can't run it yet...but here's a few things I
> noticed.
>
> ######
> test_filesystem
>
> 80 + self.mask = fs.FILE_
> 81 + fs.FILE_
>
> Minor style thing, but I noticed in a big import (that was removed) that long
> lines were wrapped with parentheses instead of breaking with backslashes.
> Maybe do the same here? (Sorry, had to get my first nitpick cosmetic comment
> out of the way :)
It makes sense, I actually prefer to use () than \ since for me it seems cleaner. Will get that fix.
>
> =======
> filesystem_
>
> _get_partial_
>
> It's about 2-3x faster to take a slice of the last character and compare it
> rather than using path.endswith. This may not be as clean as using endswith,
> but figured it may be worth mentioning. Something like "if not path[-1] ==
> os.path.sep:"
>
On it! good catch!
> Other than that, this method looks good.
> ******
> + self.log.
>
> none existing > nonexistent
Fixing
> ******
> + def add_child_
>
> This new method looks fine to me.
> ******
> + """Add a new path to be watch.
>
> watch > watched
Fixing.
> ============
> os_helper.py
>
> native_listdir
>
> What is the purpose of having listdir and native_listdir if they are the same?
Really good question, and it is indeed a bug. os.listdir does return certain values, such as system folders, that we want to ignore. I should be using the wrapped method from os_helper.
Thx for the review!
- 1199. By Manuel de la Peña
-
Fixed code according to review.
Diego Sarmentero (diegosarmentero) wrote : | # |
+1
All green!
Alejandro J. Cura (alecu) wrote : | # |
The branch looks interesting.
A few comments:
----
941 + if any([file_
942 + for path, watched in self._subdirs.
The list comprehension used inside the "any" could be replaced by a generator expression just by dropping the [ and ]. With that change a new list won't have to be constructed, and the any will be able to finish faster by not checking the rest if any item matches the expression.
----
"we get no events, lets therefore ignore them" ->
"we get no events, let's therefore ignore them
----
"lets remove the path" ->
"let's remove the path"
----
There are many references to "manager._wdm[0].", and it looks a bit flaky.
I think that instead this line:
self.patch(fs, 'Watch', MementoWatch)
should be replaced by something like:
self.patch(fs, 'Watch', lambda *args, **kwargs: self.mementowat
and:
def mementowatch_
return self.memento_watch
----
"suddirs" -> "subdirs"
- 1200. By Manuel de la Peña
-
Fixed code according to review comments.
- 1201. By Manuel de la Peña
-
Fixed some stupidly broken tests.
- 1202. By Manuel de la Peña
-
Merged with trunk.
Facundo Batista (facundo) wrote : | # |
There's a race condition, as we talked by IRC, when getting subdirs for the _subdirs internal structure, and the real directory/files creation in disk.
Alejandro J. Cura (alecu) wrote : | # |
Line 132 seems to be missing a leading space:
130 + def mementowatch_
131 + """Factory that creates a new Watch for the tests."""
132 + kwargs[
133 + self.memento_watch = MementoWatch(*args, **kwargs)
134 + return self.memento_watch
- 1203. By Manuel de la Peña
-
Added a number of extra tests to ensure that we can work around the race condition.
- 1204. By Manuel de la Peña
-
Face palm.
- 1205. By Manuel de la Peña
-
Face palm.
- 1206. By Manuel de la Peña
-
Fixed the race condition to ensure that if a child is missing we add it to the _subdirs dict. Fixed old tests that relied in some implementation details.
- 1207. By Manuel de la Peña
-
Fixed broken listdir.
- 1208. By Manuel de la Peña
-
Simplified the number of if statements used to ensure that the child path is present.
- 1209. By Manuel de la Peña
-
Face palm.
- 1210. By Manuel de la Peña
-
Place the parent_dir definition closer to the use. Fix the deep test because a and A are the same on windows.
Unmerged revisions
- 1210. By Manuel de la Peña
-
Place the parent_dir definition closer to the use. Fix the deep test because a and A are the same on windows.
- 1209. By Manuel de la Peña
-
Face palm.
- 1208. By Manuel de la Peña
-
Simplified the number of if statements used to ensure that the child path is present.
- 1207. By Manuel de la Peña
-
Fixed broken listdir.
- 1206. By Manuel de la Peña
-
Fixed the race condition to ensure that if a child is missing we add it to the _subdirs dict. Fixed old tests that relied in some implementation details.
- 1205. By Manuel de la Peña
-
Face palm.
- 1204. By Manuel de la Peña
-
Face palm.
- 1203. By Manuel de la Peña
-
Added a number of extra tests to ensure that we can work around the race condition.
- 1202. By Manuel de la Peña
-
Merged with trunk.
- 1201. By Manuel de la Peña
-
Fixed some stupidly broken tests.
Preview Diff
1 | === modified file 'tests/platform/windows/test_filesystem_notifications.py' |
2 | --- tests/platform/windows/test_filesystem_notifications.py 2011-10-28 18:43:23 +0000 |
3 | +++ tests/platform/windows/test_filesystem_notifications.py 2012-02-03 11:33:18 +0000 |
4 | @@ -21,8 +21,8 @@ |
5 | import os |
6 | import tempfile |
7 | import thread |
8 | -import time |
9 | import itertools |
10 | +import string |
11 | |
12 | from twisted.internet import defer |
13 | from win32file import FILE_NOTIFY_INFORMATION |
14 | @@ -37,25 +37,12 @@ |
15 | IN_DELETE, |
16 | IN_OPEN, |
17 | ) |
18 | -from ubuntuone.platform.windows import filesystem_notifications |
19 | -from ubuntuone.platform.windows.filesystem_notifications import ( |
20 | - FilesystemMonitor, |
21 | - NotifyProcessor, |
22 | - Watch, |
23 | - WatchManager, |
24 | - FILE_NOTIFY_CHANGE_FILE_NAME, |
25 | - FILE_NOTIFY_CHANGE_DIR_NAME, |
26 | - FILE_NOTIFY_CHANGE_ATTRIBUTES, |
27 | - FILE_NOTIFY_CHANGE_SIZE, |
28 | - FILE_NOTIFY_CHANGE_LAST_WRITE, |
29 | - FILE_NOTIFY_CHANGE_SECURITY, |
30 | - FILE_NOTIFY_CHANGE_LAST_ACCESS, |
31 | - WINDOWS_ACTIONS_NAMES, |
32 | -) |
33 | +from ubuntuone.platform.windows import filesystem_notifications as fs |
34 | +from ubuntuone.platform.windows.filesystem_notifications import Watch |
35 | |
36 | #create a rever mapping to use it in the tests. |
37 | REVERSE_WINDOWS_ACTIONS = {} |
38 | -for key, value in filesystem_notifications.WINDOWS_ACTIONS.iteritems(): |
39 | +for key, value in fs.WINDOWS_ACTIONS.iteritems(): |
40 | REVERSE_WINDOWS_ACTIONS[value] = key |
41 | |
42 | |
43 | @@ -101,6 +88,25 @@ |
44 | if self.main_thread_id: |
45 | assert self.main_thread_id == thread.get_ident() |
46 | |
47 | +class MementoWatch(Watch): |
48 | + """A watch that will remember all the events raised.""" |
49 | + |
50 | + def __init__(self, watch_descriptor, path, mask, processor, buf_size=8192, |
51 | + all_events_count=0): |
52 | + """Create a new instance.""" |
53 | + super(MementoWatch, self).__init__(watch_descriptor, path, mask, |
54 | + processor, buf_size) |
55 | + self.all_events_count = all_events_count |
56 | + self._all_events = [] |
57 | + self.deferred = defer.Deferred() |
58 | + |
59 | + def _process_events(self, events): |
60 | + """Process evens from the queue.""" |
61 | + self._all_events.extend(events) |
62 | + super(MementoWatch, self)._process_events(events) |
63 | + if len(self._all_events) == self.all_events_count: |
64 | + self.deferred.callback(self._all_events) |
65 | + |
66 | |
67 | class TestWatch(BaseTwistedTestCase): |
68 | """Test the watch so that it returns the same events as pyinotify.""" |
69 | @@ -111,18 +117,18 @@ |
70 | def setUp(self): |
71 | yield super(TestWatch, self).setUp() |
72 | self.basedir = self.mktemp('test_root') |
73 | - self.mask = FILE_NOTIFY_CHANGE_FILE_NAME | \ |
74 | - FILE_NOTIFY_CHANGE_DIR_NAME | \ |
75 | - FILE_NOTIFY_CHANGE_ATTRIBUTES | \ |
76 | - FILE_NOTIFY_CHANGE_SIZE | \ |
77 | - FILE_NOTIFY_CHANGE_LAST_WRITE | \ |
78 | - FILE_NOTIFY_CHANGE_SECURITY | \ |
79 | - FILE_NOTIFY_CHANGE_LAST_ACCESS |
80 | + self.mask = fs.FILE_NOTIFY_CHANGE_FILE_NAME | \ |
81 | + fs.FILE_NOTIFY_CHANGE_DIR_NAME | \ |
82 | + fs.FILE_NOTIFY_CHANGE_ATTRIBUTES | \ |
83 | + fs.FILE_NOTIFY_CHANGE_SIZE | \ |
84 | + fs.FILE_NOTIFY_CHANGE_LAST_WRITE | \ |
85 | + fs.FILE_NOTIFY_CHANGE_SECURITY | \ |
86 | + fs.FILE_NOTIFY_CHANGE_LAST_ACCESS |
87 | self.memento = MementoHandler() |
88 | self.memento.setLevel(logging.DEBUG) |
89 | self.raw_events = [] |
90 | self.paths_checked = [] |
91 | - old_is_dir = Watch._path_is_dir |
92 | + old_is_dir = fs.Watch._path_is_dir |
93 | |
94 | def file_notify_information_wrapper(buf, data): |
95 | """Wrapper that gets the events and adds them to the list.""" |
96 | @@ -131,7 +137,7 @@ |
97 | # If we use extend we wont have the same logging because it will |
98 | # group all events in a single lists which is not what the COM API |
99 | # does. |
100 | - str_events = [(WINDOWS_ACTIONS_NAMES[action], path) for action, path in |
101 | + str_events = [(fs.WINDOWS_ACTIONS_NAMES[action], path) for action, path in |
102 | events] |
103 | self.raw_events.append(str_events) |
104 | return events |
105 | @@ -142,46 +148,55 @@ |
106 | self.paths_checked.append((path, result)) |
107 | return result |
108 | |
109 | - self.patch(filesystem_notifications, 'FILE_NOTIFY_INFORMATION', |
110 | + self.patch(fs, 'FILE_NOTIFY_INFORMATION', |
111 | file_notify_information_wrapper) |
112 | - self.patch(filesystem_notifications.Watch, '_path_is_dir', |
113 | + self.patch(fs.Watch, '_path_is_dir', |
114 | path_is_dir_wrapper) |
115 | |
116 | @defer.inlineCallbacks |
117 | - def _perform_operations(self, path, mask, auto_add, actions, |
118 | - number_events): |
119 | + def _perform_operations(self, path, mask, actions, filtered_events_count, |
120 | + children_paths=[], all_events_count=0, ignored_paths=[]): |
121 | """Perform the file operations and returns the recorded events.""" |
122 | - handler = TestCaseHandler(number_events=number_events) |
123 | - manager = WatchManager(handler) |
124 | - yield manager.add_watch(os_helper.get_windows_valid_path(path), mask, |
125 | - auto_add=auto_add) |
126 | + handler = TestCaseHandler(number_events=filtered_events_count) |
127 | + manager = fs.WatchManager(handler) |
128 | + self.memento_watch = None |
129 | + |
130 | + def mementowatch_factory(all_events_count, *args, **kwargs): |
131 | + """Factory that creates a new Watch for the tests.""" |
132 | + kwargs['all_events_count'] = all_events_count |
133 | + self.memento_watch = MementoWatch(*args, **kwargs) |
134 | + return self.memento_watch |
135 | + |
136 | + # lets always use the memento watch, that way we know the number of events in total |
137 | + self.patch(fs, 'Watch', lambda *args, **kwargs: |
138 | + mementowatch_factory(all_events_count, *args, **kwargs)) |
139 | + |
140 | + yield manager.add_watch(os_helper.get_windows_valid_path(path), mask) |
141 | + self.memento_watch.all_events_count = all_events_count |
142 | + |
143 | + # add a watch to the children paths |
144 | + for child_path in children_paths: |
145 | + yield manager.add_watch(os_helper.get_windows_valid_path(child_path), |
146 | + mask) |
147 | + |
148 | + # add that paths that will be ignored in the watch |
149 | + for ignored in ignored_paths: |
150 | + self.memento_watch.ignore_path(ignored) |
151 | + |
152 | # change the logger so that we can check the logs if we wanted |
153 | - manager._wdm[0].log.addHandler(self.memento) |
154 | + self.memento_watch.log.addHandler(self.memento) |
155 | # clean logger later |
156 | - self.addCleanup(manager._wdm[0].log.removeHandler, self.memento) |
157 | + self.addCleanup(self.memento_watch.log.removeHandler, self.memento) |
158 | # execution the actions |
159 | actions() |
160 | # process the recorded events |
161 | - ret = yield handler.deferred |
162 | + events = yield self.memento_watch.deferred |
163 | + if filtered_events_count != 0: |
164 | + ret = yield handler.deferred |
165 | + else: |
166 | + ret = handler.processed_events |
167 | self.addCleanup(manager.stop) |
168 | - defer.returnValue(ret) |
169 | - |
170 | - def _perform_timed_operations(self, path, mask, auto_add, actions, |
171 | - time_out): |
172 | - """Perform the file operations and returns the recorded events.""" |
173 | - manager = WatchManager() |
174 | - manager.add_watch(os_helper.get_windows_valid_path(path), mask, |
175 | - auto_add=auto_add) |
176 | - # change the logger so that we can check the logs if we wanted |
177 | - manager._wdm[0].log.addHandler(self.memento) |
178 | - # clean logger later |
179 | - self.addCleanup(manager._wdm[0].log.removeHandler, self.memento) |
180 | - # execution the actions |
181 | - actions() |
182 | - # process the recorded events |
183 | - time.sleep(time_out) |
184 | - events = self.handler.processed_events |
185 | - return events |
186 | + defer.returnValue((ret, events)) |
187 | |
188 | def _assert_logs(self, events): |
189 | """Assert the debug logs.""" |
190 | @@ -208,8 +223,10 @@ |
191 | os.fsync(fd) |
192 | fd.close() |
193 | |
194 | - events = yield self._perform_operations(self.basedir, self.mask, False, |
195 | - create_file, 1) |
196 | + events, all_events = yield self._perform_operations(self.basedir, self.mask, |
197 | + create_file, |
198 | + filtered_events_count=1, |
199 | + all_events_count=1) |
200 | event = events[0] |
201 | self.assertFalse(event.dir) |
202 | self.assertEqual(0x100, event.mask) |
203 | @@ -230,8 +247,10 @@ |
204 | """Action for the test.""" |
205 | os.mkdir(dir_name) |
206 | |
207 | - events = yield self._perform_operations(self.basedir, self.mask, False, |
208 | - create_dir, 1) |
209 | + events, all_events = yield self._perform_operations(self.basedir, self.mask, |
210 | + create_dir, |
211 | + filtered_events_count=1, |
212 | + all_events_count=1) |
213 | event = events[0] |
214 | self.assertTrue(event.dir) |
215 | self.assertEqual(0x40000100, event.mask) |
216 | @@ -254,8 +273,10 @@ |
217 | """Action for the test.""" |
218 | os.remove(file_name) |
219 | |
220 | - events = yield self._perform_operations(self.basedir, self.mask, False, |
221 | - remove_file, 1) |
222 | + events, all_events = yield self._perform_operations(self.basedir, self.mask, |
223 | + remove_file, |
224 | + filtered_events_count=1, |
225 | + all_events_count=1) |
226 | event = events[0] |
227 | self.assertFalse(event.dir) |
228 | self.assertEqual(0x200, event.mask) |
229 | @@ -278,8 +299,10 @@ |
230 | """Action for the test.""" |
231 | os.rmdir(dir_name) |
232 | |
233 | - events = yield self._perform_operations(self.basedir, self.mask, False, |
234 | - remove_dir, 1) |
235 | + events, all_events = yield self._perform_operations(self.basedir, self.mask, |
236 | + remove_dir, |
237 | + filtered_events_count=1, |
238 | + all_events_count=1) |
239 | event = events[0] |
240 | self.assertTrue(event.dir) |
241 | self.assertEqual(0x40000200, event.mask) |
242 | @@ -304,8 +327,10 @@ |
243 | fd.write('test') |
244 | fd.close() |
245 | |
246 | - events = yield self._perform_operations(self.basedir, self.mask, False, |
247 | - write_file, 1) |
248 | + events, all_events = yield self._perform_operations(self.basedir, self.mask, |
249 | + write_file, |
250 | + filtered_events_count=1, |
251 | + all_events_count=1) |
252 | event = events[0] |
253 | self.assertFalse(event.dir) |
254 | self.assertEqual(0x2, event.mask) |
255 | @@ -331,8 +356,10 @@ |
256 | """Action for the test.""" |
257 | os.rename(from_file_name, to_file_name) |
258 | |
259 | - events = yield self._perform_operations(self.basedir, self.mask, |
260 | - False, move_file, 2) |
261 | + events, all_events = yield self._perform_operations(self.basedir, self.mask, |
262 | + move_file, |
263 | + filtered_events_count=2, |
264 | + all_events_count=2) |
265 | move_from_event = events[0] |
266 | move_to_event = events[1] |
267 | # first test the move from |
268 | @@ -376,8 +403,10 @@ |
269 | # while on linux we will have to do some sort of magic like facundo |
270 | # did, on windows we will get a deleted event which is much more |
271 | # cleaner, first time ever windows is nicer! |
272 | - events = yield self._perform_operations(self.basedir, self.mask, False, |
273 | - move_file, 1) |
274 | + events, all_events = yield self._perform_operations(self.basedir, self.mask, |
275 | + move_file, |
276 | + filtered_events_count=1, |
277 | + all_events_count=1) |
278 | event = events[0] |
279 | self.assertFalse(event.dir) |
280 | self.assertEqual(0x200, event.mask) |
281 | @@ -406,8 +435,10 @@ |
282 | |
283 | # while on linux we have to do some magic operations like facundo did |
284 | # on windows we have a created file, hurray! |
285 | - events = yield self._perform_operations(self.basedir, self.mask, False, |
286 | - move_files, 1) |
287 | + events, all_events = yield self._perform_operations(self.basedir, self.mask, |
288 | + move_files, |
289 | + filtered_events_count=1, |
290 | + all_events_count=1) |
291 | event = events[0] |
292 | self.assertFalse(event.dir) |
293 | self.assertEqual(0x100, event.mask) |
294 | @@ -433,8 +464,10 @@ |
295 | """Action for the test.""" |
296 | os.rename(from_dir_name, to_dir_name) |
297 | |
298 | - events = yield self._perform_operations(self.basedir, |
299 | - self.mask, False, move_file, 2) |
300 | + events, all_events = yield self._perform_operations(self.basedir, self.mask, |
301 | + move_file, |
302 | + filtered_events_count=2, |
303 | + all_events_count=2) |
304 | move_from_event = events[0] |
305 | move_to_event = events[1] |
306 | # first test the move from |
307 | @@ -475,8 +508,10 @@ |
308 | 'test_dir_moved_to_not_watched_dir')) |
309 | |
310 | # on windows a move to outside a watched dir translates to a remove |
311 | - events = yield self._perform_operations(self.basedir, self.mask, False, |
312 | - move_dir, 1) |
313 | + events, all_events = yield self._perform_operations(self.basedir, self.mask, |
314 | + move_dir, |
315 | + filtered_events_count=1, |
316 | + all_events_count=1) |
317 | event = events[0] |
318 | self.assertTrue(event.dir) |
319 | self.assertEqual(0x40000200, event.mask) |
320 | @@ -501,8 +536,10 @@ |
321 | """Action for the test.""" |
322 | os.rename(from_dir_name, to_dir_name) |
323 | |
324 | - events = yield self._perform_operations(self.basedir, self.mask, False, |
325 | - move_dir, 1) |
326 | + events, all_events = yield self._perform_operations(self.basedir, self.mask, |
327 | + move_dir, |
328 | + filtered_events_count=1, |
329 | + all_events_count=1) |
330 | event = events[0] |
331 | self.assertTrue(event.dir) |
332 | self.assertEqual(0x40000100, event.mask) |
333 | @@ -513,22 +550,6 @@ |
334 | event.pathname) |
335 | self.assertEqual(0, event.wd) |
336 | |
337 | - def test_exclude_filter(self): |
338 | - """Test that the exclude filter works as expectd.""" |
339 | - handler = TestCaseHandler(number_events=0) |
340 | - manager = WatchManager(handler) |
341 | - # add a watch that will always exclude all actions |
342 | - manager.add_watch(os_helper.get_windows_valid_path(self.basedir), |
343 | - self.mask, auto_add=True, |
344 | - exclude_filter=lambda x: True) |
345 | - # execution the actions |
346 | - file_name = os.path.join(self.basedir, 'test_file_create') |
347 | - open(file_name, 'w').close() |
348 | - # give some time for the system to get the events |
349 | - time.sleep(1) |
350 | - self.assertEqual(0, len(handler.processed_events)) |
351 | - test_exclude_filter.skip = "we must rethink this test." |
352 | - |
353 | def test_open_dir_muted(self): |
354 | """Test that the opening of dirs is ignored.""" |
355 | dir_name = os.path.join(tempfile.mkdtemp(), 'test_dir_open') |
356 | @@ -539,134 +560,192 @@ |
357 | """Action for the test.""" |
358 | os.startfile(dir_name) |
359 | |
360 | - events = self._perform_timed_operations(self.basedir, self.mask, False, |
361 | - open_dir, 2) |
362 | + events = self._perform_operations(self.basedir, self.mask, |
363 | + open_dir, |
364 | + filtered_events_count=2, |
365 | + all_events_count=1) |
366 | self.assertEqual(0, len(events)) |
367 | test_open_dir_muted.skip = "we must rethink this test." |
368 | |
369 | + @defer.inlineCallbacks |
370 | def test_ignore_path(self): |
371 | """Test that events from a path are ignored.""" |
372 | - events = [] |
373 | - |
374 | - def fake_processor(event): |
375 | - """Memorize the processed events.""" |
376 | - events.append(event) |
377 | - |
378 | - path = u'\\\\?\\C:\\path' # a valid windows path |
379 | - child = u'child' |
380 | - watch = Watch(1, path, None, True, fake_processor) |
381 | - watch.ignore_path(os.path.join(path, child)) |
382 | - paths_to_ignore = [] |
383 | - for file_name in 'abcdef': |
384 | - paths_to_ignore.append((1, os.path.join(child, file_name))) |
385 | - # ensure that the watch is watching |
386 | - watch._watching = True |
387 | - watch._process_events(paths_to_ignore) |
388 | - self.assertEqual(0, len(events), |
389 | - 'All events should have been ignored.') |
390 | - |
391 | + child_path = os.path.join(self.basedir, 'child') |
392 | + os.mkdir(child_path) |
393 | + file_names = 'abcdef' |
394 | + |
395 | + def create_files(): |
396 | + """Create files to get events.""" |
397 | + for file_name in file_names: |
398 | + # create a file similar to the ignore, but not a dir to test if |
399 | + # we do not match them wrongly |
400 | + fd = open(os.path.join(child_path, file_name), 'w') |
401 | + fd.flush() |
402 | + os.fsync(fd) |
403 | + fd.close() |
404 | + |
405 | + events, all_events = yield self._perform_operations(self.basedir, self.mask, |
406 | + create_files, |
407 | + filtered_events_count=0, |
408 | + all_events_count= len(file_names) * 2, |
409 | + children_paths=[child_path], |
410 | + ignored_paths=[u'\\\\?\\' + child_path]) |
411 | + |
412 | + not_processed_children = [file_name for event, file_name in all_events] |
413 | + for file_name in file_names: |
414 | + self.assertIn(os.path.join('child', file_name), not_processed_children) |
415 | + |
416 | + @defer.inlineCallbacks |
417 | def test_not_ignore_path(self): |
418 | """Test that we do get the events when they do not match.""" |
419 | - events = [] |
420 | - |
421 | - def fake_processor(event): |
422 | - """Memorize the processed events.""" |
423 | - events.append(event) |
424 | - |
425 | - path = u'\\\\?\\C:\\path' # a valid windows path |
426 | - child = u'child' |
427 | - watch = Watch(1, path, None, True, fake_processor) |
428 | - watch.ignore_path(os.path.join(path, child)) |
429 | - paths_not_to_ignore = [] |
430 | - for file_name in 'abcdef': |
431 | - paths_not_to_ignore.append((1, os.path.join( |
432 | - child + file_name, file_name))) |
433 | - # ensure that the watch is watching |
434 | - watch._watching = True |
435 | - watch._process_events(paths_not_to_ignore) |
436 | - self.assertEqual(len(paths_not_to_ignore), len(events), |
437 | - 'No events should have been ignored.') |
438 | - |
439 | + child_path = os.path.join(self.basedir, 'child') |
440 | + os.mkdir(child_path) |
441 | + file_names = 'abcdef' |
442 | + |
443 | + def create_files(): |
444 | + """Create files to get events.""" |
445 | + for file_name in file_names: |
446 | + # create a file similar to the ignore, but not a dir to test if |
447 | + # we do not match them wrongly |
448 | + fd = open(child_path + file_name, 'w') |
449 | + fd.flush() |
450 | + os.fsync(fd) |
451 | + fd.close() |
452 | + |
453 | + events, all_events = yield self._perform_operations(self.basedir, self.mask, |
454 | + create_files, |
455 | + filtered_events_count=len(file_names), |
456 | + all_events_count= len(file_names), |
457 | + children_paths=[child_path], |
458 | + ignored_paths=[u'\\\\?\\' + child_path]) |
459 | + |
460 | + processed_children = [e.pathname for e in events] |
461 | + for file_name in file_names: |
462 | + self.assertIn(child_path + file_name, processed_children) |
463 | + |
464 | + |
465 | + @defer.inlineCallbacks |
466 | def test_mixed_ignore_path(self): |
467 | """Test that we do get the correct events.""" |
468 | - events = [] |
469 | - |
470 | - def fake_processor(event): |
471 | - """Memorize the processed events.""" |
472 | - events.append(event.pathname) |
473 | - |
474 | - child = u'child' |
475 | - path = u'\\\\?\\C:\\path\\' # a valid windows path |
476 | - watch = Watch(1, path, None, True, fake_processor) |
477 | - watch.ignore_path(os.path.join(path, child)) |
478 | - paths_not_to_ignore = [] |
479 | - paths_to_ignore = [] |
480 | - expected_events = [] |
481 | - for file_name in 'abcdef': |
482 | - valid = os.path.join(child + file_name, file_name) |
483 | - paths_to_ignore.append((1, os.path.join(child, file_name))) |
484 | - paths_not_to_ignore.append((1, valid)) |
485 | - expected_events.append(os.path.join('C:\\path', valid)) |
486 | - # ensure that the watch is watching |
487 | - watch._watching = True |
488 | - watch._process_events(paths_not_to_ignore) |
489 | - self.assertEqual(len(paths_not_to_ignore), len(events), |
490 | - 'Wrong number of events ignored.') |
491 | - self.assertTrue(all([event in expected_events for event in events]), |
492 | - 'Paths ignored that should have not been ignored.') |
493 | - |
494 | + child_path = os.path.join(self.basedir, 'child') |
495 | + os.mkdir(child_path) |
496 | + ignored_path = os.path.join(self.basedir, 'ignored') |
497 | + os.mkdir(ignored_path) |
498 | + file_names = 'abcdef' |
499 | + |
500 | + def create_files(): |
501 | + """Create files to get events.""" |
502 | + for file_name in file_names: |
503 | + # create a file per child path |
504 | + fd = open(os.path.join(child_path, file_name), 'w') |
505 | + fd.flush() |
506 | + os.fsync(fd) |
507 | + fd.close() |
508 | + fd = open(os.path.join(ignored_path, file_name), 'w') |
509 | + fd.flush() |
510 | + os.fsync(fd) |
511 | + fd.close() |
512 | + |
513 | + events, all_events = yield self._perform_operations(self.basedir, self.mask, |
514 | + create_files, |
515 | + filtered_events_count=len(file_names), |
516 | + all_events_count= len(file_names * 2) * 2, |
517 | + children_paths=[child_path, ignored_path], |
518 | + ignored_paths=[u'\\\\?\\' + ignored_path]) |
519 | + |
520 | + processed_children = [e.pathname for e in events] |
521 | + |
522 | + # assert that each expected child path is present |
523 | + for file_name in file_names: |
524 | + self.assertIn(os.path.join(child_path, file_name), processed_children) |
525 | + |
526 | + # assert that the ignored are not present |
527 | + for file_name in file_names: |
528 | + self.assertNotIn(os.path.join(ignored_path, file_name), processed_children) |
529 | + |
530 | + # assert that the ignored path are indeed in all the events |
531 | + not_processed_children = [file_name for event, file_name in all_events] |
532 | + |
533 | + for file_name in file_names: |
534 | + self.assertIn(os.path.join('ignored', file_name), not_processed_children) |
535 | + |
536 | + @defer.inlineCallbacks |
537 | def test_undo_ignore_path_ignored(self): |
538 | """Test that we do deal with events from and old ignored path.""" |
539 | - events = [] |
540 | - |
541 | - def fake_processor(event): |
542 | - """Memorize the processed events.""" |
543 | - events.append(event) |
544 | - |
545 | - path = u'\\\\?\\C:\\path' # a valid windows path |
546 | - child = u'child' |
547 | - watch = Watch(1, path, None, True, fake_processor) |
548 | - watch.ignore_path(os.path.join(path, child)) |
549 | - watch.remove_ignored_path(os.path.join(path, child)) |
550 | - paths_not_to_ignore = [] |
551 | - for file_name in 'abcdef': |
552 | - paths_not_to_ignore.append((1, os.path.join(child, file_name))) |
553 | - # ensure that the watch is watching |
554 | - watch._watching = True |
555 | - watch._process_events(paths_not_to_ignore) |
556 | - self.assertEqual(len(paths_not_to_ignore), len(events), |
557 | - 'All events should have been accepted.') |
558 | - |
559 | + child_path = os.path.join(self.basedir, 'child') |
560 | + os.mkdir(child_path) |
561 | + file_names = 'abcdef' |
562 | + |
563 | + def create_files(): |
564 | + """Create files to get events.""" |
565 | + self.memento_watch.remove_ignored_path(u'\\\\?\\' + child_path) |
566 | + for file_name in file_names: |
567 | + # create a file similar to the ignore, but not a dir to test if |
568 | + # we do not match them wrongly |
569 | + fd = open(os.path.join(child_path, file_name), 'w') |
570 | + fd.flush() |
571 | + os.fsync(fd) |
572 | + fd.close() |
573 | + |
574 | + events, all_events = yield self._perform_operations(self.basedir, self.mask, |
575 | + create_files, |
576 | + filtered_events_count=len(file_names), |
577 | + all_events_count= len(file_names) * 2, |
578 | + children_paths=[child_path], |
579 | + ignored_paths=[u'\\\\?\\' + child_path]) |
580 | + |
581 | + processed_children = [e.pathname for e in events] |
582 | + |
583 | + # assert that each expected child path is present |
584 | + for file_name in file_names: |
585 | + self.assertIn(os.path.join(child_path, file_name), processed_children) |
586 | + |
587 | + @defer.inlineCallbacks |
588 | def test_undo_ignore_path_other_ignored(self): |
589 | """Test that we can undo and the other path is ignored.""" |
590 | - events = [] |
591 | - |
592 | - def fake_processor(event): |
593 | - """Memorize the processed events.""" |
594 | - events.append(event.pathname) |
595 | - |
596 | - path = u'\\\\?\\C:\\path' # a valid windows path |
597 | - child_a = u'childa' |
598 | - child_b = u'childb' |
599 | - watch = Watch(1, path, None, True, fake_processor) |
600 | - watch.ignore_path(os.path.join(path, child_a)) |
601 | - watch.ignore_path(os.path.join(path, child_b)) |
602 | - watch.remove_ignored_path(os.path.join(path, child_a)) |
603 | - paths_to_ignore = [] |
604 | - paths_not_to_ignore = [] |
605 | - expected_events = [] |
606 | - for file_name in 'abcdef': |
607 | - paths_to_ignore.append((1, os.path.join(child_b, file_name))) |
608 | - valid = os.path.join(child_a, file_name) |
609 | - paths_not_to_ignore.append((1, valid)) |
610 | - expected_events.append(os.path.join('C:\\path', valid)) |
611 | - # ensure that the watch is watching |
612 | - watch._watching = True |
613 | - watch._process_events(paths_not_to_ignore) |
614 | - self.assertEqual(len(paths_not_to_ignore), len(events), |
615 | - 'All events should have been accepted.') |
616 | - self.assertTrue(all([event in expected_events for event in events]), |
617 | - 'Paths ignored that should have not been ignored.') |
618 | + child_path = os.path.join(self.basedir, 'child') |
619 | + os.mkdir(child_path) |
620 | + ignored_path = os.path.join(self.basedir, 'ignored') |
621 | + os.mkdir(ignored_path) |
622 | + file_names = 'abcdef' |
623 | + |
624 | + def create_files(): |
625 | + """Create files to get events.""" |
626 | + self.memento_watch.remove_ignored_path(u'\\\\?\\' + child_path) |
627 | + for file_name in file_names: |
628 | + # create a file per child path |
629 | + fd = open(os.path.join(child_path, file_name), 'w') |
630 | + fd.flush() |
631 | + os.fsync(fd) |
632 | + fd.close() |
633 | + fd = open(os.path.join(ignored_path, file_name), 'w') |
634 | + fd.flush() |
635 | + os.fsync(fd) |
636 | + fd.close() |
637 | + |
638 | + events, all_events = yield self._perform_operations(self.basedir, self.mask, |
639 | + create_files, |
640 | + filtered_events_count=len(file_names), |
641 | + all_events_count= len(file_names * 2) * 2, |
642 | + children_paths=[child_path, ignored_path], |
643 | + ignored_paths=[u'\\\\?\\' + ignored_path, |
644 | + u'\\\\?\\' + child_path]) |
645 | + |
646 | + processed_children = [e.pathname for e in events] |
647 | + |
648 | + # assert that each expected child path is present |
649 | + for file_name in file_names: |
650 | + self.assertIn(os.path.join(child_path, file_name), processed_children) |
651 | + |
652 | + # assert that the ignored are not present |
653 | + for file_name in file_names: |
654 | + self.assertNotIn(os.path.join(ignored_path, file_name), processed_children) |
655 | + |
656 | + # assert that the ignored path are indeed in all the events |
657 | + not_processed_children = [file_name for event, file_name in all_events] |
658 | + |
659 | + for file_name in file_names: |
660 | + self.assertIn(os.path.join('ignored', file_name), not_processed_children) |
661 | |
662 | @defer.inlineCallbacks |
663 | def test_call_deferred_already_called(self): |
664 | @@ -678,7 +757,7 @@ |
665 | method_args.append((args, kwargs),) |
666 | |
667 | path = u'\\\\?\\C:\\path' # a valid windows path |
668 | - watch = Watch(1, path, None, True, None) |
669 | + watch = fs.Watch(1, path, None, None) |
670 | yield watch._watch_started_deferred.callback(True) |
671 | watch._call_deferred(fake_call, None) |
672 | self.assertEqual(0, len(method_args)) |
673 | @@ -692,20 +771,20 @@ |
674 | method_args.append((args, kwargs),) |
675 | |
676 | path = u'\\\\?\\C:\\path' # a valid windows path |
677 | - watch = Watch(1, path, None, True, None) |
678 | + watch = fs.Watch(1, path, None, None) |
679 | watch._call_deferred(fake_call, None) |
680 | self.assertEqual(1, len(method_args)) |
681 | |
682 | def test_started_property(self): |
683 | """Test that the started property returns the started deferred.""" |
684 | path = u'\\\\?\\C:\\path' # a valid windows path |
685 | - watch = Watch(1, path, None, True, None) |
686 | + watch = fs.Watch(1, path, None, None) |
687 | self.assertEqual(watch.started, watch._watch_started_deferred) |
688 | |
689 | def test_stopped_property(self): |
690 | """Test that the stopped property returns the stopped deferred.""" |
691 | path = u'\\\\?\\C:\\path' # a valid windows path |
692 | - watch = Watch(1, path, None, True, None) |
693 | + watch = fs.Watch(1, path, None, None) |
694 | self.assertEqual(watch.stopped, watch._watch_stopped_deferred) |
695 | |
696 | def random_error(self, *args): |
697 | @@ -716,8 +795,8 @@ |
698 | def test_start_watching_fails_early_in_thread(self): |
699 | """An early failure inside the thread should errback the deferred.""" |
700 | test_path = self.mktemp("test_directory") |
701 | - self.patch(filesystem_notifications, "CreateFileW", self.random_error) |
702 | - watch = Watch(1, test_path, None, True, None) |
703 | + self.patch(fs, "CreateFileW", self.random_error) |
704 | + watch = fs.Watch(1, test_path, None, None) |
705 | d = watch.start_watching() |
706 | yield self.assertFailure(d, FakeException) |
707 | |
708 | @@ -725,9 +804,9 @@ |
709 | def test_start_watching_fails_late_in_thread(self): |
710 | """A late failure inside the thread should errback the deferred.""" |
711 | test_path = self.mktemp("test_directory") |
712 | - self.patch(filesystem_notifications, "ReadDirectoryChangesW", |
713 | + self.patch(fs, "ReadDirectoryChangesW", |
714 | self.random_error) |
715 | - watch = Watch(1, test_path, None, True, None) |
716 | + watch = fs.Watch(1, test_path, None, None) |
717 | d = watch.start_watching() |
718 | yield self.assertFailure(d, FakeException) |
719 | |
720 | @@ -736,12 +815,12 @@ |
721 | """CloseHandle is called when there's an error in the watch thread.""" |
722 | test_path = self.mktemp("test_directory") |
723 | close_called = [] |
724 | - self.patch(filesystem_notifications, "CreateFileW", lambda *_: None) |
725 | - self.patch(filesystem_notifications, "CloseHandle", |
726 | + self.patch(fs, "CreateFileW", lambda *_: None) |
727 | + self.patch(fs, "CloseHandle", |
728 | close_called.append) |
729 | - self.patch(filesystem_notifications, "ReadDirectoryChangesW", |
730 | + self.patch(fs, "ReadDirectoryChangesW", |
731 | self.random_error) |
732 | - watch = Watch(1, test_path, self.mask, True, None) |
733 | + watch = fs.Watch(1, test_path, self.mask, None) |
734 | d = watch.start_watching() |
735 | yield self.assertFailure(d, FakeException) |
736 | self.assertEqual(len(close_called), 1) |
737 | @@ -751,7 +830,7 @@ |
738 | def test_stop_watching_fired_when_watch_thread_finishes(self): |
739 | """The deferred returned is fired when the watch thread finishes.""" |
740 | test_path = self.mktemp("another_test_directory") |
741 | - watch = Watch(1, test_path, self.mask, True, None) |
742 | + watch = fs.Watch(1, test_path, self.mask, None) |
743 | yield watch.start_watching() |
744 | self.assertNotEqual(watch._watch_handle, None) |
745 | yield watch.stop_watching() |
746 | @@ -762,7 +841,7 @@ |
747 | path = u'\\\\?\\C:\\path\\to\\no\\dir' |
748 | test_path = self.mktemp("test_directory") |
749 | self.patch(os.path, 'exists', lambda path: False) |
750 | - watch = Watch(1, test_path, self.mask, True, None) |
751 | + watch = fs.Watch(1, test_path, self.mask, None) |
752 | self.assertFalse(watch._path_is_dir(path)) |
753 | |
754 | def test_is_path_dir_missing_in_subdir(self): |
755 | @@ -770,8 +849,9 @@ |
756 | path = u'\\\\?\\C:\\path\\to\\no\\dir' |
757 | test_path = self.mktemp("test_directory") |
758 | self.patch(os.path, 'exists', lambda path: False) |
759 | - watch = Watch(1, test_path, self.mask, True, None) |
760 | - watch._subdirs.add(path) |
761 | + watch = fs.Watch(1, test_path, self.mask, None) |
762 | + child_path = watch._get_partial_child_path_dir(path) |
763 | + watch._subdirs[child_path] = False |
764 | self.assertTrue(watch._path_is_dir(path)) |
765 | |
766 | def test_is_path_dir_present_is_dir(self): |
767 | @@ -780,8 +860,8 @@ |
768 | test_path = self.mktemp("test_directory") |
769 | self.patch(os.path, 'exists', lambda path: True) |
770 | self.patch(os.path, 'isdir', lambda path: True) |
771 | - watch = Watch(1, test_path, self.mask, True, None) |
772 | - watch._subdirs.add(path) |
773 | + watch = fs.Watch(1, test_path, self.mask, None) |
774 | + watch._subdirs[path] = False |
775 | self.assertTrue(watch._path_is_dir(path)) |
776 | |
777 | def test_is_path_dir_present_no_dir(self): |
778 | @@ -790,34 +870,36 @@ |
779 | test_path = self.mktemp("test_directory") |
780 | self.patch(os.path, 'exists', lambda path: True) |
781 | self.patch(os.path, 'isdir', lambda path: False) |
782 | - watch = Watch(1, test_path, self.mask, True, None) |
783 | - watch._subdirs.add(path) |
784 | + watch = fs.Watch(1, test_path, self.mask, None) |
785 | + watch._subdirs[path] = False |
786 | self.assertFalse(watch._path_is_dir(path)) |
787 | |
788 | def test_update_subdirs_create_not_present(self): |
789 | """Test when we update on a create event and not present.""" |
790 | path = u'\\\\?\\C:\\path\\to\\no\\dir' |
791 | test_path = self.mktemp("test_directory") |
792 | - watch = Watch(1, test_path, self.mask, True, None) |
793 | - watch._update_subdirs(path, REVERSE_WINDOWS_ACTIONS[IN_CREATE]) |
794 | - self.assertTrue(path in watch._subdirs) |
795 | + watch = fs.Watch(1, test_path, self.mask, None) |
796 | + child_path = watch._get_partial_child_path_dir(path) |
797 | + watch._update_subdirs(child_path, REVERSE_WINDOWS_ACTIONS[IN_CREATE]) |
798 | + self.assertTrue(child_path in watch._subdirs) |
799 | |
800 | def test_update_subdirs_create_present(self): |
801 | """Test when we update on a create event and is present.""" |
802 | path = u'\\\\?\\C:\\path\\to\\no\\dir' |
803 | test_path = self.mktemp("test_directory") |
804 | - watch = Watch(1, test_path, self.mask, True, None) |
805 | - watch._subdirs.add(path) |
806 | + watch = fs.Watch(1, test_path, self.mask, None) |
807 | + child_path = watch._get_partial_child_path_dir(path) |
808 | + watch._subdirs[child_path] = False |
809 | old_length = len(watch._subdirs) |
810 | - watch._update_subdirs(path, REVERSE_WINDOWS_ACTIONS[IN_CREATE]) |
811 | - self.assertTrue(path in watch._subdirs) |
812 | + watch._update_subdirs(child_path, REVERSE_WINDOWS_ACTIONS[IN_CREATE]) |
813 | + self.assertTrue(child_path in watch._subdirs) |
814 | self.assertEqual(old_length, len(watch._subdirs)) |
815 | |
816 | def test_update_subdirs_delete_not_present(self): |
817 | """Test when we delete and is not present.""" |
818 | path = u'\\\\?\\C:\\path\\to\\no\\dir' |
819 | test_path = self.mktemp("test_directory") |
820 | - watch = Watch(1, test_path, self.mask, True, None) |
821 | + watch = fs.Watch(1, test_path, self.mask, None) |
822 | watch._update_subdirs(path, REVERSE_WINDOWS_ACTIONS[IN_DELETE]) |
823 | self.assertTrue(path not in watch._subdirs) |
824 | |
825 | @@ -825,11 +907,203 @@ |
826 | """Test when we delete and is present.""" |
827 | path = u'\\\\?\\C:\\path\\to\\no\\dir' |
828 | test_path = self.mktemp("test_directory") |
829 | - watch = Watch(1, test_path, self.mask, True, None) |
830 | - watch._subdirs.add(path) |
831 | + watch = fs.Watch(1, test_path, self.mask, None) |
832 | + child_path = watch._get_partial_child_path_dir(path) |
833 | + watch._subdirs[child_path] = False |
834 | watch._update_subdirs(path, REVERSE_WINDOWS_ACTIONS[IN_DELETE]) |
835 | self.assertTrue(path not in watch._subdirs) |
836 | |
837 | + @defer.inlineCallbacks |
838 | + def test_events_ignored_no_virtual_watch(self): |
839 | + """Test when the events of a subdir are ignored.""" |
840 | + child_folder = os.path.join(self.basedir, 'child') |
841 | + os.mkdir(child_folder) |
842 | + file_name = os.path.join(child_folder, 'test_file_create') |
843 | + |
844 | + def create_file(): |
845 | + """Action used for the test.""" |
846 | + # simply create a new file |
847 | + fd = open(file_name, 'w') |
848 | + fd.flush() |
849 | + os.fsync(fd) |
850 | + fd.close() |
851 | + |
852 | + events, all_events = yield self._perform_operations(self.basedir, self.mask, |
853 | + create_file, |
854 | + filtered_events_count=0, |
855 | + all_events_count=2) |
856 | + self.assertEqual(0, len(events), events) |
857 | + # assert the logging |
858 | + self._assert_logs(events) |
859 | + |
860 | + @defer.inlineCallbacks |
861 | + def test_file_events_raised_no_virtual_watch(self): |
862 | + """Test when the events of files are fired but not suddirs.""" |
863 | + child_folder = os.path.join(self.basedir, 'child') |
864 | + os.mkdir(child_folder) |
865 | + file_name = os.path.join(self.basedir, 'test_file_create') |
866 | + child_file_name = os.path.join(child_folder, 'test_file_create') |
867 | + |
868 | + def create_file(): |
869 | + """Action used for the test.""" |
870 | + # simply create a new file |
871 | + fd = open(file_name, 'w') |
872 | + fd.flush() |
873 | + os.fsync(fd) |
874 | + fd.close() |
875 | + # do the same with the file that should be ignored |
876 | + fd = open(child_file_name, 'w') |
877 | + fd.flush() |
878 | + os.fsync(fd) |
879 | + fd.close() |
880 | + |
881 | + events, all_events = yield self._perform_operations(self.basedir, self.mask, |
882 | + create_file, |
883 | + filtered_events_count=1, |
884 | + all_events_count=2) |
885 | + self.assertEqual(1, len(events)) |
886 | + event = events[0] |
887 | + self.assertFalse(event.dir) |
888 | + self.assertEqual(0x100, event.mask) |
889 | + self.assertEqual('IN_CREATE', event.maskname) |
890 | + self.assertEqual(os.path.split(file_name)[1], event.name) |
891 | + self.assertEqual('.', event.path) |
892 | + self.assertEqual(os.path.join(self.basedir, file_name), event.pathname) |
893 | + self.assertEqual(0, event.wd) |
894 | + # assert the logging |
895 | + self._assert_logs(events) |
896 | + |
897 | + @defer.inlineCallbacks |
898 | + def test_events_ignored_deep_tree(self): |
899 | + """Test that all the events if a deep tree are ignored.""" |
900 | + child_folder = os.path.join(self.basedir, *[c for c in string.letters]) |
901 | + os.makedirs(child_folder) |
902 | + |
903 | + def create_files(): |
904 | + """Action used for the test.""" |
905 | + # create a file per level of the child folder |
906 | + current_path = self.basedir |
907 | + for c in string.letters[:24]: |
908 | + current_path = os.path.join(current_path, c) |
909 | + file_name = os.path.join(current_path, 'test_file_create') |
910 | + fd = open(file_name, 'w') |
911 | + fd.flush() |
912 | + os.fsync(fd) |
913 | + fd.close() |
914 | + |
915 | + events, all_events = yield self._perform_operations(self.basedir, self.mask, |
916 | + create_files, filtered_events_count=0, |
917 | + all_events_count=len(string.letters[:24])) |
918 | + self.assertEqual(0, len(events), str(events)) |
919 | + # assert the logging |
920 | + self._assert_logs(events) |
921 | + |
922 | + @defer.inlineCallbacks |
923 | + def test_events_raised_virtual_watch(self): |
924 | + """Test that the events from a virtual watch are raised.""" |
925 | + child_folder = os.path.join(self.basedir, 'child') |
926 | + os.mkdir(child_folder) |
927 | + file_name = os.path.join(self.basedir, 'test_file_create') |
928 | + child_file_name = os.path.join(child_folder, 'test_file_create') |
929 | + |
930 | + def create_file(): |
931 | + """Action used for the test.""" |
932 | + # simply create a new file |
933 | + fd = open(file_name, 'w') |
934 | + fd.flush() |
935 | + os.fsync(fd) |
936 | + fd.close() |
937 | + # do the same with the file that should be ignored |
938 | + fd = open(child_file_name, 'w') |
939 | + fd.flush() |
940 | + os.fsync(fd) |
941 | + fd.close() |
942 | + |
943 | + events, all_events = yield self._perform_operations(self.basedir, self.mask, |
944 | + create_file, |
945 | + filtered_events_count=2, |
946 | + children_paths=[child_folder], |
947 | + all_events_count=2) |
948 | + self.assertEqual(2, len(events)) |
949 | + # assert the logging |
950 | + self._assert_logs(events) |
951 | + |
952 | + @defer.inlineCallbacks |
953 | + def test_start_watching_race_condition(self): |
954 | + """Test that we can deal with a race condition in the start method.""" |
955 | + # create a dir with several child folders and add a watch. We will |
956 | + # patch the native_listdir so that we do not see some of the child dirs |
957 | + # and later will perfrom an event on one of those kids. The event |
958 | + # should be filtered. |
959 | + for c in string.letters[:24]: |
960 | + os.mkdir(os.path.join(self.basedir, c)) |
961 | + |
962 | + forgotten_dirs = set(string.letters[15:22]) |
963 | + |
964 | + def slow_listdir(path): |
965 | + """Similar to list dir but ignore some dirs.""" |
966 | + all_dirs = set(os.listdir(path)) |
967 | + return all_dirs - forgotten_dirs |
968 | + |
969 | + self.patch(fs, 'native_listdir', slow_listdir) |
970 | + |
971 | + # lets perform a number of events in the forgotten dirs |
972 | + def create_files(): |
973 | + """Create a file on each forgotten dir.""" |
974 | + for c in forgotten_dirs: |
975 | + file_name = os.path.join(self.basedir, c, 'test.txt') |
976 | + fd = open(file_name, 'w') |
977 | + fd.flush() |
978 | + os.fsync(fd) |
979 | + fd.close() |
980 | + |
981 | + events, all_events = yield self._perform_operations(self.basedir, self.mask, |
982 | + create_files, |
983 | + filtered_events_count=0, |
984 | + all_events_count=len(forgotten_dirs) * 2) |
985 | + # assert that the watch does have the correct subdirs |
986 | + for c in string.letters[:24]: |
987 | + self.assertIn(os.path.join(c, u''), self.memento_watch._subdirs) |
988 | + self.assertEqual(0, len(events)) |
989 | + |
990 | + def test_add_child_watch_race_condition(self): |
991 | + """Test that we can deal with a race condition in the add child.""" |
992 | + # like with test_Start_watching race condition we are going to add a |
993 | + # watch and a child watch. In the second addintion the data returned by |
994 | + # the native_listdir will be wrong. |
995 | + child_path = os.path.join(self.basedir, 'child_path') |
996 | + os.mkdir(child_path) |
997 | + for c in string.letters[:24]: |
998 | + os.mkdir(os.path.join(child_path), c) |
999 | + |
1000 | + forgotten_dirs = set(string.letters[15:22]) |
1001 | + |
1002 | + def slow_listdir(path): |
1003 | + """Similar to list dir but ignore some dirs.""" |
1004 | + # are we adding a child or stating? |
1005 | + if path == self.basedir: |
1006 | + return os.listdir(path) |
1007 | + else: |
1008 | + all_dirs = set(os.listdir(path)) |
1009 | + return all_dirs - forgotten_dirs |
1010 | + |
1011 | + def create_files(): |
1012 | + """Create a file on each forgotten dir.""" |
1013 | + for c in forgotten_dirs: |
1014 | + file_name = os.path.join(self.basedir, c, 'test.txt') |
1015 | + fd = open(file_name, 'w') |
1016 | + fd.flush() |
1017 | + os.fsync(fd) |
1018 | + fd.close() |
1019 | + |
1020 | + events, all_events = yield self._perform_operations(self.basedir, self.mask, |
1021 | + create_files, |
1022 | + filtered_events_count=0, |
1023 | + all_events_count=len(forgotten_dirs) * 2) |
1024 | + for c in string.letters[:24]: |
1025 | + self.assertIn(os.path.join(c, u''), self.memento_watch._subdirs) |
1026 | + self.assertEqual(0, len(events)) |
1027 | + |
1028 | |
1029 | class TestWatchManager(BaseTwistedTestCase): |
1030 | """Test the watch manager.""" |
1031 | @@ -840,8 +1114,8 @@ |
1032 | yield super(TestWatchManager, self).setUp() |
1033 | self.parent_path = u'\\\\?\\C:\\' # a valid windows path |
1034 | self.path = self.parent_path + u'path' |
1035 | - self.watch = Watch(1, self.path, None, True, None) |
1036 | - self.manager = WatchManager(None) |
1037 | + self.watch = fs.Watch(1, self.path, None, None) |
1038 | + self.manager = fs.WatchManager(None) |
1039 | self.manager._wdm = {1: self.watch} |
1040 | |
1041 | @defer.inlineCallbacks |
1042 | @@ -854,7 +1128,7 @@ |
1043 | self.was_called = True |
1044 | return defer.succeed(True) |
1045 | |
1046 | - self.patch(Watch, "stop_watching", fake_stop_watching) |
1047 | + self.patch(fs.Watch, "stop_watching", fake_stop_watching) |
1048 | yield self.manager.stop() |
1049 | self.assertTrue(self.was_called, 'The watch stop should be called.') |
1050 | |
1051 | @@ -866,9 +1140,9 @@ |
1052 | """Another fake stop watch.""" |
1053 | return watch.stopped |
1054 | |
1055 | - self.patch(Watch, "stop_watching", fake_stop_watching) |
1056 | + self.patch(fs.Watch, "stop_watching", fake_stop_watching) |
1057 | second_path = self.parent_path + u"second_path" |
1058 | - second_watch = Watch(2, second_path, None, True, None) |
1059 | + second_watch = fs.Watch(2, second_path, None, None) |
1060 | self.manager._wdm[2] = second_watch |
1061 | d = self.manager.stop() |
1062 | self.assertFalse(d.called, "Not fired before all watches end") |
1063 | @@ -908,24 +1182,21 @@ |
1064 | """Fake start watch.""" |
1065 | self.was_called = True |
1066 | |
1067 | - self.patch(Watch, "start_watching", fake_start_watching) |
1068 | + self.patch(fs.Watch, "start_watching", fake_start_watching) |
1069 | self.manager._wdm = {} |
1070 | |
1071 | mask = 'bit_mask' |
1072 | - auto_add = True |
1073 | - self.manager.add_watch(self.path, mask, auto_add=auto_add) |
1074 | + self.manager.add_watch(self.path, mask) |
1075 | self.assertEqual(1, len(self.manager._wdm)) |
1076 | self.assertTrue(self.was_called, 'The watch start was not called.') |
1077 | self.assertEqual(self.path + os.path.sep, self.manager._wdm[0]._path) |
1078 | self.assertEqual(mask, self.manager._wdm[0]._mask) |
1079 | - self.assertEqual(auto_add, self.manager._wdm[0]._auto_add) |
1080 | |
1081 | def test_update_present_watch(self): |
1082 | """Test the update of a present watch.""" |
1083 | mask = 'mask' |
1084 | self.assertRaises(NotImplementedError, self.manager.update_watch, |
1085 | 1, mask) |
1086 | - |
1087 | def test_get_watch_present_wd(self): |
1088 | """Test that the correct path is returned.""" |
1089 | self.assertEqual(self.path + os.path.sep, self.manager.get_path(1)) |
1090 | @@ -1002,11 +1273,12 @@ |
1091 | |
1092 | def test_add_watch_twice(self): |
1093 | """Adding a watch twice succeeds when the watch is running.""" |
1094 | - self.patch(Watch, "start_watching", lambda self: self.started) |
1095 | - manager = WatchManager(None) |
1096 | + self.patch(fs.Watch, "start_watching", lambda self: self.started) |
1097 | + manager = fs.WatchManager(None) |
1098 | # no need to stop watching because start_watching is fake |
1099 | |
1100 | - path = u'\\\\?\\C:\\test' # a valid windows path |
1101 | + path = self.mktemp("test_directory") # a valid windows path |
1102 | + path = u'\\\\?\\' + path |
1103 | mask = 'fake bit mask' |
1104 | d1 = manager.add_watch(path, mask) |
1105 | d2 = manager.add_watch(path, mask) |
1106 | @@ -1114,7 +1386,7 @@ |
1107 | def setUp(self): |
1108 | """set up the diffeent tests.""" |
1109 | yield super(TestNotifyProcessor, self).setUp() |
1110 | - self.processor = NotifyProcessor(None) |
1111 | + self.processor = fs.NotifyProcessor(None) |
1112 | self.general = FakeGeneralProcessor() |
1113 | self.processor.general_processor = self.general |
1114 | |
1115 | @@ -1466,8 +1738,8 @@ |
1116 | |
1117 | def test_add_watch_twice(self): |
1118 | """Check the deferred returned by a second add_watch.""" |
1119 | - self.patch(Watch, "start_watching", lambda self: self.started) |
1120 | - monitor = FilesystemMonitor(None, None) |
1121 | + self.patch(fs.Watch, "start_watching", lambda self: self.started) |
1122 | + monitor = fs.FilesystemMonitor(None, None) |
1123 | # no need to stop watching because start_watching is fake |
1124 | |
1125 | parent_path = 'C:\\test' # a valid windows path in utf-8 bytes |
1126 | @@ -1496,7 +1768,7 @@ |
1127 | |
1128 | ancestors = ['~', '~\\Pictures', '~\\Pictures\\Home', ] |
1129 | volume = FakeVolume(ancestors) |
1130 | - monitor = FilesystemMonitor(None, None) |
1131 | + monitor = fs.FilesystemMonitor(None, None) |
1132 | added = yield monitor.add_watches_to_udf_ancestors(volume) |
1133 | self.assertTrue(added, 'We should always return true.') |
1134 | # lets ensure that we never added the watches |
1135 | |
1136 | === modified file 'ubuntuone/platform/windows/filesystem_notifications.py' |
1137 | --- ubuntuone/platform/windows/filesystem_notifications.py 2011-10-18 13:44:10 +0000 |
1138 | +++ ubuntuone/platform/windows/filesystem_notifications.py 2012-02-03 11:33:18 +0000 |
1139 | @@ -1,9 +1,5 @@ |
1140 | # |
1141 | -# Authors: Manuel de la Pena <manuel@canonical.com> |
1142 | -# Natalia B. Bidart <natalia.bidart@canonical.com> |
1143 | -# Alejandro J. Cura <alecu@canonical.com> |
1144 | -# |
1145 | -# Copyright 2011 Canonical Ltd. |
1146 | +# Copyright 2011-2012 Canonical Ltd. |
1147 | # |
1148 | # This program is free software: you can redistribute it and/or modify it |
1149 | # under the terms of the GNU General Public License version 3, as published |
1150 | @@ -70,6 +66,7 @@ |
1151 | is_valid_syncdaemon_path, |
1152 | is_valid_windows_path, |
1153 | get_syncdaemon_valid_path, |
1154 | + native_listdir, |
1155 | windowspath, |
1156 | ) |
1157 | from ubuntuone import logger |
1158 | @@ -118,13 +115,13 @@ |
1159 | |
1160 | # the default mask to be used in the watches added by the FilesystemMonitor |
1161 | # class |
1162 | -FILESYSTEM_MONITOR_MASK = FILE_NOTIFY_CHANGE_FILE_NAME | \ |
1163 | - FILE_NOTIFY_CHANGE_DIR_NAME | \ |
1164 | - FILE_NOTIFY_CHANGE_ATTRIBUTES | \ |
1165 | - FILE_NOTIFY_CHANGE_SIZE | \ |
1166 | - FILE_NOTIFY_CHANGE_LAST_WRITE | \ |
1167 | - FILE_NOTIFY_CHANGE_SECURITY | \ |
1168 | - FILE_NOTIFY_CHANGE_LAST_ACCESS |
1169 | +FILESYSTEM_MONITOR_MASK = (FILE_NOTIFY_CHANGE_FILE_NAME | |
1170 | + FILE_NOTIFY_CHANGE_DIR_NAME | |
1171 | + FILE_NOTIFY_CHANGE_ATTRIBUTES | |
1172 | + FILE_NOTIFY_CHANGE_SIZE | |
1173 | + FILE_NOTIFY_CHANGE_LAST_WRITE | |
1174 | + FILE_NOTIFY_CHANGE_SECURITY | |
1175 | + FILE_NOTIFY_CHANGE_LAST_ACCESS) |
1176 | |
1177 | THREADPOOL_MAX = 20 |
1178 | |
1179 | @@ -133,9 +130,7 @@ |
1180 | class Watch(object): |
1181 | """Implement the same functions as pyinotify.Watch.""" |
1182 | |
1183 | - def __init__(self, watch_descriptor, path, mask, auto_add, processor, |
1184 | - buf_size=8192): |
1185 | - super(Watch, self).__init__() |
1186 | + def __init__(self, watch_descriptor, path, mask, processor, buf_size=8192): |
1187 | self.log = logging.getLogger('ubuntuone.SyncDaemon.platform.windows.' + |
1188 | 'filesystem_notifications.Watch') |
1189 | self.log.setLevel(TRACE) |
1190 | @@ -146,18 +141,20 @@ |
1191 | self._overlapped.hEvent = CreateEvent(None, 0, 0, None) |
1192 | self._watching = False |
1193 | self._descriptor = watch_descriptor |
1194 | - self._auto_add = auto_add |
1195 | - self._ignore_paths = [] |
1196 | + # ignored paths are those that we actually do want to ignore while |
1197 | + # not watched paths are those whose events are not propagated to |
1198 | + # mimic the linux behaviour |
1199 | + self._ignore_paths = set() |
1200 | self._cookie = None |
1201 | self._source_pathname = None |
1202 | self._process_thread = None |
1203 | self._watch_handle = None |
1204 | # remember the subdirs we have so that when we have a delete we can |
1205 | # check if it was a remove |
1206 | - self._subdirs = set() |
1207 | + self._subdirs = {} |
1208 | # ensure that we work with an abspath and that we can deal with |
1209 | # long paths over 260 chars. |
1210 | - if not path.endswith(os.path.sep): |
1211 | + if path[-1] != os.path.sep: |
1212 | path += os.path.sep |
1213 | self._path = os.path.abspath(path) |
1214 | self._mask = mask |
1215 | @@ -167,7 +164,18 @@ |
1216 | # and this one is fired when the watch has stopped |
1217 | self._watch_stopped_deferred = defer.Deferred() |
1218 | |
1219 | - @is_valid_windows_path(path_indexes=[1]) |
1220 | + def _get_partial_child_path_dir(self, path): |
1221 | + """Return a partial child path for a dir. |
1222 | + |
1223 | + The path is used internally by the watcher to keep track of |
1224 | + child directories. |
1225 | + """ |
1226 | + if path[-1] != os.path.sep: |
1227 | + path += os.path.sep |
1228 | + if path.startswith(self._path): |
1229 | + path = path[len(self._path):] |
1230 | + return path |
1231 | + |
1232 | def _update_subdirs(self, path, event): |
1233 | """Adds the path to the internal subdirs. |
1234 | |
1235 | @@ -175,13 +183,13 @@ |
1236 | will not be checked. |
1237 | """ |
1238 | if WINDOWS_ACTIONS[event] == IN_CREATE: |
1239 | - self._subdirs.add(path) |
1240 | + self._subdirs[path] = False |
1241 | elif WINDOWS_ACTIONS[event] == IN_DELETE and\ |
1242 | path in self._subdirs: |
1243 | - self._subdirs.remove(path) |
1244 | + del self._subdirs[path] |
1245 | |
1246 | @is_valid_windows_path(path_indexes=[1]) |
1247 | - def _path_is_dir(self, path): |
1248 | + def _path_is_dir(self, full_path): |
1249 | """Check if the path is a dir and update the local subdir list.""" |
1250 | |
1251 | # The logic of this function is the following: |
1252 | @@ -191,14 +199,14 @@ |
1253 | # 3. We check if a path is a dir by: |
1254 | # * Asking the os if the path exists. |
1255 | # * Finding the path in self._subdirs |
1256 | - |
1257 | is_dir = False |
1258 | - if os.path.exists(path): |
1259 | - is_dir = os.path.isdir(path) |
1260 | + if os.path.exists(full_path): |
1261 | + is_dir = os.path.isdir(full_path) |
1262 | else: |
1263 | # path does not exists, was it in the internal list? |
1264 | - is_dir = path in self._subdirs |
1265 | - self.log.debug('Is path %r a dir? %s', path, is_dir) |
1266 | + child_path = self._get_partial_child_path_dir(full_path) |
1267 | + is_dir = child_path in self._subdirs |
1268 | + self.log.debug('Is path %r a dir? %s', full_path, is_dir) |
1269 | return is_dir |
1270 | |
1271 | def _process_events(self, events): |
1272 | @@ -210,8 +218,20 @@ |
1273 | # we transform the events to be the same as the one in pyinotify |
1274 | # and then use the proc_fun |
1275 | for action, file_name in events: |
1276 | - if any([file_name.startswith(path) |
1277 | - for path in self._ignore_paths]): |
1278 | + # ignore those paths that are present in the _ignore_paths, this |
1279 | + # are those paths whose watch has been removed. |
1280 | + if any(file_name.startswith(path) |
1281 | + for path in self._ignore_paths): |
1282 | + continue |
1283 | + |
1284 | + # lets get the parent dir to be faster |
1285 | + parent_dir = os.path.dirname(file_name) + os.path.sep |
1286 | + |
1287 | + if parent_dir != unicode(os.path.sep) and\ |
1288 | + not self._subdirs.get(parent_dir, False): |
1289 | + if not parent_dir in self._subdirs: |
1290 | + self.log.warn('Child path %r is missing.', parent_dir) |
1291 | + self._subdirs[parent_dir] = False |
1292 | continue |
1293 | # map the windows events to the pyinotify ones, tis is dirty but |
1294 | # makes the multiplatform better, linux was first :P |
1295 | @@ -221,7 +241,7 @@ |
1296 | is_dir = self._path_is_dir(full_dir_path) |
1297 | if is_dir: |
1298 | # we need to update the list of subdirs that we have |
1299 | - self._update_subdirs(full_dir_path, action) |
1300 | + self._update_subdirs(file_name + os.path.sep, action) |
1301 | mask = WINDOWS_ACTIONS[action] |
1302 | head, tail = os.path.split(file_name) |
1303 | if is_dir: |
1304 | @@ -232,6 +252,11 @@ |
1305 | 'mask': mask, |
1306 | 'name': tail, |
1307 | 'path': '.'} |
1308 | + # if we have a in_modify for a dir that is not watched on linux |
1309 | + # we get no events, let's therefore ignore them |
1310 | + if (WINDOWS_ACTIONS[action] == IN_MODIFY |
1311 | + and is_dir and file_name + os.path.sep in self._subdirs): |
1312 | + continue |
1313 | # by the way in which the win api fires the events we know for |
1314 | # sure that no move events will be added in the wrong order, this |
1315 | # is kind of hacky, I dont like it too much |
1316 | @@ -246,9 +271,6 @@ |
1317 | # FIXME: event deduces the pathname wrong and we need to manually |
1318 | # set it |
1319 | event.pathname = syncdaemon_path |
1320 | - # add the event only if we do not have an exclude filter or |
1321 | - # the exclude filter returns False, that is, the event will not |
1322 | - # be excluded |
1323 | self.log.debug('Pushing event %r to processor.', event) |
1324 | self._processor(event) |
1325 | |
1326 | @@ -301,7 +323,7 @@ |
1327 | ReadDirectoryChangesW( |
1328 | handle, |
1329 | buf, |
1330 | - self._auto_add, |
1331 | + True, |
1332 | self._mask, |
1333 | self._overlapped, |
1334 | ) |
1335 | @@ -316,9 +338,9 @@ |
1336 | if rc == WAIT_OBJECT_0: |
1337 | # Stop event |
1338 | break |
1339 | - # if we continue, it means that we got some data, lets read it |
1340 | + # if we continue, it means that we got some data, let's read it |
1341 | data = GetOverlappedResult(handle, self._overlapped, True) |
1342 | - # lets ead the data and store it in the results |
1343 | + # let's read the data and store it in the results |
1344 | events = FILE_NOTIFY_INFORMATION(buf, data) |
1345 | self.log.debug('Got from ReadDirectoryChangesW %r.', |
1346 | [(WINDOWS_ACTIONS_NAMES[action], path) for action, path in |
1347 | @@ -328,28 +350,40 @@ |
1348 | @is_valid_windows_path(path_indexes=[1]) |
1349 | def ignore_path(self, path): |
1350 | """Add the path of the events to ignore.""" |
1351 | - if not path.endswith(os.path.sep): |
1352 | - path += os.path.sep |
1353 | - if path.startswith(self._path): |
1354 | - path = path[len(self._path):] |
1355 | - self._ignore_paths.append(path) |
1356 | + path = self._get_partial_child_path_dir(path) |
1357 | + self._ignore_paths.add(path) |
1358 | |
1359 | @is_valid_windows_path(path_indexes=[1]) |
1360 | def remove_ignored_path(self, path): |
1361 | """Reaccept path.""" |
1362 | - if not path.endswith(os.path.sep): |
1363 | - path += os.path.sep |
1364 | - if path.startswith(self._path): |
1365 | - path = path[len(self._path):] |
1366 | - if path in self._ignore_paths: |
1367 | - self._ignore_paths.remove(path) |
1368 | + path = self._get_partial_child_path_dir(path) |
1369 | + if path in self._ignore_paths: |
1370 | + self._ignore_paths.remove(path) |
1371 | + |
1372 | + @is_valid_windows_path(path_indexes=[1]) |
1373 | + def add_child_watch(self, path): |
1374 | + """Adds a watch to a child path.""" |
1375 | + if not os.path.exists(path): |
1376 | + self.log.warn('Adding watch to nonexisting path "%s"', path) |
1377 | + return |
1378 | + # we are adding a watch to a path, but not to the children of that |
1379 | + # path, ergo we have to add some extra ignored paths, and then remove |
1380 | + # the parent path from the ignored paths |
1381 | + for current_child in native_listdir(path): |
1382 | + full_child_path = os.path.join(self._path, current_child) |
1383 | + if os.path.isdir(full_child_path): |
1384 | + self._subdirs[current_child + os.path.sep] = False |
1385 | + # let's remove the path from the ignored paths since the children are |
1386 | + # the ones ignored. |
1387 | + sort_path = self._get_partial_child_path_dir(path) |
1388 | + self._subdirs[sort_path] = True |
1389 | |
1390 | def start_watching(self): |
1391 | """Tell the watch to start processing events.""" |
1392 | - for current_child in os.listdir(self._path): |
1393 | + for current_child in native_listdir(self._path): |
1394 | full_child_path = os.path.join(self._path, current_child) |
1395 | if os.path.isdir(full_child_path): |
1396 | - self._subdirs.add(full_child_path) |
1397 | + self._subdirs[current_child + os.path.sep] = False |
1398 | # start to diff threads, one to watch the path, the other to |
1399 | # process the events. |
1400 | self.log.debug('Start watching path.') |
1401 | @@ -362,14 +396,13 @@ |
1402 | self.log.info('Stop watching %s', self._path) |
1403 | SetEvent(self._wait_stop) |
1404 | self._watching = False |
1405 | - self._subdirs = set() |
1406 | + self._subdirs = {} |
1407 | return self.stopped |
1408 | |
1409 | - def update(self, mask, auto_add=False): |
1410 | + def update(self, mask): |
1411 | """Update the info used by the watcher.""" |
1412 | - self.log.debug('update(%s, %s)', mask, auto_add) |
1413 | + self.log.debug('update(%s, %s)', mask) |
1414 | self._mask = mask |
1415 | - self._auto_add = auto_add |
1416 | |
1417 | @property |
1418 | def path(self): |
1419 | @@ -377,10 +410,6 @@ |
1420 | return self._path |
1421 | |
1422 | @property |
1423 | - def auto_add(self): |
1424 | - return self._auto_add |
1425 | - |
1426 | - @property |
1427 | def started(self): |
1428 | """A deferred that will be called when the watch is running.""" |
1429 | return self._watch_started_deferred |
1430 | @@ -433,28 +462,26 @@ |
1431 | except KeyError, e: |
1432 | logging.error(str(e)) |
1433 | |
1434 | - def _add_single_watch(self, path, mask, auto_add=False, |
1435 | - quiet=True): |
1436 | + def _add_single_watch(self, path, mask, quiet=True): |
1437 | if path in self._ignored_paths: |
1438 | # simply removed it from the filter |
1439 | self._ignored_paths.remove(path) |
1440 | return |
1441 | # we need to add a new watch |
1442 | - self.log.debug('add_single_watch(%s, %s, %s, %s)', path, mask, |
1443 | - auto_add, quiet) |
1444 | + self.log.debug('add_single_watch(%s, %s, %s)', path, mask, |
1445 | + quiet) |
1446 | |
1447 | # adjust the number of threads based on the UDFs watched |
1448 | reactor.suggestThreadPoolSize(THREADPOOL_MAX + self._wd_count + 1) |
1449 | self._wdm[self._wd_count] = Watch(self._wd_count, path, |
1450 | - mask, auto_add, self._processor) |
1451 | + mask, self._processor) |
1452 | d = self._wdm[self._wd_count].start_watching() |
1453 | self._wd_count += 1 |
1454 | return d |
1455 | |
1456 | @is_valid_windows_path(path_indexes=[1]) |
1457 | - def add_watch(self, path, mask, auto_add=False, |
1458 | - quiet=True): |
1459 | - """Add a new path tp be watch. |
1460 | + def add_watch(self, path, mask, quiet=True): |
1461 | + """Add a new path to be watched. |
1462 | |
1463 | The method will ensure that the path is not already present. |
1464 | """ |
1465 | @@ -464,24 +491,24 @@ |
1466 | wd = self.get_wd(path) |
1467 | if wd is None: |
1468 | self.log.debug('Adding single watch on %r', path) |
1469 | - return self._add_single_watch(path, mask, auto_add, quiet) |
1470 | + return self._add_single_watch(path, mask, quiet) |
1471 | else: |
1472 | self.log.debug('Watch already exists on %r', path) |
1473 | + if path != self._wdm[wd].path: |
1474 | + self._wdm[wd].add_child_watch(path) |
1475 | return self._wdm[wd].started |
1476 | |
1477 | - def update_watch(self, wd, mask=None, rec=False, |
1478 | - auto_add=False, quiet=True): |
1479 | + def update_watch(self, wd, mask=None, rec=False, quiet=True): |
1480 | raise NotImplementedError("Not implemented on windows.") |
1481 | |
1482 | @is_valid_windows_path(path_indexes=[1]) |
1483 | def get_wd(self, path): |
1484 | """Return the watcher that is used to watch the given path.""" |
1485 | - if not path.endswith(os.path.sep): |
1486 | + if path[-1] != os.path.sep: |
1487 | path = path + os.path.sep |
1488 | for current_wd in self._wdm: |
1489 | watch_path = self._wdm[current_wd].path |
1490 | - if ((watch_path == path or ( |
1491 | - watch_path in path and self._wdm[current_wd].auto_add)) |
1492 | + if (watch_path in path |
1493 | and path not in self._ignored_paths): |
1494 | return current_wd |
1495 | |
1496 | @@ -560,10 +587,10 @@ |
1497 | |
1498 | def process_IN_MODIFY(self, event): |
1499 | """Capture a modify event and fake an open ^ close write events.""" |
1500 | - # lets ignore dir changes |
1501 | + # let's ignore dir changes |
1502 | if event.dir: |
1503 | return |
1504 | - # on windows we just get IN_MODIFY, lets always fake |
1505 | + # on windows we just get IN_MODIFY, let's always fake |
1506 | # an OPEN & CLOSE_WRITE couple |
1507 | raw_open = raw_close = { |
1508 | 'wd': event.wd, |
1509 | @@ -760,7 +787,7 @@ |
1510 | # the logic to check if the watch is already set |
1511 | # is all in WatchManager.add_watch |
1512 | return self._watch_manager.add_watch(dirpath, |
1513 | - FILESYSTEM_MONITOR_MASK, auto_add=True) |
1514 | + FILESYSTEM_MONITOR_MASK) |
1515 | |
1516 | def add_watches_to_udf_ancestors(self, volume): |
1517 | """Add a inotify watch to volume's ancestors if it's an UDF.""" |
1518 | |
1519 | === modified file 'ubuntuone/platform/windows/os_helper.py' |
1520 | --- ubuntuone/platform/windows/os_helper.py 2012-01-10 18:09:30 +0000 |
1521 | +++ ubuntuone/platform/windows/os_helper.py 2012-02-03 11:33:18 +0000 |
1522 | @@ -707,7 +707,11 @@ |
1523 | @windowspath() |
1524 | def listdir(directory): |
1525 | """List a directory.""" |
1526 | - |
1527 | + return map(_unicode_to_bytes, native_listdir(directory)) |
1528 | + |
1529 | + |
1530 | +def native_listdir(directory): |
1531 | + """List a directory.""" |
1532 | # The main reason why we have to append os.path.sep is the following: |
1533 | # |
1534 | # os.listdir implementation will append a unix path separator to the |
1535 | @@ -732,8 +736,8 @@ |
1536 | # return those paths that are system paths. Those paths are the ones that |
1537 | # we do not want to work with. |
1538 | |
1539 | - return map(_unicode_to_bytes, [p for p in os.listdir(directory) if not |
1540 | - native_is_system_path(os.path.join(directory, p))]) |
1541 | + return [p for p in os.listdir(directory) if not |
1542 | + native_is_system_path(os.path.join(directory, p))] |
1543 | |
1544 | |
1545 | @windowspath() |
Here's a mini-review. I don't have my dev environment setup and don't know a lot of the context, plus I can't run it yet...but here's a few things I noticed.
###### _notifications. py
test_filesystem
80 + self.mask = fs.FILE_ NOTIFY_ CHANGE_ FILE_NAME | \ NOTIFY_ CHANGE_ DIR_NAME | \
81 + fs.FILE_
Minor style thing, but I noticed in a big import (that was removed) that long lines were wrapped with parentheses instead of breaking with backslashes. Maybe do the same here? (Sorry, had to get my first nitpick cosmetic comment out of the way :)
======= ======= ======= ====== notifications. py
filesystem_
_get_partial_ child_path_ dir
It's about 2-3x faster to take a slice of the last character and compare it rather than using path.endswith. This may not be as clean as using endswith, but figured it may be worth mentioning. Something like "if not path[-1] == os.path.sep:"
Other than that, this method looks good. warn('Adding watch to none existing path "%s"', path)
******
+ self.log.
none existing > nonexistent watch(self, path):
******
+ def add_child_
This new method looks fine to me.
******
+ """Add a new path to be watch.
watch > watched
============
os_helper.py
native_listdir
What is the purpose of having listdir and native_listdir if they are the same?