Skip to content

Commit

Permalink
Reduce complexity of verify_can_exec funcs
Browse files Browse the repository at this point in the history 
This applies to most action types. The original way that the verify_can_exec
functions were written predates a nice 'filesys' object to wrap things like
os.access(). Now that the filesys can handle high level operations, there's no
need for every verification function to break down checks into multiple locally
defined functions. Also adds a bit of extra tooling to the abstract filesys
type to support this behavior for the common case of checking writability of a
file or its only existing ancestor.
  • Loading branch information
@sirosen
sirosen committed Oct 24, 2015
1 parent 2f023d5 commit 78144cb
Show file tree
Hide file tree
Showing 10 changed files with 53 additions and 198 deletions.
3 changes: 1 addition & 2 deletions salve/action/backup/directory.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ def execute(self, filesys):
# for now, to keep it super-simple, we ignore empty dirs
for f in files:
filename = paths.pjoin(dirname, f)
self.append(FileBackupAction(filename,
self.file_context))
self.append(FileBackupAction(filename, self.file_context))

ActionList.execute(self, filesys)
37 changes: 3 additions & 34 deletions salve/action/backup/file.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,53 +41,22 @@ def __str__(self):
",context=" + str(self.file_context) + ")")

def verify_can_exec(self, filesys):
# transition to the action verification phase,
# confirming execution will work
ExecutionContext().transition(ExecutionContext.phases.VERIFICATION)

def writable_target():
"""
Checks if the backup target dir is writable.
"""
if filesys.access(self.dst, access_codes.W_OK):
return True

if filesys.access(self.dst, access_codes.F_OK):
return False # pragma: no cover

# at this point, the dir is known not to exist
# now check properties of the containing dir
containing_dir = filesys.get_existing_ancestor(self.dst)
if filesys.access(containing_dir, access_codes.W_OK):
return True

# if the dir doesn't exist, and the dir containing it
# isn't writable, then the dir can't be written
return False

def existent_source():
return filesys.access(self.src, access_codes.F_OK)

def readable_source():
"""
Checks if the source is a readable file.
"""
return filesys.access(self.src, access_codes.R_OK)

logger.info(
'{0}: FileBackup: Checking source existence, \"{1}\"'.format(
str(self.file_context), self.src)
)

if not existent_source():
if not filesys.exists(self.src):
return self.verification_codes.NONEXISTENT_SOURCE

logger.info(
'{0}: FileBackup: Checking source is readable, \"{1}\"'.format(
str(self.file_context), self.src)
)

if not readable_source():
if not filesys.access(self.src, access_codes.R_OK):
return self.verification_codes.UNREADABLE_SOURCE

logger.info(
Expand All @@ -96,7 +65,7 @@ def readable_source():
str(self.file_context), self.dst)
)

if not writable_target():
if not filesys.writable_path_or_ancestor(self.dst):
return self.verification_codes.UNWRITABLE_TARGET

return self.verification_codes.OK
Expand Down
39 changes: 2 additions & 37 deletions salve/action/copy/directory.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,6 @@


class DirCopyAction(CopyAction):
"""
An action to copy a directory tree.
"""
def __init__(self, src, dst, file_context):
"""
DirCopyAction constructor.
Args:
@src
Source path.
@dst
Destination path.
@file_context
The FileContext.
"""
CopyAction.__init__(self, src, dst, file_context)

