I have a working script that will batch add users to an instance of nextcloud running on top of docker. This version is the result of changes made after asking this question.
I guess I'm looking for a re-review to ensure that I haven't missed anything. I'm also interested if there are areas where I could make things better in terms of readability, performance, security, etc. You're the reviewer. My code is your oyster.
#!/bin/sh
set -eu
# Handle printing errors
die () {
printf '%s\n' "1ドル" >&2
exit 1
}
message () {
printf '%s\n' "1ドル" >&2
}
usage () {
cat <<'EOF'
NAME
batch_up - batch upload users
SYNOPSIS
batch_up -?
batch_up -h
batch_up --help
batch_up -v
batch_up -vv
batch_up -vvv
batch_up --verbose
batch_up -d
batch_up --delimiter
batch_up --delimiter=
DESCRIPTION
Batch_up is a program that adds a batch of users to an instance of NextCloud
running inside of a docker container by reading a comma separated list from
stdin or a csv file. Stdin is the default and will be used if no file is
supplied. The delimiter does not have to be a comma, and can be set via the
-d flag.
CSV file should be formatted in one of the following configurations:
username,Display Name,group,[email protected]
username,Display Name,group
username,Display Name
username
CSV files should not include the header.
OPTIONS
-? or -h or --help
This option displays a summary of the commands accepted by batch_up.
-v or -vv or -vvv or --verbose
This option sets the verbosity. Each v adds to the verbosity.
See the occ command for OwnCloud/NextCloud for more details as this
option is passed on to the occ command.
If this option is not passed, the default behavior is to pass the
-q option to occ (the quiet option), making occ non-verbose.
-d or --delimiter or --delimiter=
This option allows you to choose which delimiter you would like to use.
The following are acceptable characters for this option. ,.;:|
ENVIRONMENT VARIABLES
OC_PASS
Sets the password for users added. The OC_PASS environment variable
is required.
EXAMPLES
The command:
batch_up.sh foobar.csv
Will add the users from foobar.csv. Users will be given the password
in the OC_PASS environment variable.
The command:
batch_up.sh <<< "jack,Jack Frost,users,[email protected]"
Will add the user jack, set his display name to Jack Frost, add him
to the group users, and set his email to [email protected].
The command:
echo "jack,Jack Frost,users,[email protected]" | batch_up.sh
Will add the user jack, set his display name to Jack Frost, add him
to the group users, and set his email to [email protected].
The command:
batch_up.sh -d : foobar.csv
Will set the delimiter to : and add the users from foobar.csv.
EOF
}
# Set verbosity. Default is silent.
verbose=-q
# flags
while [ $# -gt 0 ]
do
case 1ドル in
-h|-\?|--help)
usage # Display a usage synopsis.
exit
;;
-v|-vv|-vvv|--verbose)
verbose=1ドル
;;
-d|--delimiter)
if [ $# -gt 1 ]
then
case 2ドル in
,|.|\;|:|\|)
delimiter=2ドル
shift
;;
*)
die 'Error: delimiter should be one of ,.;:| characters.'
esac
else
die 'Error: "--delimiter requires a non-empty option argument.'
fi
;;
--delimiter=?*)
case ${1#*=} in # Delete everything up to the = and assign the remainder.
,|.|\;|:|\|)
delimiter=${1#*=}
shift
;;
*)
die 'Error: delimiter should be one of ,.;:| characters.'
esac
;;
--delimiter=) # Handle the case of empty --delimiter=
die 'Error: "--delimiter=" requires a non-empty option argument.'
;;
--)
shift
break
;;
-?*)
printf 'WARN: Unknown option (ignored): %s\n' "1ドル" >&2
;;
*) # Default case. No more options, so break out of the loop
break
esac
shift
done
# Is the file readable?
if [ $# -gt 0 ]
then
[ -r "1ドル" ] || die "1ドル: Couldn't read file."
fi
# Is the OC_PASS environment variable set?
[ ${OC_PASS:-} ] || die "0ドル: No password specified. Run with --help for more info."
status=true # until a command fails
message 'Adding users'
while IFS=${delimiter:-,} read -r f1 f2 f3 f4
do
if [ "$f1" ]
then
docker-compose exec -T -e OC_PASS --user www-data app php occ \
user:add --password-from-env \
${verbose:+"$verbose"} \
${f2:+"--display-name=$f2"} \
${f3:+"--group=$f3"} \
"$f1" </dev/null \
|| status=false
# If there is a fourth value in the csv, use it to set the user email.
if [ "$f4" ]
then
docker-compose exec -T \
--user www-data app php occ \
user:setting "$f1" settings email "$f4" \
</dev/null \
|| status=false
fi
else
echo "Expected at least one field, but none were supplied." >&2
status=false
continue
fi
message '...'
done <"${1:-/dev/stdin}"
message 'Done'
exec $status
1 Answer 1
Don't repeat yourself
There is redundant logic in the processing of the -d and --delimiter=... options. I would eliminate that, to be something more like this:
-d|--delimiter)
[ $# -gt 1 ] && arg=1ドル || arg=
parseDelimiter "$arg"
;;
--delimiter=*)
parseDelimiter "${1#*=}"
;;
Where parseDelimiter is a function that will parse the passed argument and set delimiter appropriately, or exit with an error.
A simpler pattern in case (maybe)
Instead of ,|.|\;|:|\|), perhaps [,.\;:\|]) is slightly simpler and easier to type (less likely you mistype something, for example less likely to forget a | between the different allowed values).
Assign positional arguments to descriptive variables early on
Near the end of the script I see done <"${1:-/dev/stdin}", and wonder:
what was 1ドル again?
I think it's easier to understand when positional arguments are assigned to variables with descriptive names early on in a script.
Prefer comments on their own lines
I read code from top to bottom, and this line makes my eyes make an unnecessary detour to the right:
status=true # until a command fails
I would have preferred that comment on its own line just before the code it's referring to.
Incidentally, I don't understand what this comment is trying to say.
The status variable is used to determine the exit status of the program.
In this script, it will be "failure" if some failure happened in the loop body, otherwise "success".
I would have named this exit_status, and then at the end of the script do exit "$exit_status", rather than the current exec $status, which I find a bit odd.