ueberDB2 can return either undefined or null for a missing key, depending on
which DB driver is used. This patch changes the promise version of the API so
that it will always return null.
some code chunks previously used `async.parallel` but if you
use `await` that forces them to be run serially. Instead,
you can initiate the operation (getting a Promise) and then
_later_ `await` the result of that Promise.
If you use `await` inside a loop it makes the loop inherently serial.
If you omit the `await` however, the tasks will all start but the loop
will finish while the tasks are still being scheduled.
So, to make a set of tasks run in parallel but then have the
code block after the loop once all the tasks have been completed
you have to get an array of Promises (one for each iteration) and
then use `Promise.all()` to wait for those promises to be resolved.
Using `Array#map` is a convenient way to go from an array of inputs
to the require array of Promises.
NB1: needs additional review and testing - no abiword available on my test bed
NB2: in ImportHandler.js, directly delete the file, and handle the eventual
error later: checking before for existence is prone to race conditions,
and does not handle any errors anyway.
Also converted the handler functions that depend on checkAccess() into async
functions too.
NB: this commit needs specific attention to it because it touches a lot of
security related code!
This patch also contains significant refactoring relating to error checking of
arguments supplied to the functions (e.g. rev) facilitated by use of `throw`
instead of nodeback errors.
1. This module was not migrated to Promises, because it is only used via
express-session, which can't actually use promises anyway.
2. all(), clear() and length() depend on the presence of the `db.forEach()`
function, which in ueberdb2 doesn't even exist.
Fortunately those three methods are optional, so I made their existence
conditional on the presence of `db.forEach`.
3. in SessionStore.clear(), replaced a call to db.db.remove() with db.remove()
Changed two occurrences of:
process.nextTick(function() {
if (fn) fn();
});
with
if (fn) {
process.nextTick(fn);
}
i.e. such that no function even gets added to the `nextTick` queue unless
there's actually a function to be called.
Extracted from Ray's work.
This change is in preparation of the future async refactoring by Ray. It tries
to extract as many changes in boolean conditions as possible, in order to make
more evident identifying eventual logic bugs in the future work.
This proved already useful in at least one case.
BEWARE: this commit exposes an incoherency in the DB API, in which, depending
on the driver used, some functions can return null or undefined. This condition
will be externally fixed by the final commit in this series ("db/DB.js: prevent
DB layer from returning undefined"). Until that commit, the code base may have
some bugs.
This change extracts the grammar correction performed on the async branch,
anticipating them in a single commit. It cannot be folded with the previous
one, as it is not purely cosmetic.
This change is only cosmetic. Its aim is do make it easier to understand the
async changes that are going to be merged later on. It was extracted from the
original work from Ray Bellis.
To verify that nothing has changed, you can run the following command on each
file touched by this commit:
npm install uglify-es
diff --unified <(uglify-js --beautify bracketize <BEFORE.js>) <(uglify-js --beautify bracketize <AFTER.js>)
This is a complete script that does the same automatically (works from a
mercurial clone):
```bash
#!/usr/bin/env bash
set -eu
REVISION=<THIS_REVISION>
PARENT_REV=$(hg identify --rev "${REVISION}" --template '{p1rev}')
FILE_LIST=$(hg status --no-status --change ${REVISION})
UGLIFYJS="node_modules/uglify-es/bin/uglifyjs"
for FILE_NAME in ${FILE_LIST[@]}; do
echo "Checking ${FILE_NAME}"
diff --unified \
<("${UGLIFYJS}" --beautify bracketize <(hg cat --rev "${PARENT_REV}" "${FILE_NAME}")) \
<("${UGLIFYJS}" --beautify bracketize <(hg cat --rev "${REVISION}" "${FILE_NAME}"))
done
```
This is documented to be more performant.
The substitution was made on frontend code, too (i.e., the one in /static),
because Date.now() is supported since IE 9, and we are life supporting only
IE 11.
Commands:
find . -name *.js | xargs sed --in-place "s/new Date().getTime()/Date.now()/g"
find . -name *.js | xargs sed --in-place "s/(new Date()).getTime()/Date.now()/g"
Not done on jQuery.
The HTTP API doesn't ever omit arguments, it always passes `undefined` for a
parameter that wasn't supplied in the request.
The functions that were simplified are:
- getRevisionChangeset()
- getText()
- getHTML()
- saveRevision()
The only function still supporting optional arguments is getPadSafe(), which is
only called from this module.
This function was simulating two overloads:
1. copy(destinationID, force, callback)
2. copy(destinationID, callback), in this case "force" would be assumed false
But all the call sites always used the version with arity 3.
Thus, we can remove that optionality and always assume that the funcion will be
called with three parameters. This will simplify future work.
Since the original comparison compared for truthy and not for "===", and it's
3 AM now, I blindly negated it, in order to show how fragile it was in the first
instance.
No functional changes.
This is the final commit of this refactoring series.
