Status: | Merged | ||||||||
---|---|---|---|---|---|---|---|---|---|
Approved by: | Raoul Snyman | ||||||||
Approved revision: | 2817 | ||||||||
Merged at revision: | 2810 | ||||||||
Proposed branch: | lp:~phill-ridout/openlp/pathlib12 | ||||||||
Merge into: | lp:openlp | ||||||||
Diff against target: |
682 lines (+181/-291) 6 files modified
openlp/core/lib/serviceitem.py (+9/-8) openlp/core/ui/mainwindow.py (+8/-6) openlp/core/ui/servicemanager.py (+128/-241) tests/functional/openlp_core/lib/test_serviceitem.py (+2/-1) tests/functional/openlp_core/ui/test_mainwindow.py (+29/-2) tests/functional/openlp_core/ui/test_servicemanager.py (+5/-33) |
||||||||
To merge this branch: | bzr merge lp:~phill-ridout/openlp/pathlib12 | ||||||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Tim Bentley | Approve | ||
Raoul Snyman | Approve | ||
Review via email: mp+336568@code.launchpad.net |
This proposal supersedes a proposal from 2018-01-21.
Commit message
Description of the change
Started work on storing path objects in service file.
Refactored save code and reduced duplication.
Fixed + improved the loading / saving progress bars
improved performance
loading powerpoint from a service still does work
-------
lp:~phill-ridout/openlp/pathlib12 (revision 2817)
https:/
[RUNNING]
[SUCCESS]
https:/
[RUNNING]
[SUCCESS]
https:/
[SUCCESS]
https:/
[SUCCESS]
https:/
[RUNNING]
[SUCCESS]
https:/
[SUCCESS]
https:/
[SUCCESS]
https:/
[RUNNING]
[FAILURE]
Stopping after failure
Failed builds:
- Branch-
Process finished with exit code 0
Tim Bentley (trb143) wrote : Posted in a previous version of this proposal | # |
Raoul Snyman (raoul-snyman) wrote : | # |
Looks fine to me. Not sure why you're removing a test, but I'll go with it as I'm sure there's a valid reason.
Preview Diff
1 | === modified file 'openlp/core/lib/serviceitem.py' |
2 | --- openlp/core/lib/serviceitem.py 2017-12-29 09:15:48 +0000 |
3 | +++ openlp/core/lib/serviceitem.py 2018-01-24 18:53:18 +0000 |
4 | @@ -36,6 +36,7 @@ |
5 | from openlp.core.common.applocation import AppLocation |
6 | from openlp.core.common.i18n import translate |
7 | from openlp.core.common.mixins import RegistryProperties |
8 | +from openlp.core.common.path import Path |
9 | from openlp.core.common.settings import Settings |
10 | from openlp.core.lib import ImageSource, build_icon, clean_tags, expand_tags, expand_chords |
11 | |
12 | @@ -427,13 +428,13 @@ |
13 | self.has_original_files = True |
14 | if 'background_audio' in header: |
15 | self.background_audio = [] |
16 | - for filename in header['background_audio']: |
17 | - # Give them real file paths. |
18 | - filepath = str(filename) |
19 | - if path: |
20 | + for file_path in header['background_audio']: |
21 | + # In OpenLP 3.0 we switched to storing Path objects in JSON files |
22 | + if isinstance(file_path, str): |
23 | + # Handle service files prior to OpenLP 3.0 |
24 | # Windows can handle both forward and backward slashes, so we use ntpath to get the basename |
25 | - filepath = os.path.join(path, ntpath.basename(str(filename))) |
26 | - self.background_audio.append(filepath) |
27 | + file_path = Path(path, ntpath.basename(file_path)) |
28 | + self.background_audio.append(file_path) |
29 | self.theme_overwritten = header.get('theme_overwritten', False) |
30 | if self.service_item_type == ServiceItemType.Text: |
31 | for slide in service_item['serviceitem']['data']: |
32 | @@ -444,8 +445,8 @@ |
33 | if path: |
34 | self.has_original_files = False |
35 | for text_image in service_item['serviceitem']['data']: |
36 | - filename = os.path.join(path, text_image) |
37 | - self.add_from_image(filename, text_image, background) |
38 | + file_path = os.path.join(path, text_image) |
39 | + self.add_from_image(file_path, text_image, background) |
40 | else: |
41 | for text_image in service_item['serviceitem']['data']: |
42 | self.add_from_image(text_image['path'], text_image['title'], background) |
43 | |
44 | === modified file 'openlp/core/ui/mainwindow.py' |
45 | --- openlp/core/ui/mainwindow.py 2018-01-12 18:29:32 +0000 |
46 | +++ openlp/core/ui/mainwindow.py 2018-01-24 18:53:18 +0000 |
47 | @@ -1314,11 +1314,13 @@ |
48 | self.load_progress_bar.setValue(0) |
49 | self.application.process_events() |
50 | |
51 | - def increment_progress_bar(self): |
52 | - """ |
53 | - Increase the Progress Bar value by 1 |
54 | - """ |
55 | - self.load_progress_bar.setValue(self.load_progress_bar.value() + 1) |
56 | + def increment_progress_bar(self, increment=1): |
57 | + """ |
58 | + Increase the Progress Bar by the value in increment. |
59 | + |
60 | + :param int increment: The value you to increase the progress bar by. |
61 | + """ |
62 | + self.load_progress_bar.setValue(self.load_progress_bar.value() + increment) |
63 | self.application.process_events() |
64 | |
65 | def finished_progress_bar(self): |
66 | @@ -1386,4 +1388,4 @@ |
67 | if not isinstance(filename, str): |
68 | filename = str(filename, sys.getfilesystemencoding()) |
69 | if filename.endswith(('.osz', '.oszl')): |
70 | - self.service_manager_contents.load_file(filename) |
71 | + self.service_manager_contents.load_file(Path(filename)) |
72 | |
73 | === modified file 'openlp/core/ui/servicemanager.py' |
74 | --- openlp/core/ui/servicemanager.py 2017-12-29 09:15:48 +0000 |
75 | +++ openlp/core/ui/servicemanager.py 2018-01-24 18:53:18 +0000 |
76 | @@ -27,8 +27,9 @@ |
77 | import os |
78 | import shutil |
79 | import zipfile |
80 | +from contextlib import suppress |
81 | from datetime import datetime, timedelta |
82 | -from tempfile import mkstemp |
83 | +from tempfile import NamedTemporaryFile |
84 | |
85 | from PyQt5 import QtCore, QtGui, QtWidgets |
86 | |
87 | @@ -36,11 +37,13 @@ |
88 | from openlp.core.common.actions import ActionList, CategoryOrder |
89 | from openlp.core.common.applocation import AppLocation |
90 | from openlp.core.common.i18n import UiStrings, format_time, translate |
91 | +from openlp.core.common.json import OpenLPJsonDecoder, OpenLPJsonEncoder |
92 | from openlp.core.common.mixins import LogMixin, RegistryProperties |
93 | -from openlp.core.common.path import Path, create_paths, str_to_path |
94 | +from openlp.core.common.path import Path, str_to_path |
95 | from openlp.core.common.registry import Registry, RegistryBase |
96 | from openlp.core.common.settings import Settings |
97 | from openlp.core.lib import ServiceItem, ItemCapabilities, PluginStatus, build_icon |
98 | +from openlp.core.lib.exceptions import ValidationError |
99 | from openlp.core.lib.ui import critical_error_message_box, create_widget_action, find_and_set_in_combo_box |
100 | from openlp.core.ui import ServiceNoteForm, ServiceItemEditForm, StartTimeForm |
101 | from openlp.core.widgets.dialogs import FileDialog |
102 | @@ -449,7 +452,7 @@ |
103 | else: |
104 | file_path = str_to_path(load_file) |
105 | Settings().setValue(self.main_window.service_manager_settings_section + '/last directory', file_path.parent) |
106 | - self.load_file(str(file_path)) |
107 | + self.load_file(file_path) |
108 | |
109 | def save_modified_service(self): |
110 | """ |
111 | @@ -475,7 +478,7 @@ |
112 | elif result == QtWidgets.QMessageBox.Save: |
113 | self.decide_save_method() |
114 | sender = self.sender() |
115 | - self.load_file(sender.data()) |
116 | + self.load_file(Path(sender.data())) |
117 | |
118 | def new_file(self): |
119 | """ |
120 | @@ -503,7 +506,32 @@ |
121 | service.append({'openlp_core': core}) |
122 | return service |
123 | |
124 | - def save_file(self, field=None): |
125 | + def get_write_file_list(self): |
126 | + """ |
127 | + Get a list of files used in the service and files that are missing. |
128 | + |
129 | + :return: A list of files used in the service that exist, and a list of files that don't. |
130 | + :rtype: (list[openlp.core.common.path.Path], list[openlp.core.common.path.Path]) |
131 | + """ |
132 | + write_list = [] |
133 | + missing_list = [] |
134 | + for item in self.service_items: |
135 | + if item['service_item'].uses_file(): |
136 | + for frame in item['service_item'].get_frames(): |
137 | + path_from = item['service_item'].get_frame_path(frame=frame) |
138 | + if path_from in write_list or path_from in missing_list: |
139 | + continue |
140 | + if not os.path.exists(path_from): |
141 | + missing_list.append(Path(path_from)) |
142 | + else: |
143 | + write_list.append(Path(path_from)) |
144 | + for audio_path in item['service_item'].background_audio: |
145 | + if audio_path in write_list: |
146 | + continue |
147 | + write_list.append(audio_path) |
148 | + return write_list, missing_list |
149 | + |
150 | + def save_file(self): |
151 | """ |
152 | Save the current service file. |
153 | |
154 | @@ -511,178 +539,74 @@ |
155 | there be an error when saving. Audio files are also copied into the service manager directory, and then packaged |
156 | into the zip file. |
157 | """ |
158 | - if not self.file_name(): |
159 | - return self.save_file_as() |
160 | - temp_file, temp_file_name = mkstemp('.osz', 'openlp_') |
161 | - # We don't need the file handle. |
162 | - os.close(temp_file) |
163 | - self.log_debug(temp_file_name) |
164 | - path_file_name = str(self.file_name()) |
165 | - path, file_name = os.path.split(path_file_name) |
166 | - base_name = os.path.splitext(file_name)[0] |
167 | - service_file_name = '{name}.osj'.format(name=base_name) |
168 | - self.log_debug('ServiceManager.save_file - {name}'.format(name=path_file_name)) |
169 | - Settings().setValue(self.main_window.service_manager_settings_section + '/last directory', Path(path)) |
170 | + file_path = self.file_name() |
171 | + self.log_debug('ServiceManager.save_file - {name}'.format(name=file_path)) |
172 | + self.application.set_busy_cursor() |
173 | + |
174 | service = self.create_basic_service() |
175 | + |
176 | write_list = [] |
177 | missing_list = [] |
178 | - audio_files = [] |
179 | - total_size = 0 |
180 | - self.application.set_busy_cursor() |
181 | - # Number of items + 1 to zip it |
182 | - self.main_window.display_progress_bar(len(self.service_items) + 1) |
183 | - # Get list of missing files, and list of files to write |
184 | - for item in self.service_items: |
185 | - if not item['service_item'].uses_file(): |
186 | - continue |
187 | - for frame in item['service_item'].get_frames(): |
188 | - path_from = item['service_item'].get_frame_path(frame=frame) |
189 | - if path_from in write_list or path_from in missing_list: |
190 | - continue |
191 | - if not os.path.exists(path_from): |
192 | - missing_list.append(path_from) |
193 | - else: |
194 | - write_list.append(path_from) |
195 | - if missing_list: |
196 | - self.application.set_normal_cursor() |
197 | - title = translate('OpenLP.ServiceManager', 'Service File(s) Missing') |
198 | - message = translate('OpenLP.ServiceManager', |
199 | - 'The following file(s) in the service are missing: {name}\n\n' |
200 | - 'These files will be removed if you continue to save.' |
201 | - ).format(name="\n\t".join(missing_list)) |
202 | - answer = QtWidgets.QMessageBox.critical(self, title, message, |
203 | - QtWidgets.QMessageBox.StandardButtons(QtWidgets.QMessageBox.Ok | |
204 | - QtWidgets.QMessageBox.Cancel)) |
205 | - if answer == QtWidgets.QMessageBox.Cancel: |
206 | - self.main_window.finished_progress_bar() |
207 | - return False |
208 | + |
209 | + if not self._save_lite: |
210 | + write_list, missing_list = self.get_write_file_list() |
211 | + |
212 | + if missing_list: |
213 | + self.application.set_normal_cursor() |
214 | + title = translate('OpenLP.ServiceManager', 'Service File(s) Missing') |
215 | + message = translate('OpenLP.ServiceManager', |
216 | + 'The following file(s) in the service are missing: {name}\n\n' |
217 | + 'These files will be removed if you continue to save.' |
218 | + ).format(name='\n\t'.join(missing_list)) |
219 | + answer = QtWidgets.QMessageBox.critical(self, title, message, |
220 | + QtWidgets.QMessageBox.StandardButtons( |
221 | + QtWidgets.QMessageBox.Ok | QtWidgets.QMessageBox.Cancel)) |
222 | + if answer == QtWidgets.QMessageBox.Cancel: |
223 | + return False |
224 | # Check if item contains a missing file. |
225 | for item in list(self.service_items): |
226 | - self.main_window.increment_progress_bar() |
227 | - item['service_item'].remove_invalid_frames(missing_list) |
228 | - if item['service_item'].missing_frames(): |
229 | - self.service_items.remove(item) |
230 | - else: |
231 | - service_item = item['service_item'].get_service_repr(self._save_lite) |
232 | - if service_item['header']['background_audio']: |
233 | - for i, file_name in enumerate(service_item['header']['background_audio']): |
234 | - new_file = os.path.join('audio', item['service_item'].unique_identifier, str(file_name)) |
235 | - audio_files.append((file_name, new_file)) |
236 | - service_item['header']['background_audio'][i] = new_file |
237 | - # Add the service item to the service. |
238 | - service.append({'serviceitem': service_item}) |
239 | + if not self._save_lite: |
240 | + item['service_item'].remove_invalid_frames(missing_list) |
241 | + if item['service_item'].missing_frames(): |
242 | + self.service_items.remove(item) |
243 | + continue |
244 | + service_item = item['service_item'].get_service_repr(self._save_lite) |
245 | + # Add the service item to the service. |
246 | + service.append({'serviceitem': service_item}) |
247 | self.repaint_service_list(-1, -1) |
248 | + service_content = json.dumps(service, cls=OpenLPJsonEncoder) |
249 | + service_content_size = len(bytes(service_content, encoding='utf-8')) |
250 | + total_size = service_content_size |
251 | for file_item in write_list: |
252 | - file_size = os.path.getsize(file_item) |
253 | - total_size += file_size |
254 | + total_size += file_item.stat().st_size |
255 | self.log_debug('ServiceManager.save_file - ZIP contents size is %i bytes' % total_size) |
256 | - service_content = json.dumps(service) |
257 | - # Usual Zip file cannot exceed 2GiB, file with Zip64 cannot be extracted using unzip in UNIX. |
258 | - allow_zip_64 = (total_size > 2147483648 + len(service_content)) |
259 | - self.log_debug('ServiceManager.save_file - allowZip64 is {text}'.format(text=allow_zip_64)) |
260 | - zip_file = None |
261 | - success = True |
262 | - self.main_window.increment_progress_bar() |
263 | + self.main_window.display_progress_bar(total_size) |
264 | try: |
265 | - zip_file = zipfile.ZipFile(temp_file_name, 'w', zipfile.ZIP_STORED, allow_zip_64) |
266 | - # First we add service contents.. |
267 | - zip_file.writestr(service_file_name, service_content) |
268 | - # Finally add all the listed media files. |
269 | - for write_from in write_list: |
270 | - zip_file.write(write_from, write_from) |
271 | - for audio_from, audio_to in audio_files: |
272 | - audio_from = str(audio_from) |
273 | - audio_to = str(audio_to) |
274 | - if audio_from.startswith('audio'): |
275 | - # When items are saved, they get new unique_identifier. Let's copy the file to the new location. |
276 | - # Unused files can be ignored, OpenLP automatically cleans up the service manager dir on exit. |
277 | - audio_from = os.path.join(self.service_path, audio_from) |
278 | - save_file = os.path.join(self.service_path, audio_to) |
279 | - save_path = os.path.split(save_file)[0] |
280 | - create_paths(Path(save_path)) |
281 | - if not os.path.exists(save_file): |
282 | - shutil.copy(audio_from, save_file) |
283 | - zip_file.write(audio_from, audio_to) |
284 | - except OSError: |
285 | - self.log_exception('Failed to save service to disk: {name}'.format(name=temp_file_name)) |
286 | - self.main_window.error_message(translate('OpenLP.ServiceManager', 'Error Saving File'), |
287 | - translate('OpenLP.ServiceManager', 'There was an error saving your file.')) |
288 | - success = False |
289 | - finally: |
290 | - if zip_file: |
291 | - zip_file.close() |
292 | - self.main_window.finished_progress_bar() |
293 | - self.application.set_normal_cursor() |
294 | - if success: |
295 | - try: |
296 | - shutil.copy(temp_file_name, path_file_name) |
297 | - except (shutil.Error, PermissionError): |
298 | - return self.save_file_as() |
299 | - except OSError as ose: |
300 | - QtWidgets.QMessageBox.critical(self, translate('OpenLP.ServiceManager', 'Error Saving File'), |
301 | - translate('OpenLP.ServiceManager', 'An error occurred while writing the ' |
302 | - 'service file: {error}').format(error=ose.strerror), |
303 | - QtWidgets.QMessageBox.StandardButtons(QtWidgets.QMessageBox.Ok)) |
304 | - success = False |
305 | - self.main_window.add_recent_file(path_file_name) |
306 | - self.set_modified(False) |
307 | - delete_file(Path(temp_file_name)) |
308 | - return success |
309 | - |
310 | - def save_local_file(self): |
311 | - """ |
312 | - Save the current service file but leave all the file references alone to point to the current machine. |
313 | - This format is not transportable as it will not contain any files. |
314 | - """ |
315 | - if not self.file_name(): |
316 | + with NamedTemporaryFile(dir=str(file_path.parent), prefix='.') as temp_file, \ |
317 | + zipfile.ZipFile(temp_file, 'w') as zip_file: |
318 | + # First we add service contents.. |
319 | + zip_file.writestr('service_data.osj', service_content) |
320 | + self.main_window.increment_progress_bar(service_content_size) |
321 | + # Finally add all the listed media files. |
322 | + for write_path in write_list: |
323 | + zip_file.write(str(write_path), str(write_path)) |
324 | + self.main_window.increment_progress_bar(write_path.stat().st_size) |
325 | + with suppress(FileNotFoundError): |
326 | + file_path.unlink() |
327 | + os.link(temp_file.name, str(file_path)) |
328 | + Settings().setValue(self.main_window.service_manager_settings_section + '/last directory', file_path.parent) |
329 | + except (PermissionError, OSError) as error: |
330 | + self.log_exception('Failed to save service to disk: {name}'.format(name=file_path)) |
331 | + self.main_window.error_message( |
332 | + translate('OpenLP.ServiceManager', 'Error Saving File'), |
333 | + translate('OpenLP.ServiceManager', |
334 | + 'There was an error saving your file.\n\n{error}').format(error=error)) |
335 | return self.save_file_as() |
336 | - temp_file, temp_file_name = mkstemp('.oszl', 'openlp_') |
337 | - # We don't need the file handle. |
338 | - os.close(temp_file) |
339 | - self.log_debug(temp_file_name) |
340 | - path_file_name = str(self.file_name()) |
341 | - path, file_name = os.path.split(path_file_name) |
342 | - base_name = os.path.splitext(file_name)[0] |
343 | - service_file_name = '{name}.osj'.format(name=base_name) |
344 | - self.log_debug('ServiceManager.save_file - {name}'.format(name=path_file_name)) |
345 | - Settings().setValue(self.main_window.service_manager_settings_section + '/last directory', Path(path)) |
346 | - service = self.create_basic_service() |
347 | - self.application.set_busy_cursor() |
348 | - # Number of items + 1 to zip it |
349 | - self.main_window.display_progress_bar(len(self.service_items) + 1) |
350 | - for item in self.service_items: |
351 | - self.main_window.increment_progress_bar() |
352 | - service_item = item['service_item'].get_service_repr(self._save_lite) |
353 | - # TODO: check for file item on save. |
354 | - service.append({'serviceitem': service_item}) |
355 | - self.main_window.increment_progress_bar() |
356 | - service_content = json.dumps(service) |
357 | - zip_file = None |
358 | - success = True |
359 | - self.main_window.increment_progress_bar() |
360 | - try: |
361 | - zip_file = zipfile.ZipFile(temp_file_name, 'w', zipfile.ZIP_STORED, True) |
362 | - # First we add service contents. |
363 | - zip_file.writestr(service_file_name, service_content) |
364 | - except OSError: |
365 | - self.log_exception('Failed to save service to disk: {name}'.format(name=temp_file_name)) |
366 | - self.main_window.error_message(translate('OpenLP.ServiceManager', 'Error Saving File'), |
367 | - translate('OpenLP.ServiceManager', 'There was an error saving your file.')) |
368 | - success = False |
369 | - finally: |
370 | - if zip_file: |
371 | - zip_file.close() |
372 | self.main_window.finished_progress_bar() |
373 | self.application.set_normal_cursor() |
374 | - if success: |
375 | - try: |
376 | - shutil.copy(temp_file_name, path_file_name) |
377 | - except (shutil.Error, PermissionError): |
378 | - return self.save_file_as() |
379 | - self.main_window.add_recent_file(path_file_name) |
380 | - self.set_modified(False) |
381 | - delete_file(Path(temp_file_name)) |
382 | - return success |
383 | + self.main_window.add_recent_file(file_path) |
384 | + self.set_modified(False) |
385 | + return True |
386 | |
387 | def save_file_as(self, field=None): |
388 | """ |
389 | @@ -743,87 +667,49 @@ |
390 | """ |
391 | if not self.file_name(): |
392 | return self.save_file_as() |
393 | - if self._save_lite: |
394 | - return self.save_local_file() |
395 | - else: |
396 | - return self.save_file() |
397 | + return self.save_file() |
398 | |
399 | - def load_file(self, file_name): |
400 | + def load_file(self, file_path): |
401 | """ |
402 | Load an existing service file |
403 | - :param file_name: |
404 | + :param file_path: |
405 | """ |
406 | - if not file_name: |
407 | - return False |
408 | - file_name = str(file_name) |
409 | - if not os.path.exists(file_name): |
410 | - return False |
411 | - zip_file = None |
412 | - file_to = None |
413 | + if not file_path.exists(): |
414 | + return False |
415 | + service_data = None |
416 | self.application.set_busy_cursor() |
417 | try: |
418 | - zip_file = zipfile.ZipFile(file_name) |
419 | - for zip_info in zip_file.infolist(): |
420 | - try: |
421 | - ucs_file = zip_info.filename |
422 | - except UnicodeDecodeError: |
423 | - self.log_exception('file_name "{name}" is not valid UTF-8'.format(name=zip_info.file_name)) |
424 | - critical_error_message_box(message=translate('OpenLP.ServiceManager', |
425 | - 'File is not a valid service.\n The content encoding is not UTF-8.')) |
426 | - continue |
427 | - os_file = ucs_file.replace('/', os.path.sep) |
428 | - os_file = os.path.basename(os_file) |
429 | - self.log_debug('Extract file: {name}'.format(name=os_file)) |
430 | - zip_info.filename = os_file |
431 | - zip_file.extract(zip_info, self.service_path) |
432 | - if os_file.endswith('osj') or os_file.endswith('osd'): |
433 | - p_file = os.path.join(self.service_path, os_file) |
434 | - if 'p_file' in locals(): |
435 | - file_to = open(p_file, 'r') |
436 | - if p_file.endswith('osj'): |
437 | - items = json.load(file_to) |
438 | - else: |
439 | - critical_error_message_box(message=translate('OpenLP.ServiceManager', |
440 | - 'The service file you are trying to open is in an old ' |
441 | - 'format.\n Please save it using OpenLP 2.0.2 or ' |
442 | - 'greater.')) |
443 | - return |
444 | - file_to.close() |
445 | + with zipfile.ZipFile(str(file_path)) as zip_file: |
446 | + compressed_size = 0 |
447 | + for zip_info in zip_file.infolist(): |
448 | + compressed_size += zip_info.compress_size |
449 | + self.main_window.display_progress_bar(compressed_size) |
450 | + for zip_info in zip_file.infolist(): |
451 | + self.log_debug('Extract file: {name}'.format(name=zip_info.filename)) |
452 | + # The json file has been called 'service_data.osj' since OpenLP 3.0 |
453 | + if zip_info.filename == 'service_data.osj' or zip_info.filename.endswith('osj'): |
454 | + with zip_file.open(zip_info, 'r') as json_file: |
455 | + service_data = json_file.read() |
456 | + else: |
457 | + zip_info.filename = os.path.basename(zip_info.filename) |
458 | + zip_file.extract(zip_info, str(self.service_path)) |
459 | + self.main_window.increment_progress_bar(zip_info.compress_size) |
460 | + if service_data: |
461 | + items = json.loads(service_data, cls=OpenLPJsonDecoder) |
462 | self.new_file() |
463 | - self.set_file_name(str_to_path(file_name)) |
464 | - self.main_window.display_progress_bar(len(items)) |
465 | self.process_service_items(items) |
466 | - delete_file(Path(p_file)) |
467 | - self.main_window.add_recent_file(file_name) |
468 | + self.set_file_name(file_path) |
469 | + self.main_window.add_recent_file(file_path) |
470 | self.set_modified(False) |
471 | - Settings().setValue('servicemanager/last file', Path(file_name)) |
472 | - else: |
473 | - critical_error_message_box(message=translate('OpenLP.ServiceManager', 'File is not a valid service.')) |
474 | - self.log_error('File contains no service data') |
475 | - except (OSError, NameError): |
476 | - self.log_exception('Problem loading service file {name}'.format(name=file_name)) |
477 | - critical_error_message_box(message=translate('OpenLP.ServiceManager', |
478 | - 'File could not be opened because it is corrupt.')) |
479 | - except zipfile.BadZipFile: |
480 | - if os.path.getsize(file_name) == 0: |
481 | - self.log_exception('Service file is zero sized: {name}'.format(name=file_name)) |
482 | - QtWidgets.QMessageBox.information(self, translate('OpenLP.ServiceManager', 'Empty File'), |
483 | - translate('OpenLP.ServiceManager', |
484 | - 'This service file does not contain ' |
485 | - 'any data.')) |
486 | - else: |
487 | - self.log_exception('Service file is cannot be extracted as zip: {name}'.format(name=file_name)) |
488 | - QtWidgets.QMessageBox.information(self, translate('OpenLP.ServiceManager', 'Corrupt File'), |
489 | - translate('OpenLP.ServiceManager', |
490 | - 'This file is either corrupt or it is not an OpenLP 2 ' |
491 | - 'service file.')) |
492 | - self.application.set_normal_cursor() |
493 | - return |
494 | - finally: |
495 | - if file_to: |
496 | - file_to.close() |
497 | - if zip_file: |
498 | - zip_file.close() |
499 | + Settings().setValue('servicemanager/last file', file_path) |
500 | + else: |
501 | + raise ValidationError(msg='No service data found') |
502 | + except (NameError, OSError, ValidationError, zipfile.BadZipFile) as e: |
503 | + self.log_exception('Problem loading service file {name}'.format(name=file_path)) |
504 | + critical_error_message_box( |
505 | + message=translate('OpenLP.ServiceManager', |
506 | + 'The service file {file_path} could not be loaded because it is either corrupt, or ' |
507 | + 'not a valid OpenLP 2 or OpenLP 3 service file.'.format(file_path=file_path))) |
508 | self.main_window.finished_progress_bar() |
509 | self.application.set_normal_cursor() |
510 | self.repaint_service_list(-1, -1) |
511 | @@ -838,7 +724,8 @@ |
512 | self.main_window.increment_progress_bar() |
513 | service_item = ServiceItem() |
514 | if 'openlp_core' in item: |
515 | - item = item.get('openlp_core') |
516 | + item = item['openlp_core'] |
517 | + self._save_lite = item.get('lite-service', False) |
518 | theme = item.get('service-theme', None) |
519 | if theme: |
520 | find_and_set_in_combo_box(self.theme_combo_box, theme, set_missing=False) |
521 | @@ -861,9 +748,9 @@ |
522 | Load the last service item from the service manager when the service was last closed. Can be blank if there was |
523 | no service present. |
524 | """ |
525 | - file_name = str_to_path(Settings().value('servicemanager/last file')) |
526 | - if file_name: |
527 | - self.load_file(file_name) |
528 | + file_path = Settings().value('servicemanager/last file') |
529 | + if file_path: |
530 | + self.load_file(file_path) |
531 | |
532 | def context_menu(self, point): |
533 | """ |
534 | |
535 | === modified file 'tests/functional/openlp_core/lib/test_serviceitem.py' |
536 | --- tests/functional/openlp_core/lib/test_serviceitem.py 2017-12-28 08:22:55 +0000 |
537 | +++ tests/functional/openlp_core/lib/test_serviceitem.py 2018-01-24 18:53:18 +0000 |
538 | @@ -27,6 +27,7 @@ |
539 | from unittest.mock import MagicMock, patch |
540 | |
541 | from openlp.core.common import md5_hash |
542 | +from openlp.core.common.path import Path |
543 | from openlp.core.common.registry import Registry |
544 | from openlp.core.common.settings import Settings |
545 | from openlp.core.lib import ItemCapabilities, ServiceItem, ServiceItemType, FormattingTags |
546 | @@ -351,5 +352,5 @@ |
547 | '"Amazing Grace! how sweet the s" has been returned as the title' |
548 | assert '’Twas grace that taught my hea' == service_item.get_frame_title(1), \ |
549 | '"’Twas grace that taught my hea" has been returned as the title' |
550 | - assert '/test/amazing_grace.mp3' == service_item.background_audio[0], \ |
551 | + assert Path('/test/amazing_grace.mp3') == service_item.background_audio[0], \ |
552 | '"/test/amazing_grace.mp3" should be in the background_audio list' |
553 | |
554 | === modified file 'tests/functional/openlp_core/ui/test_mainwindow.py' |
555 | --- tests/functional/openlp_core/ui/test_mainwindow.py 2018-01-07 05:24:55 +0000 |
556 | +++ tests/functional/openlp_core/ui/test_mainwindow.py 2018-01-24 18:53:18 +0000 |
557 | @@ -23,6 +23,7 @@ |
558 | Package to test openlp.core.ui.mainwindow package. |
559 | """ |
560 | import os |
561 | +from pathlib import Path |
562 | from unittest import TestCase |
563 | from unittest.mock import MagicMock, patch |
564 | |
565 | @@ -84,14 +85,13 @@ |
566 | """ |
567 | # GIVEN a service as an argument to openlp |
568 | service = os.path.join(TEST_RESOURCES_PATH, 'service', 'test.osz') |
569 | - self.main_window.arguments = [service] |
570 | |
571 | # WHEN the argument is processed |
572 | with patch.object(self.main_window.service_manager, 'load_file') as mocked_load_file: |
573 | self.main_window.open_cmd_line_files(service) |
574 | |
575 | # THEN the service from the arguments is loaded |
576 | - mocked_load_file.assert_called_with(service) |
577 | + mocked_load_file.assert_called_with(Path(service)) |
578 | |
579 | @patch('openlp.core.ui.servicemanager.ServiceManager.load_file') |
580 | def test_cmd_line_arg(self, mocked_load_file): |
581 | @@ -242,3 +242,30 @@ |
582 | |
583 | # THEN: projector_manager_dock.setVisible should had been called once |
584 | mocked_dock.setVisible.assert_called_once_with(False) |
585 | + |
586 | + def test_increment_progress_bar_default_increment(self): |
587 | + """ |
588 | + Test that increment_progress_bar increments the progress bar by 1 when called without the `increment` arg. |
589 | + """ |
590 | + # GIVEN: A mocked progress bar |
591 | + with patch.object(self.main_window, 'load_progress_bar', **{'value.return_value': 0}) as mocked_progress_bar: |
592 | + |
593 | + # WHEN: Calling increment_progress_bar without the `increment` arg |
594 | + self.main_window.increment_progress_bar() |
595 | + |
596 | + # THEN: The progress bar value should have been incremented by 1 |
597 | + mocked_progress_bar.setValue.assert_called_once_with(1) |
598 | + |
599 | + def test_increment_progress_bar_custom_increment(self): |
600 | + """ |
601 | + Test that increment_progress_bar increments the progress bar by the `increment` arg when called with the |
602 | + `increment` arg with a set value. |
603 | + """ |
604 | + # GIVEN: A mocked progress bar |
605 | + with patch.object(self.main_window, 'load_progress_bar', **{'value.return_value': 0}) as mocked_progress_bar: |
606 | + |
607 | + # WHEN: Calling increment_progress_bar with `increment` set to 10 |
608 | + self.main_window.increment_progress_bar(increment=10) |
609 | + |
610 | + # THEN: The progress bar value should have been incremented by 10 |
611 | + mocked_progress_bar.setValue.assert_called_once_with(10) |
612 | |
613 | === modified file 'tests/functional/openlp_core/ui/test_servicemanager.py' |
614 | --- tests/functional/openlp_core/ui/test_servicemanager.py 2017-12-20 20:38:43 +0000 |
615 | +++ tests/functional/openlp_core/ui/test_servicemanager.py 2018-01-24 18:53:18 +0000 |
616 | @@ -623,10 +623,10 @@ |
617 | # THEN: make_preview() should not have been called |
618 | assert mocked_make_preview.call_count == 0, 'ServiceManager.make_preview() should not be called' |
619 | |
620 | - @patch('openlp.core.ui.servicemanager.shutil.copy') |
621 | @patch('openlp.core.ui.servicemanager.zipfile') |
622 | @patch('openlp.core.ui.servicemanager.ServiceManager.save_file_as') |
623 | - def test_save_file_raises_permission_error(self, mocked_save_file_as, mocked_zipfile, mocked_shutil_copy): |
624 | + @patch('openlp.core.ui.servicemanager.os') |
625 | + def test_save_file_raises_permission_error(self, mocked_os, mocked_save_file_as, mocked_zipfile): |
626 | """ |
627 | Test that when a PermissionError is raised when trying to save a file, it is handled correctly |
628 | """ |
629 | @@ -636,50 +636,22 @@ |
630 | Registry().register('main_window', mocked_main_window) |
631 | Registry().register('application', MagicMock()) |
632 | service_manager = ServiceManager(None) |
633 | - service_manager._service_path = os.path.join('temp', 'filename.osz') |
634 | + service_manager._service_path = MagicMock() |
635 | service_manager._save_lite = False |
636 | service_manager.service_items = [] |
637 | service_manager.service_theme = 'Default' |
638 | service_manager.service_manager_list = MagicMock() |
639 | mocked_save_file_as.return_value = True |
640 | mocked_zipfile.ZipFile.return_value = MagicMock() |
641 | - mocked_shutil_copy.side_effect = PermissionError |
642 | + mocked_os.link.side_effect = PermissionError |
643 | |
644 | - # WHEN: The service is saved and a PermissionError is thrown |
645 | + # WHEN: The service is saved and a PermissionError is raised |
646 | result = service_manager.save_file() |
647 | |
648 | # THEN: The "save_as" method is called to save the service |
649 | assert result is True |
650 | mocked_save_file_as.assert_called_with() |
651 | |
652 | - @patch('openlp.core.ui.servicemanager.shutil.copy') |
653 | - @patch('openlp.core.ui.servicemanager.zipfile') |
654 | - @patch('openlp.core.ui.servicemanager.ServiceManager.save_file_as') |
655 | - def test_save_local_file_raises_permission_error(self, mocked_save_file_as, mocked_zipfile, mocked_shutil_copy): |
656 | - """ |
657 | - Test that when a PermissionError is raised when trying to save a local file, it is handled correctly |
658 | - """ |
659 | - # GIVEN: A service manager, a service to save |
660 | - mocked_main_window = MagicMock() |
661 | - mocked_main_window.service_manager_settings_section = 'servicemanager' |
662 | - Registry().register('main_window', mocked_main_window) |
663 | - Registry().register('application', MagicMock()) |
664 | - service_manager = ServiceManager(None) |
665 | - service_manager._service_path = os.path.join('temp', 'filename.osz') |
666 | - service_manager._save_lite = False |
667 | - service_manager.service_items = [] |
668 | - service_manager.service_theme = 'Default' |
669 | - mocked_save_file_as.return_value = True |
670 | - mocked_zipfile.ZipFile.return_value = MagicMock() |
671 | - mocked_shutil_copy.side_effect = PermissionError |
672 | - |
673 | - # WHEN: The service is saved and a PermissionError is thrown |
674 | - result = service_manager.save_local_file() |
675 | - |
676 | - # THEN: The "save_as" method is called to save the service |
677 | - assert result is True |
678 | - mocked_save_file_as.assert_called_with() |
679 | - |
680 | @patch('openlp.core.ui.servicemanager.ServiceManager.regenerate_service_items') |
681 | def test_theme_change_global(self, mocked_regenerate_service_items): |
682 | """ |
Tried to save a file and got this error
There was an error saving your file.
[Errno 18] Invalid cross-device link: '/tmp/tmpz25kj0a1' -> '/home/ tim/Projects/ OpenLP/ Service 2018-01-21 17-30.osz'