def __str__(self):
return ("DirCopyAction(src=" + str(self.src) + ",dst=" +
str(self.dst) + ",context=" + repr(self.file_context) + ")")
Expand All @@ -31,38 +14,21 @@ def verify_can_exec(self, filesys):
Check to ensure that execution can proceed without errors.
Ensures that the the target directory is writable.
"""
# transition to the action verification phase,
# confirming execution will work
ExecutionContext().transition(ExecutionContext.phases.VERIFICATION)

def readable_source():
"""
Checks if the source is a readable and traversable directory. If
not, then it will be impossible to view and copy its contents.
"""
return filesys.access(self.src,
access_codes.R_OK | access_codes.X_OK)

def writable_target():
"""
Checks if the target is in a writable directory.
"""
return filesys.access(paths.dirname(self.dst),
access_codes.W_OK)

logstr = ('DirCopy: Checking source is readable + traversable, ' +
'{0}'.format(self.dst))
logger.info('{0}: {1}'.format(self.file_context, logstr))

if not readable_source():
if not filesys.access(self.src, access_codes.R_OK | access_codes.X_OK):
return self.verification_codes.UNREADABLE_SOURCE

logger.info(
('{0}: DirCopy: Checking target is writeable, \"{1}\"').format(
self.file_context, self.dst)
)

if not writable_target():
if not filesys.access(paths.dirname(self.dst), access_codes.W_OK):
return self.verification_codes.UNWRITABLE_TARGET

return self.verification_codes.OK
Expand All @@ -83,7 +49,6 @@ def execute(self, filesys):
logger.warn('{0}: {1}'.format(self.file_context, logstr))
return

# transition to the execution phase
ExecutionContext().transition(ExecutionContext.phases.EXECUTION)

logstr = ('Performing Directory Copy \"%s\" -> \"%s\"' %
Expand Down
60 changes: 4 additions & 56 deletions salve/action/copy/file.py
View file
Original file line number Diff line number Diff line change
@@ -1,27 +1,10 @@
from salve import logger, paths
from salve import logger
from salve.action.copy.base import CopyAction
from salve.filesys import access_codes
from salve.context import ExecutionContext


class FileCopyAction(CopyAction):
"""
An action to copy a single file.
"""
def __init__(self, src, dst, file_context):
"""
FileCopyAction constructor.
Args:
@src
Source path.
@dst
Destination path.
@file_context
The FileContext.
"""
CopyAction.__init__(self, src, dst, file_context)

def __str__(self):
"""
Stringification into type, source, dst, and context.
Expand All @@ -35,59 +18,25 @@ def verify_can_exec(self, filesys):
Ensures that the source file exists and is readable, and that
the target file can be created or is writable.
"""
# transition to the action verification phase,
# confirming execution will work
ExecutionContext().transition(ExecutionContext.phases.VERIFICATION)

def writable_target():
"""
Checks if the target file is writable.
"""
if filesys.access(self.dst, access_codes.W_OK):
return True
if filesys.access(self.dst, access_codes.F_OK):
return False

# at this point, the file is known not to exist
# now check properties of the containing dir
containing_dir = paths.dirname(self.dst)
if filesys.access(containing_dir, access_codes.W_OK):
return True

# if the file doesn't exist, and the dir containing it
# isn't writable, then the file can't be written
return False

def readable_source():
"""
Checks if the source is a readable file.
"""
return filesys.access(self.src, access_codes.R_OK)

def source_islink():
"""
Checks if the source is a symlink (copied by value, not
dereferenced)
"""
return filesys.lookup_type(self.src) is filesys.element_types.LINK

logstr = 'FileCopy: Checking destination is writable, \"%s\"' % \
self.dst
logger.info('{0}: {1}'.format(self.file_context, logstr))

if not writable_target():
if not filesys.writable_path_or_ancestor(self.dst):
return self.verification_codes.UNWRITABLE_TARGET

logstr = 'FileCopy: Checking if source is link, "%s"' % self.src
logger.info('{0}: {1}'.format(self.file_context, logstr))

if source_islink():
if filesys.lookup_type(self.src) is filesys.element_types.LINK:
return self.verification_codes.OK

logstr = 'FileCopy: Checking source is readable, \"%s\"' % self.src
logger.info('{0}: {1}'.format(self.file_context, logstr))

if not readable_source():
if not filesys.access(self.src, access_codes.R_OK):
return self.verification_codes.UNREADABLE_SOURCE

return self.verification_codes.OK
Expand All @@ -111,7 +60,6 @@ def execute(self, filesys):
logger.warn('{0}: {1}'.format(self.file_context, logstr))
return

# transition to the execution phase
ExecutionContext().transition(ExecutionContext.phases.EXECUTION)

logstr = 'Performing File Copy \"%s\" -> \"%s\"' % (self.src, self.dst)
Expand Down
13 changes: 13 additions & 0 deletions salve/action/create/base.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,16 @@ class CreateAction(with_metaclass(abc.ABCMeta, Action)):
"""
verification_codes = \
Action.verification_codes.extend('UNWRITABLE_TARGET')

