Skip to content

Commit

Permalink
Remove "cyclic" references (#2969)
Browse files Browse the repository at this point in the history
  • Loading branch information
@schmidt-sebastian
schmidt-sebastian committed Apr 23, 2020
1 parent ce07010 commit 4ad5733
Show file tree
Hide file tree
Showing 36 changed files with 182 additions and 221 deletions.
50 changes: 23 additions & 27 deletions packages/firestore/src/api/database.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ import {
validateStringEnum,
valueDescription
} from '../util/input_validation';
import { logError, setLogLevel, LogLevel, getLogLevel } from '../util/log';
import { getLogLevel, logError, LogLevel, setLogLevel } from '../util/log';
import { AutoId } from '../util/misc';
import { Deferred, Rejecter, Resolver } from '../util/promise';
import { FieldPath as ExternalFieldPath } from './field_path';
Expand Down Expand Up @@ -836,7 +836,7 @@ export class WriteBatch implements firestore.WriteBatch {
convertedValue
);
this._mutations = this._mutations.concat(
parsed.toMutations(ref._key, Precondition.NONE)
parsed.toMutations(ref._key, Precondition.none())
);
return this;
}
Expand Down Expand Up @@ -906,7 +906,7 @@ export class WriteBatch implements firestore.WriteBatch {
this._firestore
);
this._mutations = this._mutations.concat(
new DeleteMutation(ref._key, Precondition.NONE)
new DeleteMutation(ref._key, Precondition.none())
);
return this;
}
Expand Down Expand Up @@ -1031,7 +1031,7 @@ export class DocumentReference<T = firestore.DocumentData>
)
: this.firestore._dataReader.parseSetData(functionName, convertedValue);
return this._firestoreClient.write(
parsed.toMutations(this._key, Precondition.NONE)
parsed.toMutations(this._key, Precondition.none())
);
}

Expand Down Expand Up @@ -1075,7 +1075,7 @@ export class DocumentReference<T = firestore.DocumentData>
delete(): Promise<void> {
validateExactNumberOfArgs('DocumentReference.delete', arguments, 0);
return this._firestoreClient.write([
new DeleteMutation(this._key, Precondition.NONE)
new DeleteMutation(this._key, Precondition.none())
]);
}

