refactor: getter defaults, more comments, update docs

README.md

@@ -77,8 +77,52 @@ const query = Movie.query()
   ...
 ```
 
+That doesn't mean raw queries aren't supported at all. You do need to use a special function for this though, called `orderByExplicit` (because `orderByRaw` was taken...)
+
+```js
+const {raw} = require('objection');
+
+const query = Movie.query()
+
+  // Coalesce null values into empty string
+  .orderByExplicit(raw('COALESCE(??, ?)', ['alt_title', '']))
+
+  // Same as above
+  .orderByExplicit(raw('COALESCE(??, ?)', ['alt_title', '']), 'asc')
+
+  // Works with reference builders and strings
+  .orderByExplicit(ref('details:completed').castText(), 'desc')
+
+   // Reference builders can be used as part of raw queries
+  .orderByExplicit(raw('COALESCE(??, ?, ?)', ['even_more_alt_title', ref('alt_title'), raw('?', '')]))
+
+   // Sometimes you need to go deeper...
+  .orderByExplicit(
+    raw('CASE WHEN ?? IS NULL THEN ? ELSE ?? END', ['alt_title', '', 'alt_title'])
+    'asc',
+
+    /* Since this is a cursor plugin, we need to compare actual values that are encoded in the cursor.
+     * `orderByExplicit` needs to know how to compare a column to a value, which isn't easy to guess
+     * when you're throwing raw queries at it! By default, `orderByExplicit` uses the first binding you
+     * passed to the column's raw query, but if that column isn't the first or only column binding you
+     * passed, you need to help the function a bit.
+     */
+    value => raw('CASE WHEN ? = NULL THEN ? ELSE ? END', [value, '', value]),
+
+    /* If the column isn't the first binding in the raw query, you will need to specify how to access
+     * it in the resulting object(s). This is also true if you use some postprocessing on the returned
+     * data which changes the name of the property where the value is stored.
+     */
+    'alt_title'
+  )
+  .orderBy('id')
+  ...
+```
+
 Cursors ordered by nullable columns won't work out-of-the-box. For this reason the mixin also introduces an `orderByCoalesce` method, which you can use to treat nulls as some other value for the sake of comparisons. Same as `orderBy`, `orderByCoalesce` supports reference builders, but not raw queries.
 
+**Deprecated!** Use `orderByExplicit` instead.
+
 ```js
 const query = Movie.query()
   .orderByCoalesce('alt_title', 'asc', '') // Coalesce null values into empty string
@@ -179,14 +223,26 @@ Alias for `cursorPage`, with `before: true`.
 
 ### `orderByCoalesce(column, [direction, [values]])`
 
+> **Deprecated**: use `orderByExplicit` instead.
+
 Use this if you want to sort by a nullable column.
 
 - `column` - Column to sort by.
 - `direction` - Sort direction.
   - Default: `asc`
-- `values` - Values to coalesce to. If column has a null value, treat it as the first non-null value in `values`. Can be one or many of: *string*, *ReferenceBuilder* or *RawQuery*.
+- `values` - Values to coalesce to. If column has a null value, treat it as the first non-null value in `values`. Can be one or many of: *primitive*, *ReferenceBuilder* or *RawQuery*.
   - Default: `['']`
 
+### `orderByExplicit(column, [direction, [getValue, [property]]])`
+
+Use this if you want to sort by a nullable column.
+
+- `column` - Column to sort by. If this is _not_ a RawBuilder, `getValue` and `property` will be ignored.
+- `direction` - Sort direction.
+  - Default: `asc`
+- `getValue` callback - Callback is called with a value, and should return one of *primitive*, *ReferenceBuilder* or *RawQuery*. The returned value will be compared against `column` when determining which row to show results before/after.
+- `property` - Values will be encoded inside cursors based on ordering, and for this reason `orderByExplicit` needs to know how to access the correct value in the resulting objects. The function will try to guess by picking the first binding you pass to `column` raw query, but if for some reason this guess would be wrong, you need to specify here how to access the value.
+
 # Options
 
 Values shown are defaults.
@@ -219,6 +275,6 @@ Values shown are defaults.
 
 **`remaining` vs `remainingBefore` and `remainingAfter`:**
 