def __init__(self, dst, file_context):
"""
CreateAction initializer
Args:
@dst
Destination path.
@file_context
The FileContext.
"""
Action.__init__(self, file_context)
self.dst = dst
27 changes: 1 addition & 26 deletions salve/action/create/directory.py
View file
Original file line number Diff line number Diff line change
@@ -1,28 +1,13 @@
from salve import logger
from salve.action.base import Action
from salve.action.create.base import CreateAction

from salve.filesys import access_codes
from salve.context import ExecutionContext


class DirCreateAction(CreateAction):
"""
An action to create a directory.
"""
def __init__(self, dst, file_context):
"""
DirCreateAction constructor.
Args:
@dst
Destination path.
@file_context
The FileContext.
"""
Action.__init__(self, file_context)
self.dst = dst

def __str__(self):
return ("DirCreateAction(dst=" + self.dst + ",context=" +
repr(self.file_context) + ")")
Expand All @@ -31,17 +16,8 @@ def verify_can_exec(self, filesys):
"""
Checks if the target dir already exists, or if its parent is writable.
"""
# transition to the action verification phase,
# confirming execution will work
ExecutionContext().transition(ExecutionContext.phases.VERIFICATION)

def writable_target():
"""
Checks if the target is in a writable directory.
"""
ancestor = filesys.get_existing_ancestor(self.dst)
return filesys.access(ancestor, access_codes.W_OK)

logstr = 'DirCreate: Checking if target exists, \"%s\"' % self.dst
logger.info('{0}: {1}'.format(self.file_context, logstr))

Expand All @@ -52,7 +28,7 @@ def writable_target():
logstr = 'DirCreate: Checking target is writable, \"%s\"' % self.dst
logger.info('{0}: {1}'.format(self.file_context, logstr))

if not writable_target():
if not filesys.writable_path_or_ancestor(self.dst):
return self.verification_codes.UNWRITABLE_TARGET

return self.verification_codes.OK
Expand All @@ -69,7 +45,6 @@ def execute(self, filesys):
logger.warn('{0}: {1}'.format(self.file_context, logstr))
return

# transition to the execution phase
ExecutionContext().transition(ExecutionContext.phases.EXECUTION)

logstr = 'Performing Directory Creation of \"%s\"' % self.dst
Expand Down
40 changes: 2 additions & 38 deletions salve/action/create/file.py
View file
Original file line number Diff line number Diff line change
@@ -1,27 +1,13 @@
from salve import logger, paths
from salve import logger
from salve.action.create.base import CreateAction

from salve.filesys import access_codes
from salve.context import ExecutionContext


class FileCreateAction(CreateAction):
"""
An action to create a single file.
"""
def __init__(self, dst, file_context):
"""
FileCreateAction constructor.
Args:
@dst
Destination path.
@file_context
The FileContext.
"""
CreateAction.__init__(self, file_context)
self.dst = dst

def __str__(self):
return ("FileCreateAction(dst=" + self.dst +
",context=" + repr(self.file_context) + ")")
Expand All @@ -31,33 +17,12 @@ def verify_can_exec(self, filesys):
Ensures that the target file exists and is writable, or that
it does not exist and is in a writable directory.
"""
# transition to the action verification phase,
# confirming execution will work
ExecutionContext().transition(ExecutionContext.phases.VERIFICATION)

def writable_target():
"""
Checks if the target is in a writable directory.
"""
if filesys.access(self.dst, access_codes.W_OK):
return True
if filesys.access(self.dst, access_codes.F_OK):
return False
# file is now known not to exist
assert not filesys.exists(self.dst)

parent = paths.dirname(self.dst)
if filesys.access(parent, access_codes.W_OK):
return True

# the file is doesn't exist and the containing dir is
# not writable or doesn't exist
return False

logstr = 'FileCreate: Checking target is writable, \"%s\"' % self.dst
logger.info('{0}: {1}'.format(self.file_context, logstr))

if not writable_target():
if not filesys.writable_path_or_ancestor(self.dst):
return self.verification_codes.UNWRITABLE_TARGET

return self.verification_codes.OK
Expand All @@ -75,7 +40,6 @@ def execute(self, filesys):
logger.warn('{0}: {1}'.format(self.file_context, logstr))
return

# transition to the execution phase
ExecutionContext().transition(ExecutionContext.phases.EXECUTION)

logstr = 'Performing File Creation of \"%s\"' % self.dst
Expand Down
Loading

0 comments on commit 78144cb

Please sign in to comment.