Expand Down Expand Up @@ -1447,32 +1447,31 @@ export class Query<T = firestore.DocumentData> implements firestore.Query<T> {

// Enumerated from the WhereFilterOp type in index.d.ts.
const whereFilterOpEnums = [
'<',
'<=',
'==',
'>=',
'>',
'array-contains',
'in',
'array-contains-any'
Operator.LESS_THAN,
Operator.LESS_THAN_OR_EQUAL,
Operator.EQUAL,
Operator.GREATER_THAN_OR_EQUAL,
Operator.GREATER_THAN,
Operator.ARRAY_CONTAINS,
Operator.IN,
Operator.ARRAY_CONTAINS_ANY
];
validateStringEnum('Query.where', whereFilterOpEnums, 2, opStr);
const op = validateStringEnum('Query.where', whereFilterOpEnums, 2, opStr);

let fieldValue: api.Value;
const fieldPath = fieldPathFromArgument('Query.where', field);
const operator = Operator.fromString(opStr);
if (fieldPath.isKeyField()) {
if (
operator === Operator.ARRAY_CONTAINS ||
operator === Operator.ARRAY_CONTAINS_ANY
op === Operator.ARRAY_CONTAINS ||
op === Operator.ARRAY_CONTAINS_ANY
) {
throw new FirestoreError(
Code.INVALID_ARGUMENT,
`Invalid Query. You can't perform '${operator.toString()}' ` +
`Invalid Query. You can't perform '${op}' ` +
'queries on FieldPath.documentId().'
);
} else if (operator === Operator.IN) {
this.validateDisjunctiveFilterElements(value, operator);
} else if (op === Operator.IN) {
this.validateDisjunctiveFilterElements(value, op);
const referenceList: api.Value[] = [];
for (const arrayValue of value as api.Value[]) {
referenceList.push(this.parseDocumentIdValue(arrayValue));
Expand All @@ -1482,20 +1481,17 @@ export class Query<T = firestore.DocumentData> implements firestore.Query<T> {
fieldValue = this.parseDocumentIdValue(value);
}
} else {
if (
operator === Operator.IN ||
operator === Operator.ARRAY_CONTAINS_ANY
) {
this.validateDisjunctiveFilterElements(value, operator);
if (op === Operator.IN || op === Operator.ARRAY_CONTAINS_ANY) {
this.validateDisjunctiveFilterElements(value, op);
}
fieldValue = this.firestore._dataReader.parseQueryValue(
'Query.where',
value,
// We only allow nested arrays for IN queries.
/** allowArrays = */ operator === Operator.IN ? true : false
/** allowArrays = */ op === Operator.IN
);
}
const filter = FieldFilter.create(fieldPath, operator, fieldValue);
const filter = FieldFilter.create(fieldPath, op, fieldValue);
this.validateNewFilter(filter);
return new Query(
this._query.addFilter(filter),
Expand Down
90 changes: 21 additions & 69 deletions packages/firestore/src/core/query.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,28 +80,27 @@ export class Query {

get orderBy(): OrderBy[] {
if (this.memoizedOrderBy === null) {
this.memoizedOrderBy = [];

const inequalityField = this.getInequalityFilterField();
const firstOrderByField = this.getFirstOrderByField();
if (inequalityField !== null && firstOrderByField === null) {
// In order to implicitly add key ordering, we must also add the
// inequality filter field for it to be a valid query.
// Note that the default inequality field and key ordering is ascending.
if (inequalityField.isKeyField()) {
this.memoizedOrderBy = [KEY_ORDERING_ASC];
} else {
this.memoizedOrderBy = [
new OrderBy(inequalityField),
KEY_ORDERING_ASC
];
if (!inequalityField.isKeyField()) {
this.memoizedOrderBy.push(new OrderBy(inequalityField));
}
this.memoizedOrderBy.push(
new OrderBy(FieldPath.keyField(), Direction.ASCENDING)
);
} else {
debugAssert(
inequalityField === null ||
(firstOrderByField !== null &&
inequalityField.isEqual(firstOrderByField)),
'First orderBy should match inequality field.'
);
this.memoizedOrderBy = [];
let foundKeyOrdering = false;
for (const orderBy of this.explicitOrderBy) {
this.memoizedOrderBy.push(orderBy);
Expand All @@ -117,9 +116,7 @@ export class Query {
? this.explicitOrderBy[this.explicitOrderBy.length - 1].dir
: Direction.ASCENDING;
this.memoizedOrderBy.push(
lastDirection === Direction.ASCENDING
? KEY_ORDERING_ASC
: KEY_ORDERING_DESC
new OrderBy(FieldPath.keyField(), lastDirection)
);
}
}
Expand Down Expand Up @@ -468,48 +465,15 @@ export abstract class Filter {
abstract isEqual(filter: Filter): boolean;
}

export class Operator {
static LESS_THAN = new Operator('<');
static LESS_THAN_OR_EQUAL = new Operator('<=');
static EQUAL = new Operator('==');
static GREATER_THAN = new Operator('>');
static GREATER_THAN_OR_EQUAL = new Operator('>=');
static ARRAY_CONTAINS = new Operator('array-contains');
static IN = new Operator('in');
static ARRAY_CONTAINS_ANY = new Operator('array-contains-any');

static fromString(op: string): Operator {
switch (op) {
case '<':
return Operator.LESS_THAN;
case '<=':
return Operator.LESS_THAN_OR_EQUAL;
case '==':
return Operator.EQUAL;
case '>=':
return Operator.GREATER_THAN_OR_EQUAL;
case '>':
return Operator.GREATER_THAN;
case 'array-contains':
return Operator.ARRAY_CONTAINS;
case 'in':
return Operator.IN;
case 'array-contains-any':
return Operator.ARRAY_CONTAINS_ANY;
default:
return fail('Unknown FieldFilter operator: ' + op);
}
}

constructor(public name: string) {}

toString(): string {
return this.name;
}

isEqual(other: Operator): boolean {
return this.name === other.name;
}
export const enum Operator {
LESS_THAN = '<',
LESS_THAN_OR_EQUAL = '<=',
EQUAL = '==',
GREATER_THAN = '>',
GREATER_THAN_OR_EQUAL = '>=',
ARRAY_CONTAINS = 'array-contains',
IN = 'in',
ARRAY_CONTAINS_ANY = 'array-contains-any'
}

export class FieldFilter extends Filter {
Expand Down Expand Up @@ -635,7 +599,7 @@ export class FieldFilter extends Filter {
isEqual(other: Filter): boolean {
if (other instanceof FieldFilter) {
return (
this.op.isEqual(other.op) &&
this.op === other.op &&
this.field.isEqual(other.field) &&
valueEquals(this.value, other.value)
);
Expand Down Expand Up @@ -737,15 +701,9 @@ export class ArrayContainsAnyFilter extends FieldFilter {
/**
* The direction of sorting in an order by.
*/
export class Direction {
static ASCENDING = new Direction('asc');
static DESCENDING = new Direction('desc');

private constructor(public name: string) {}

toString(): string {
return this.name;
}
export const enum Direction {
ASCENDING = 'asc',
DESCENDING = 'desc'
}

/**
Expand Down Expand Up @@ -875,9 +833,3 @@ export class OrderBy {
return this.dir === other.dir && this.field.isEqual(other.field);
}
}

const KEY_ORDERING_ASC = new OrderBy(FieldPath.keyField(), Direction.ASCENDING);
const KEY_ORDERING_DESC = new OrderBy(
FieldPath.keyField(),
Direction.DESCENDING
);
6 changes: 2 additions & 4 deletions packages/firestore/src/core/snapshot_version.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,12 @@ import { Timestamp } from '../api/timestamp';
* timestamp, such as update_time or read_time.
*/
export class SnapshotVersion {
static readonly MIN = new SnapshotVersion(new Timestamp(0, 0));

static fromTimestamp(value: Timestamp): SnapshotVersion {
return new SnapshotVersion(value);
}

static forDeletedDoc(): SnapshotVersion {
return SnapshotVersion.MIN;
static min(): SnapshotVersion {
return new SnapshotVersion(new Timestamp(0, 0));
}

private constructor(private timestamp: Timestamp) {}
Expand Down
8 changes: 4 additions & 4 deletions packages/firestore/src/core/sync_engine.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import {
import { MaybeDocument, NoDocument } from '../model/document';
import { DocumentKey } from '../model/document_key';
import { Mutation } from '../model/mutation';
import { MutationBatchResult, BATCHID_UNKNOWN } from '../model/mutation_batch';
import { BATCHID_UNKNOWN, MutationBatchResult } from '../model/mutation_batch';
import { RemoteEvent, TargetChange } from '../remote/remote_event';
import { RemoteStore } from '../remote/remote_store';
import { RemoteSyncer } from '../remote/remote_syncer';
Expand All @@ -52,7 +52,7 @@ import {
} from '../local/shared_client_state_syncer';
import { SortedSet } from '../util/sorted_set';
import { ListenSequence } from './listen_sequence';
import { Query, LimitType } from './query';
import { LimitType, Query } from './query';
import { SnapshotVersion } from './snapshot_version';
import { Target } from './target';
import { TargetIdGenerator } from './target_id_generator';
Expand Down Expand Up @@ -504,11 +504,11 @@ export class SyncEngine implements RemoteSyncer {
);
documentUpdates = documentUpdates.insert(
limboKey,
new NoDocument(limboKey, SnapshotVersion.forDeletedDoc())
new NoDocument(limboKey, SnapshotVersion.min())
);
const resolvedLimboDocuments = documentKeySet().add(limboKey);
const event = new RemoteEvent(
SnapshotVersion.MIN,
SnapshotVersion.min(),
/* targetChanges= */ new Map<TargetId, TargetChange>(),
/* targetMismatches= */ new SortedSet<TargetId>(primitiveComparator),
documentUpdates,
Expand Down
10 changes: 5 additions & 5 deletions packages/firestore/src/core/transaction.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

import { ParsedSetData, ParsedUpdateData } from '../api/user_data_reader';
import { documentVersionMap } from '../model/collections';
import { Document, NoDocument, MaybeDocument } from '../model/document';
import { Document, MaybeDocument, NoDocument } from '../model/document';

import { DocumentKey } from '../model/document_key';
import {
Expand All @@ -27,7 +27,7 @@ import {
VerifyMutation
} from '../model/mutation';
import { Datastore } from '../remote/datastore';
import { fail, debugAssert } from '../util/assert';
import { debugAssert, fail } from '../util/assert';
import { Code, FirestoreError } from '../util/error';
import { SnapshotVersion } from './snapshot_version';

Expand Down Expand Up @@ -123,7 +123,7 @@ export class Transaction {
docVersion = doc.version;
} else if (doc instanceof NoDocument) {
// For deleted docs, we must use baseVersion 0 when we overwrite them.
docVersion = SnapshotVersion.forDeletedDoc();
docVersion = SnapshotVersion.min();
} else {
throw fail('Document in a transaction was a ' + doc.constructor.name);
}
Expand Down Expand Up @@ -151,7 +151,7 @@ export class Transaction {
if (!this.writtenDocs.has(key) && version) {
return Precondition.updateTime(version);
} else {
return Precondition.NONE;
return Precondition.none();
}
}

Expand All @@ -163,7 +163,7 @@ export class Transaction {
// The first time a document is written, we want to take into account the
// read time and existence
if (!this.writtenDocs.has(key) && version) {
if (version.isEqual(SnapshotVersion.forDeletedDoc())) {
if (version.isEqual(SnapshotVersion.min())) {
// The document doesn't exist, so fail the transaction.

// This has to be validated locally because you can't send a
Expand Down
4 changes: 2 additions & 2 deletions packages/firestore/src/local/index_free_query_engine.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ export class IndexFreeQueryEngine implements QueryEngine {

// Queries that have never seen a snapshot without limbo free documents
// should also be run as a full collection scan.
if (lastLimboFreeSnapshotVersion.isEqual(SnapshotVersion.MIN)) {
if (lastLimboFreeSnapshotVersion.isEqual(SnapshotVersion.min())) {
return this.executeFullCollectionScan(transaction, query);
}

Expand Down Expand Up @@ -204,7 +204,7 @@ export class IndexFreeQueryEngine implements QueryEngine {
return this.localDocumentsView!.getDocumentsMatchingQuery(
transaction,
query,
SnapshotVersion.MIN
SnapshotVersion.min()
);
}
}
Loading

0 comments on commit 4ad5733

Please sign in to comment.