-`remaining` only tells you the remaining results in *current* direction and is therefore less descriptive as `remainingBefore` and `remainingAfter` combined. However, in cases where it's enough to know if there are "more" results, using only the `remaining` information will use one less query than using any one of `remainingBefore`, `remainingAfter`. Similarly `hasMore` uses one less query than `hasPrevious`, and `hasNext`.
- 
+`remaining` only tells you the remaining results in the *current* direction and is therefore less descriptive as `remainingBefore` and `remainingAfter` combined. However, in cases where it's enough to know if there are "more" results, using only the `remaining` information will use one less query than using either of `remainingBefore` or `remainingAfter`. Similarly `hasMore` uses one less query than `hasPrevious`, and `hasNext`.
+
 However, if `total` is used, then using `remaining` no longer gives you the benefit of using one less query.

lib/query-builder/CursorQueryBuilder.js

@@ -8,9 +8,15 @@ const {resolveOrderByOperation, lockStatement} = require('../utils');
 module.exports = function (options, Base) {
 	return class extends OrderQueryBuilder(Base) {
 		cursorPage(cursor, before = false) {
+			/* We build the actual cursor logic in `runBefore` and `onBuild` handlers to relax the need to
+			 * chain operations in a specific order.
+			 */
 			return this
 				.runBefore((result, builder) => {
-					// Save current builder (before additional where statements) for pageInfo (total)
+					/* Save current builder (before additional where statements) for pageInfo. `orderBy` statements
+					 * are also invoked in a `runBefore` handler, but since we don't care about their presence
+					 * when using the original builder, the order in which these handlers run doesn't matter.
+					 */
 					const originalQuery = builder.clone()
 						.$flag('originalQuery', true)
 						.$flag('onBuild', false);
@@ -20,6 +26,10 @@ module.exports = function (options, Base) {
 					return result;
 				})
 				.onBuild(builder => lockStatement(builder, 'onBuild', () => {
+					/* We want to build the cursor only after the original query has been saved and orderBy
+					 * statements have been invoked. This is why we build the cursor in an `onBuild` handler,
+					 * which is ran strictly after `runBefore`.
+					 */
 					this._buildCursor(builder, cursor, before);
 				}))
 				.runAfter((models, builder) => {
@@ -28,6 +38,12 @@ module.exports = function (options, Base) {
 						models.reverse();
 					}
 
+					/* When building the cursor, we want to know the values of the properties that the user has
+					 * ordered their data by. We build a "partial item" based on those columns to make it easier
+					 * to visualize for the developer. For example, if the queried data was something like
+					 * {id: 2, title: 'hi', author: 'you'}, and the user ordered their data by `id` and `author`,
+					 * then the partial item would be {id: 2, author: 'you'}.
+					 */
 					const item = this._getPartialCursorItem(cursor);
 
 					/* When we reach end while going forward, save the last element of the last page, but discard
@@ -78,6 +94,20 @@ module.exports = function (options, Base) {
 			}));
 		}
 
+		/**
+		 * Recursive procedure to build where statements needed to get rows before/after some given item (row).
+		 *
+		 * Let's say we want to order by columns [c1, c2, c3], all in ascending order for simplicity.
+		 * The resulting structure looks like this:
+		 *
+		 * - If c1 > value, return row
+		 * - Otherwise, if c1 = value and c2 > value, return row
+		 * - Otherwise, if c1 = value and c2 = value and c3 > value, return row
+		 * - Otherwise, do not return row
+		 *
+		 * Comparisons are simply flipped if order is 'desc', and Objection knows to compare columns to
+		 * nulls correctly with "column IS NULL" instead of "column = NULL".
+		 */
 		_addWhereStmts(builder, orderByOperations, item, composites = []) {
 			if (!item) {
 				return;
@@ -122,6 +152,7 @@ module.exports = function (options, Base) {
 
 			this._addWhereStmts(builder, orderByOps, item);
 
+			// Add default limit unless we are in the process of calculating total rows
 			if (!builder.has(/limit/) && !builder.$flag('resultSizeQuery')) {
 				builder.limit(options.limit);
 			}
@@ -155,6 +186,7 @@ module.exports = function (options, Base) {
 			return Promise.resolve()
 				.then(() => {
 					if (isEnabled(['total', 'hasNext', 'hasPrevious', 'remainingBefore', 'remainingAfter'])) {
+						// Count number of rows without where statements or limits
 						return builder.$data('originalQuery').resultSize().then(rs => {
 							total = parseInt(rs, 10);
 							setIfEnabled('total', total);
@@ -163,6 +195,9 @@ module.exports = function (options, Base) {
 				})
 				.then(() => {
 					if (isEnabled(['hasMore', 'hasNext', 'hasPrevious', 'remaining', 'remainingBefore', 'remainingAfter'])) {
+						/* Count number of rows without limits, but retain where statements to count rows
+						 * only in one direction. I.e. get number of rows before/after current results.
+						 */
 						return builder.$data('resultSizeQuery').resultSize().then(rs => {
 							const remaining = rs - models.length;
 							setIfEnabled('remaining', remaining);

lib/query-builder/OrderQueryBuilder.js

@@ -7,11 +7,19 @@ const ContextBase = require('./ContextBase');
 module.exports = function (Base) {
 	return class extends ContextBase(Base) {
 		orderBy(column, order = 'asc') {
+			/* To build the cursor we first gather all the `orderBy` instructions in memory, and then only later
+			 * tell Objection about these instructions. This lets us relax the need to chain operations in a
+			 * specific order. We might need to "flip" the operations when getting a previous cursor page,
+			 * for example, which is something we know only after the `cursorPage` method has been called.
+			 */
 			if (this.$flag('runBefore')) {
 				// orderBy was called from an onBuild handler
 				return super.orderBy(column, order);
 			}
 
+			/* We want to know how exactly these `orderBy`s were called, which is something Objection
+			 * cannot tell us since we do modifications on the operations later.
+			 */
 			this.$data('orderBy', (this.$data('orderBy') || []).concat([{column, order}]));
 
 			return this
@@ -21,19 +29,38 @@ module.exports = function (Base) {
 				});
 		}
 
+		// DEPRECATED: replaced by `orderByExplicit`
 		orderByCoalesce(column, order, coalesceValues = ['']) {
 			coalesceValues = castArray(coalesceValues);
 			const coalesceBindingsStr = coalesceValues.map(() => '?').join(', ');
 
-			return this.orderByExplicit({
-				column: raw(`COALESCE(??, ${coalesceBindingsStr})`, [column].concat(coalesceValues)),
-				order,
-				property: columnToProperty(this.modelClass(), column),
-				getValue: val => raw(`COALESCE(?, ${coalesceBindingsStr})`, [val].concat(coalesceValues))
-			});
+			return this.orderByExplicit(
+				raw(`COALESCE(??, ${coalesceBindingsStr})`, [column].concat(coalesceValues)),
+				order
+			);
 		}
 
-		orderByExplicit({column, order, property, getValue = val => val}) {
+		orderByExplicit(column, order, getValue, property) {
+			// Convert `column` to RawBuilder if it isn't one
+			if (!column.constructor || column.constructor.name !== 'RawBuilder') {
+				return this.orderBy(column, order);
+			}
+
+			/* By default, get a RawBuilder for a value that is identical to the column RawBuilder,
+			 * except first argument is the value instead of column name.
+			 */
+			if (!getValue) {
+				// Change first ?? binding to ? (value instead of column)
+				const sql = column._sql.replace('??', '?');
+				getValue = val => raw(sql, [val].concat(column._args.slice(1)));
+			}
+
+			// By default, get column name from first argument of the column RawBuilder
+			if (!property) {
+				const columnName = column._args[0];
+				property = columnToProperty(this.modelClass(), columnName);
+			}
+
 			return this
 				.$data('orderByExplicit', Object.assign({}, this.$data('orderByExplicit'), {
 					[property]: {column, getValue}
@@ -43,11 +70,16 @@ module.exports = function (Base) {
 
 		_buildOrderBy(builder) {
 			lockStatement(builder, 'runBefore', () => {
+				/* Clear any existing `orderBy` instructions (runBefore might be called multiple times)
+				 * to prevent duplicates in the resulting query builder. This shouldn't affect the end result,
+				 * but duplicates make debugging harder.
+				 */
 				builder.clear(/orderBy/);
 
 				const orderByData = builder.$data('orderBy') || [];
 				const orderByExplicitData = builder.$data('orderByExplicit') || {};
 
+				// Tell Objection about any `orderBy` instructions
 				for (const {column: property, order} of orderByData) {
 					const column = orderByExplicitData[property] ? orderByExplicitData[property].column : property;
 					builder.orderBy(column, order);