From 31d45716ca29df2809d98d6aed3e9a5eb64d20d9 Mon Sep 17 00:00:00 2001 From: "P. Christeas" Date: Tue, 13 Jul 2010 23:53:24 +0300 Subject: [PATCH 1/5] Document storage: fix opening/closing filestore descriptors At open, if a file is created, a new store_fname has to be generated. At close, the size of the file has to be updated at all write modes. bzr revid: p_christ@hol.gr-20100713205324-q6g2tvem2qem6ftg --- addons/document/document_storage.py | 83 +++++++++++++++++++---------- 1 file changed, 56 insertions(+), 27 deletions(-) diff --git a/addons/document/document_storage.py b/addons/document/document_storage.py index 4d7c7d10eda..d3e6921324f 100644 --- a/addons/document/document_storage.py +++ b/addons/document/document_storage.py @@ -98,10 +98,9 @@ class nodefd_file(nodes.node_descriptor): def __init__(self, parent, path, mode): nodes.node_descriptor.__init__(self, parent) self.__file = open(path, mode) - if mode in ('w', 'w+', 'r+'): - self._need_index = True - else: - self._need_index = False + if mode.endswith('b'): + mode = mode[:-1] + self.mode = mode for attr in ('closed', 'read', 'write', 'seek', 'tell'): setattr(self,attr, getattr(self.__file, attr)) @@ -110,8 +109,8 @@ class nodefd_file(nodes.node_descriptor): # TODO: locking in init, close() fname = self.__file.name self.__file.close() - - if self._need_index: + + if self.mode in ('w', 'w+', 'r+'): par = self._get_parent() cr = pooler.get_db(par.context.dbname).cursor() icont = '' @@ -133,14 +132,34 @@ class nodefd_file(nodes.node_descriptor): icont_u = '' try: - cr.execute('UPDATE ir_attachment SET index_content = %s, file_type = %s WHERE id = %s', - (icont_u, mime, par.file_id)) - par.content_length = filesize + fsize = os.stat(fname).st_size + cr.execute("UPDATE ir_attachment " \ + " SET index_content = %s, file_type = %s, " \ + " file_size = %s " \ + " WHERE id = %s", + (icont_u, mime, fsize, par.file_id)) + par.content_length = fsize par.content_type = mime cr.commit() cr.close() except Exception: - logging.getLogger('document.storage').debug('Cannot save file indexed content:', exc_info=True) + logging.getLogger('document.storage').warning('Cannot save file indexed content:', exc_info=True) + + elif self.mode in ('a', 'a+' ): + try: + par = self._get_parent() + cr = pooler.get_db(par.context.dbname).cursor() + fsize = os.stat(fname).st_size + cr.execute("UPDATE ir_attachment SET file_size = %s " \ + " WHERE id = %s", + (fsize, par.file_id)) + par.content_length = fsize + par.content_type = mime + cr.commit() + cr.close() + except Exception: + logging.getLogger('document.storage').warning('Cannot save file appended content:', exc_info=True) + class nodefd_db(StringIO, nodes.node_descriptor): @@ -336,6 +355,18 @@ class document_storage(osv.osv): ('path_uniq', 'UNIQUE(type,path)', "The storage path must be unique!") ] + def __get_random_fname(self, path): + flag = None + # This can be improved + if os.path.isdir(path): + for dirs in os.listdir(path): + if os.path.isdir(os.path.join(path, dirs)) and len(os.listdir(os.path.join(path, dirs))) < 4000: + flag = dirs + break + flag = flag or create_directory(path) + filename = random_name() + return os.path.join(flag, filename) + def get_data(self, cr, uid, id, file_node, context=None, fil_obj=None): """ retrieve the contents of some file_node having storage_id = id optionally, fil_obj could point to the browse object of the file @@ -364,10 +395,17 @@ class document_storage(osv.osv): if not ira.store_fname: # On a migrated db, some files may have the wrong storage type # try to fix their directory. - if ira.file_size: - self._doclog.warning( "ir.attachment #%d does not have a filename, but is at filestore, fix it!" % ira.id) - raise IOError(errno.ENOENT, 'No file can be located') - fpath = os.path.join(boo.path, ira.store_fname) + if mode in ('r','r+'): + if ira.file_size: + self._doclog.warning( "ir.attachment #%d does not have a filename, but is at filestore, fix it!" % ira.id) + raise IOError(errno.ENOENT, 'No file can be located') + else: + store_fname = self.__get_random_fname(boo.path) + cr.execute('UPDATE ir_attachment SET store_fname = %s WHERE id = %s', + (store_fname, ira.id)) + fpath = os.path.join(boo.path, store_fname) + else: + fpath = os.path.join(boo.path, ira.store_fname) return nodefd_file(file_node, path=fpath, mode=mode) elif boo.type == 'db': @@ -385,7 +423,7 @@ class document_storage(osv.osv): self._doclog.warning("ir.attachment #%d does not have a filename, trying the name." %ira.id) sfname = ira.name fpath = os.path.join(boo.path,ira.store_fname or ira.name) - if not os.path.exists(fpath): + if (not os.path.exists(fpath)) and mode in ('r','r+'): raise IOError("File not found: %s" % fpath) return nodefd_file(file_node, path=fpath, mode=mode) @@ -464,23 +502,14 @@ class document_storage(osv.osv): if boo.type == 'filestore': path = boo.path try: - flag = None - # This can be improved - if os.path.isdir(path): - for dirs in os.listdir(path): - if os.path.isdir(os.path.join(path, dirs)) and len(os.listdir(os.path.join(path, dirs))) < 4000: - flag = dirs - break - flag = flag or create_directory(path) - filename = random_name() - fname = os.path.join(path, flag, filename) + store_fname = self.__get_random_fname(path) + fname = os.path.join(path, store_fname) fp = file(fname, 'wb') fp.write(data) fp.close() self._doclog.debug( "Saved data to %s" % fname) filesize = len(data) # os.stat(fname).st_size - store_fname = os.path.join(flag, filename) - + # TODO Here, an old file would be left hanging. except Exception, e: From d3ae0b008347718cec73df4523da0b674e2ba830 Mon Sep 17 00:00:00 2001 From: "P. Christeas" Date: Tue, 13 Jul 2010 23:53:36 +0300 Subject: [PATCH 2/5] Document FTP: after nodefd.open_data(), cursor must commit Because open_data, at the create case, updates the store_fname. bzr revid: p_christ@hol.gr-20100713205336-jodj31v722x81grb --- addons/document_ftp/ftpserver/abstracted_fs.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/addons/document_ftp/ftpserver/abstracted_fs.py b/addons/document_ftp/ftpserver/abstracted_fs.py index ba135b23a01..5e7c8c1b31c 100644 --- a/addons/document_ftp/ftpserver/abstracted_fs.py +++ b/addons/document_ftp/ftpserver/abstracted_fs.py @@ -150,6 +150,7 @@ class abstracted_fs(object): raise OSError(1, 'Operation not permited.') ret = child.open_data(cr, mode) + cr.commit() return ret except EnvironmentError: raise @@ -159,7 +160,9 @@ class abstracted_fs(object): try: child = node.create_child(cr, objname, data=None) - return child.open_data(cr, mode) + ret = child.open_data(cr, mode) + cr.commit() + return ret except Exception,e: self._log.exception('Cannot create item %s at node %s', objname, repr(node)) raise OSError(1, 'Operation not permited.') @@ -171,6 +174,7 @@ class abstracted_fs(object): cr, node, rem = datacr try: res = node.open_data(cr, mode) + cr.commit() except TypeError, e: raise IOError(errno.EINVAL, "No data") return res From 766fd336d4f8f0e17a05741542bf0a6e440e06bc Mon Sep 17 00:00:00 2001 From: "P. Christeas" Date: Tue, 13 Jul 2010 23:53:59 +0300 Subject: [PATCH 3/5] Document: fix realstore with open/close, rename. Put the path computation of realstore in a helper function, and use that in all places where a file is opened/created/moved. bzr revid: p_christ@hol.gr-20100713205359-jn0bo6fcw9lwdyge --- addons/document/document.py | 5 +- addons/document/document_storage.py | 96 +++++++++++++++++------------ addons/document/nodes.py | 8 ++- 3 files changed, 65 insertions(+), 44 deletions(-) diff --git a/addons/document/document.py b/addons/document/document.py index ac3d7b9187d..46d0981cc69 100644 --- a/addons/document/document.py +++ b/addons/document/document.py @@ -157,7 +157,10 @@ class document_file(osv.osv): return False if not self._check_duplication(cr, uid, vals, ids, 'write'): raise osv.except_osv(_('ValidateError'), _('File name must be unique!')) - if ('parent_id' in vals) or ('name' in vals): + + # if nodes call this write(), they must skip the code below + from_node = context and context.get('__from_node', False) + if (('parent_id' in vals) or ('name' in vals)) and not from_node: # perhaps this file is renaming or changing directory nctx = nodes.get_node_context(cr,uid,context={}) dirobj = self.pool.get('document.directory') diff --git a/addons/document/document_storage.py b/addons/document/document_storage.py index d3e6921324f..149e779055d 100644 --- a/addons/document/document_storage.py +++ b/addons/document/document_storage.py @@ -154,7 +154,6 @@ class nodefd_file(nodes.node_descriptor): " WHERE id = %s", (fsize, par.file_id)) par.content_length = fsize - par.content_type = mime cr.commit() cr.close() except Exception: @@ -367,6 +366,37 @@ class document_storage(osv.osv): filename = random_name() return os.path.join(flag, filename) + def __prepare_realpath(self, cr, file_node, ira, store_path, do_create=True): + """ Cleanup path for realstore, create dirs if needed + + @param file_node the node + @param ira ir.attachment browse of the file_node + @param store_path the path of the parent storage object, list + @param do_create create the directories, if needed + + @return tuple(path "/var/filestore/real/dir/", npath ['dir','fname.ext'] ) + """ + file_node.fix_ppath(cr, ira) + npath = file_node.full_path() or [] + # npath may contain empty elements, for root directory etc. + npath = filter(lambda x: x is not None, npath) + + # if self._debug: + # self._doclog.debug('Npath: %s', npath) + for n in npath: + if n == '..': + raise ValueError("Invalid '..' element in path") + for ch in ('*', '|', "\\", '/', ':', '"', '<', '>', '?',): + if ch in n: + raise ValueError("Invalid char %s in path %s" %(ch, n)) + dpath = [store_path,] + dpath += npath[:-1] + path = os.path.join(*dpath) + if not os.path.isdir(path): + self._doclog.debug("Create dirs: %s", path) + os.makedirs(path) + return path, npath + def get_data(self, cr, uid, id, file_node, context=None, fil_obj=None): """ retrieve the contents of some file_node having storage_id = id optionally, fil_obj could point to the browse object of the file @@ -416,15 +446,15 @@ class document_storage(osv.osv): return nodefd_db64(file_node, ira_browse=ira, mode=mode) elif boo.type == 'realstore': - if not ira.store_fname: - # On a migrated db, some files may have the wrong storage type - # try to fix their directory. - if ira.file_size: - self._doclog.warning("ir.attachment #%d does not have a filename, trying the name." %ira.id) - sfname = ira.name - fpath = os.path.join(boo.path,ira.store_fname or ira.name) - if (not os.path.exists(fpath)) and mode in ('r','r+'): + path, npath = self.__prepare_realpath(cr, file_node, ira, boo.path, + do_create = (mode[1] in ('w','a')) ) + fpath = os.path.join(path, npath[-1]) + if (not os.path.exists(fpath)) and mode[1] == 'r': raise IOError("File not found: %s" % fpath) + elif mode[1] in ('w', 'a') and not ira.store_fname: + store_fname = os.path.join(*npath) + cr.execute('UPDATE ir_attachment SET store_fname = %s WHERE id = %s', + (store_fname, ira.id)) return nodefd_file(file_node, path=fpath, mode=mode) elif boo.type == 'virtual': @@ -529,21 +559,7 @@ class document_storage(osv.osv): (out, file_node.file_id)) elif boo.type == 'realstore': try: - file_node.fix_ppath(cr, ira) - npath = file_node.full_path() or [] - # npath may contain empty elements, for root directory etc. - for i, n in enumerate(npath): - if n == None: - del npath[i] - for n in npath: - for ch in ('*', '|', "\\", '/', ':', '"', '<', '>', '?', '..'): - if ch in n: - raise ValueError("Invalid char %s in path %s" %(ch, n)) - dpath = [boo.path,] - dpath += npath[:-1] - path = os.path.join(*dpath) - if not os.path.isdir(path): - os.makedirs(path) + path, npath = self.__prepare_realpath(cr, file_node, ira, boo.path, do_create=True) fname = os.path.join(path, npath[-1]) fp = file(fname,'wb') fp.write(data) @@ -644,25 +660,23 @@ class document_storage(osv.osv): # nothing to do for a rename, allow to change the db field return { 'name': new_name, 'datas_fname': new_name } elif sbro.type == 'realstore': - fname = fil_bo.store_fname + ira = self.pool.get('ir.attachment').browse(cr, uid, file_node.file_id, context=context) + + path, npath = self.__prepare_realpath(cr, file_node, ira, sbro.path, do_create=False) + fname = ira.store_fname + if not fname: - return ValueError("Tried to rename a non-stored file") - path = storage_bo.path - oldpath = os.path.join(path, fname) - - for ch in ('*', '|', "\\", '/', ':', '"', '<', '>', '?', '..'): - if ch in new_name: - raise ValueError("Invalid char %s in name %s" %(ch, new_name)) - - file_node.fix_ppath(cr, ira) - npath = file_node.full_path() or [] - dpath = [path,] - dpath.extend(npath[:-1]) - dpath.append(new_name) - newpath = os.path.join(*dpath) - # print "old, new paths:", oldpath, newpath + self._doclog.warning("Trying to rename a non-stored file") + if fname != os.path.join(*npath): + self._doclog.warning("inconsistency in realstore: %s != %s" , fname, repr(npath)) + + oldpath = os.path.join(path, npath[-1]) + newpath = os.path.join(path, new_name) os.rename(oldpath, newpath) - return { 'name': new_name, 'datas_fname': new_name, 'store_fname': new_name } + store_path = npath[:-1] + store_path.append(new_name) + store_fname = os.path.join(*store_path) + return { 'name': new_name, 'datas_fname': new_name, 'store_fname': store_fname } else: raise TypeError("No %s storage" % boo.type) diff --git a/addons/document/nodes.py b/addons/document/nodes.py index e368c6fc5a0..594f200152b 100644 --- a/addons/document/nodes.py +++ b/addons/document/nodes.py @@ -631,7 +631,9 @@ class node_dir(node_database): if not in_write: # We have to update the data ourselves if ret: - dir_obj.write(cr, self.context.uid, [self.dir_id,], ret, self.context.context) + ctx = self.context.context.copy() + ctx['__from_node'] = True + dir_obj.write(cr, self.context.uid, [self.dir_id,], ret, ctx) ret = True return ret @@ -1196,7 +1198,9 @@ class node_file(node_class): if not in_write: # We have to update the data ourselves if ret: - doc_obj.write(cr, self.context.uid, [self.file_id,], ret, self.context.context) + ctx = self.context.context.copy() + ctx['__from_node'] = True + doc_obj.write(cr, self.context.uid, [self.file_id,], ret, ctx ) ret = True return ret From a47be2ef0d72c5de183b9af1d44f33055c6c2dcc Mon Sep 17 00:00:00 2001 From: "P. Christeas" Date: Tue, 13 Jul 2010 23:54:18 +0300 Subject: [PATCH 4/5] Document ftp: improve exception handling, size may return 404 bzr revid: p_christ@hol.gr-20100713205418-8oe6svsdvw2yy4jn --- addons/document_ftp/ftpserver/abstracted_fs.py | 4 +++- addons/document_ftp/ftpserver/ftpserver.py | 16 +++++++++++----- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/addons/document_ftp/ftpserver/abstracted_fs.py b/addons/document_ftp/ftpserver/abstracted_fs.py index 5e7c8c1b31c..ddd34e2a04d 100644 --- a/addons/document_ftp/ftpserver/abstracted_fs.py +++ b/addons/document_ftp/ftpserver/abstracted_fs.py @@ -163,6 +163,8 @@ class abstracted_fs(object): ret = child.open_data(cr, mode) cr.commit() return ret + except EnvironmentError: + raise except Exception,e: self._log.exception('Cannot create item %s at node %s', objname, repr(node)) raise OSError(1, 'Operation not permited.') @@ -432,7 +434,7 @@ class abstracted_fs(object): def getsize(self, datacr): """Return the size of the specified file in bytes.""" if not (datacr and datacr[1]): - return 0L + raise IOError(errno.ENOENT, "No such file or directory") if datacr[1].type in ('file', 'content'): return datacr[1].get_data_len(datacr[0]) or 0L return 0L diff --git a/addons/document_ftp/ftpserver/ftpserver.py b/addons/document_ftp/ftpserver/ftpserver.py index e267216ddec..528ce057ff9 100644 --- a/addons/document_ftp/ftpserver/ftpserver.py +++ b/addons/document_ftp/ftpserver/ftpserver.py @@ -2565,9 +2565,12 @@ class FTPHandler(asynchat.async_chat): # return size = self.run_as_current_user(self.fs.getsize, datacr) except EnvironmentError, err: - why = _strerror(err) + why = err.strerror self.log('FAIL SIZE "%s". %s.' %(line, why)) - self.respond('550 %s.' %why) + if err.errno == errno.ENOENT: + self.respond("404 %s." % why) + else: + self.respond('550 %s.' % why) else: self.respond("213 %s" %size) self.log('OK SIZE "%s".' %line) @@ -2640,10 +2643,13 @@ class FTPHandler(asynchat.async_chat): try: datacr = self.fs.get_crdata(line, mode='delete') self.run_as_current_user(self.fs.remove, datacr) - except OSError, err: - why = _strerror(err) + except EnvironmentError, err: + why = err.strerror self.log('FAIL DELE "%s". %s.' %(line, why)) - self.respond('550 %s.' %why) + if err.errno == errno.ENOENT: + self.respond('404 %s.' % why) + else: + self.respond('550 %s.' %why) else: self.log('OK DELE "%s".' %line) self.respond("250 File removed.") From 10eea231091570db9740c7fe6b5e7ed640af898e Mon Sep 17 00:00:00 2001 From: "P. Christeas" Date: Tue, 13 Jul 2010 23:54:21 +0300 Subject: [PATCH 5/5] Document tests: remove last, empty block This just suppresses a warning. bzr revid: p_christ@hol.gr-20100713205421-th0c0vuohsum3vj8 --- addons/document/test/document_test2.yml | 1 - addons/document_ftp/test/document_ftp_test2.yml | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/addons/document/test/document_test2.yml b/addons/document/test/document_test2.yml index a83cfa23bfe..712399afac3 100644 --- a/addons/document/test/document_test2.yml +++ b/addons/document/test/document_test2.yml @@ -70,4 +70,3 @@ assert ids == [ ref("attach_3rd")], ids - !delete {model: ir.attachment, id: attach_3rd, search: } -- diff --git a/addons/document_ftp/test/document_ftp_test2.yml b/addons/document_ftp/test/document_ftp_test2.yml index 55a77d6b73e..e9ac2d84582 100644 --- a/addons/document_ftp/test/document_ftp_test2.yml +++ b/addons/document_ftp/test/document_ftp_test2.yml @@ -217,5 +217,4 @@ # TODO - I move the 200 files to 'Test-Folder2' - # TODO -- \ No newline at end of file + # TODO \ No newline at end of file