Merge lp:~mandel/ubuntuone-client/add-virtual-watches into lp:ubuntuone-client

Proposed by Manuel de la Peña
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
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 ReadDirecotryChangesW and later processes them, that way we can have tests and assert if the correct number of events was raised and that we filtered the expected ones.
* 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 ReadDirecotryChangesW and later processes them, that way we can have tests and assert if the correct number of events was raised and that we filtered the expected ones.
* Added a new filter that ensures that IN_MODIFY|IS_DIR events are not raised when the child is not watched.

To post a comment you must log in.
Revision history for this message
Brian Curtin (brian.curtin) 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_notifications.py

80 + self.mask = fs.FILE_NOTIFY_CHANGE_FILE_NAME | \
81 + fs.FILE_NOTIFY_CHANGE_DIR_NAME | \

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 :)

===========================
filesystem_notifications.py

_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.
******
+ self.log.warn('Adding watch to none existing path "%s"', path)

none existing > nonexistent
******
+ def add_child_watch(self, path):

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?

Revision history for this message
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_notifications.py
>
> 80 + self.mask = fs.FILE_NOTIFY_CHANGE_FILE_NAME | \
> 81 + fs.FILE_NOTIFY_CHANGE_DIR_NAME | \
>
> 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_notifications.py
>
> _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:"
>

On it! good catch!

> Other than that, this method looks good.
> ******
> + self.log.warn('Adding watch to none existing path "%s"', path)
>
> none existing > nonexistent

Fixing

> ******
> + def add_child_watch(self, path):
>
> 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.

Revision history for this message
Diego Sarmentero (diegosarmentero) wrote :

+1
All green!

review: Approve
Revision history for this message
Alejandro J. Cura (alecu) wrote :

The branch looks interesting.

A few comments:
----
941 + if any([file_name.startswith(path) and not watched
942 + for path, watched in self._subdirs.iteritems()]):

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.mementowatch_factory(all_events_count, *args, **kwargs))

and:

   def mementowatch_factory(self, all_events_count, *args, **kwargs):
       self.memento_watch = 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.

Revision history for this message
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.

review: Needs Fixing
Revision history for this message
Alejandro J. Cura (alecu) wrote :

Line 132 seems to be missing a leading space:

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

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

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
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()

Subscribers

People subscribed via source and target branches