fix: make willCommit slightly safer when race conditions occur

ember-data-types/q/ds-model.ts

@@ -33,7 +33,7 @@ export interface ModelSchema {
   relationshipsByName: Map<string, RelationshipSchema>;
   eachAttribute<T>(callback: (this: T, key: string, attribute: AttributeSchema) => void, binding?: T): void;
   eachRelationship<T>(callback: (this: T, key: string, relationship: RelationshipSchema) => void, binding?: T): void;
-  eachTransformedAttribute<T>(callback: (this: T, key: string, type?: string) => void, binding?: T): void;
+  eachTransformedAttribute<T>(callback: (this: T, key: string, type: string | null) => void, binding?: T): void;
   toString(): string;
 }
 

packages/json-api/src/-private/cache.ts

@@ -670,6 +670,40 @@ export default class JSONAPICache implements Cache {
    */
   willCommit(identifier: StableRecordIdentifier): void {
     const cached = this.__peek(identifier, false);
+
+    /*
+      if we have multiple saves in flight at once then
+      we have information loss no matter what. This
+      attempts to lose the least information.
+
+      If we were to clear inflightAttrs, previous requests
+      would not be able to use it during their didCommit.
+
+      If we upsert inflightattrs, previous requests incorrectly
+      see more recent inflight changes as part of their own and
+      will incorrectly mark the new state as the correct remote state.
+
+      We choose this latter behavior to avoid accidentally removing
+      earlier changes.
+
+      If apps do not want this behavior they can either
+
+      - chain save requests serially vs allowing concurrent saves
+      - move to using a request handler that caches the inflight state
+        on a per-request basis
+      - change their save requests to only send a "PATCH" instead of a "PUT"
+        so that only latest changes are involved in each request, and then also
+        ensure that the API or their handler reflects only those changes back
+        for upsert into the cache.
+    */
+    if (cached.inflightAttrs) {
+      if (!cached.localAttrs) {
+        return;
+      }
+      Object.assign(cached.inflightAttrs, cached.localAttrs);
+      cached.localAttrs = null;
+      return;
+    }
     cached.inflightAttrs = cached.localAttrs;
     cached.localAttrs = null;
   }

packages/legacy-compat/src/legacy-network-handler/legacy-network-handler.ts

@@ -8,9 +8,9 @@ import { DEBUG, TESTING } from '@ember-data/env';
 import type { Handler, NextFn } from '@ember-data/request/-private/types';
 import type Store from '@ember-data/store';
 import type { StoreRequestContext, StoreRequestInfo } from '@ember-data/store/-private/cache-handler';
-import type ShimModelClass from '@ember-data/store/-private/legacy-model-support/shim-model-class';
 import type { Collection } from '@ember-data/store/-private/record-arrays/identifier-array';
 import { SingleResourceDataDocument } from '@ember-data/types/cache/document';
+import type { ModelSchema } from '@ember-data/types/q/ds-model';
 import type {
   CollectionResourceDocument,
   JsonApiDocument,
@@ -35,7 +35,7 @@ import SnapshotRecordArray from './snapshot-record-array';
 
 type AdapterErrors = Error & { errors?: unknown[]; isAdapterError?: true; code?: string };
 type SerializerWithParseErrors = MinimumSerializerInterface & {
-  extractErrors?(store: Store, modelClass: ShimModelClass, error: AdapterErrors, recordId: string | null): unknown;
+  extractErrors?(store: Store, modelClass: ModelSchema, error: AdapterErrors, recordId: string | null): unknown;
 };
 
 const PotentialLegacyOperations = new Set([

packages/legacy-compat/src/legacy-network-handler/serializer-response.ts

@@ -2,7 +2,7 @@ import { assert } from '@ember/debug';
 
 import { DEBUG } from '@ember-data/env';
 import type Store from '@ember-data/store';
-import type ShimModelClass from '@ember-data/store/-private/legacy-model-support/shim-model-class';
+import type { ModelSchema } from '@ember-data/types/q/ds-model';
 import type { JsonApiDocument } from '@ember-data/types/q/ember-data-json-api';
 import type { AdapterPayload } from '@ember-data/types/q/minimum-adapter-interface';
 import type { MinimumSerializerInterface, RequestType } from '@ember-data/types/q/minimum-serializer-interface';
@@ -70,7 +70,7 @@ function validateDocumentStructure(doc?: AdapterPayload | JsonApiDocument): asse
 export function normalizeResponseHelper(
   serializer: MinimumSerializerInterface | null,
   store: Store,
-  modelClass: ShimModelClass,
+  modelClass: ModelSchema,
   payload: AdapterPayload,
   id: string | null,
   requestType: RequestType

packages/legacy-compat/src/legacy-network-handler/snapshot.ts

@@ -32,7 +32,7 @@ type RecordId = string | null;
   @class Snapshot
   @public
 */
-export default class Snapshot implements Snapshot {
+export default class Snapshot {
   declare __attributes: Dict<unknown> | null;
   declare _belongsToRelationships: Dict<Snapshot>;
   declare _belongsToIds: Dict<RecordId>;

packages/model/src/-private/model.d.ts

@@ -0,0 +1,82 @@
+import type EmberObject from '@ember/object';
+
+import type { Errors } from '@ember-data/model/-private';
+import type Store from '@ember-data/store';
+
+import type { AttributeSchema, RelationshipSchema, RelationshipsSchema } from '@ember-data/types/q/record-data-schemas';
+import type { JsonApiError } from '@ember-data/types/q/record-data-json-api';
+import type HasManyReference from './references/has-many';
+import type BelongsToReference from './references/belongs-to';
+import type { StableRecordIdentifier } from '@ember-data/types/q/identifier';
+import type { LegacySupport } from './legacy-relationships-support';
+import type { Cache } from '@ember-data/types/q/cache';
+import type RecordState from './record-state';
+
+export type ModelCreateArgs = {
+  _createProps: Record<string, unknown>;
+  // TODO @deprecate consider deprecating accessing record properties during init which the below is necessary for
+  _secretInit: {
+    identifier: StableRecordIdentifier;
+    cache: Cache;
+    store: Store;
+    cb: (record: Model, cache: Cache, identifier: StableRecordIdentifier, store: Store) => void;
+  };
+};
+
+
+class Model extends EmberObject {
+  store: Store;
+  errors: Errors;
+  currentState: RecordState;
+  adapterError?: Error;
+  toString(): string;
+  save(): Promise<this>;
+  hasMany(key: string): HasManyReference;
+  belongsTo(key: string): BelongsToReference
+  eachRelationship<T>(callback: (this: T, key: string, meta: RelationshipSchema) => void, binding?: T): void;
+  eachAttribute<T>(callback: (this: T, key: string, meta: AttributeSchema) => void, binding?: T): void;
+  invalidErrorsChanged(errors: JsonApiError[]): void;
+  rollbackAttributes(): void;
+  changedAttributes(): Record<string, [unknown, unknown]>;
+  [key: string]: unknown;
+  isSaving: boolean;
+  isNew: boolean;
+  isDeleted: boolean;
+  hasDirtyAttributes: boolean;
+  deleteRecord(): void;
+  unloadRecord(): void;
+  serialize(): Record<string, unknown>;
+
+  static modelName: string;
+  static fields: Map<string, 'attribute' | 'belongsTo' | 'hasMany'>;
+  static attributes: Map<string, AttributeSchema>;
+  static relationshipsByName: Map<string, RelationshipSchema>;
+  static eachAttribute<T>(callback: (this: T, key: string, attribute: AttributeSchema) => void, binding?: T): void;
+  static eachRelationship<T>(callback: (this: T, key: string, relationship: RelationshipSchema) => void, binding?: T): void;
+  static eachTransformedAttribute<T>(callback: (this: T, key: string, type: string | null) => void, binding?: T): void;
+
+  static toString(): string;
+  static isModel: true;
+  static relationshipsObject: RelationshipsSchema;
+  static extend(...mixins: unknown[]): typeof Model;
+  static reopenClass(...mixins: unknown[]): void;
+  static create(createArgs: ModelCreateArgs): Model;
+  static __isMixin?: true;
+  static __mixin?: unknown;
+}
+
+interface Model {
+  constructor: typeof Model;
+}
+
+export default Model;
+
+export type StaticModel = typeof Model;
+
+export const LEGACY_SUPPORT: Map<StableRecordIdentifier | Model, LegacySupport>;
+
+export type ModelFactory = { class: StaticModel };
+export type FactoryCache = Record<string, ModelFactory>;
+// we put this on the store for interop because it's used by modelFor and
+// instantiateRecord as well.
+export type ModelStore = Store & { _modelFactoryCache: FactoryCache };

packages/model/src/-private/record-state.ts

@@ -13,7 +13,7 @@ import { addToTransaction, subscribe } from '@ember-data/tracking/-private';
 import type { Cache } from '@ember-data/types/q/cache';
 import type { StableRecordIdentifier } from '@ember-data/types/q/identifier';
 
-type Model = InstanceType<typeof import('./model')>;
+import Model from './model';
 
 const SOURCE_POINTER_REGEXP = /^\/?data\/(attributes|relationships)\/(.*)/;
 const SOURCE_POINTER_PRIMARY_REGEXP = /^\/?data/;

packages/store/src/-private/legacy-model-support/schema-definition-service.ts

@@ -5,6 +5,7 @@ import { importSync } from '@embroider/macros';
 
 import { DEPRECATE_STRING_ARG_SCHEMAS } from '@ember-data/deprecations';
 import type Model from '@ember-data/model';
+import type { ModelFactory } from '@ember-data/model/-private/model';
 import { HAS_MODEL_PACKAGE } from '@ember-data/packages';
 import type { RecordIdentifier } from '@ember-data/types/q/identifier';
 import type { AttributesSchema, RelationshipsSchema } from '@ember-data/types/q/record-data-schemas';
@@ -99,7 +100,7 @@ export class DSModelSchemaDefinitionService {
     relationships = this._relationshipsDefCache[modelName];
 
     if (relationships === undefined) {
-      let modelClass = this.store.modelFor(modelName);
+      let modelClass = this.store.modelFor(modelName) as typeof Model;
       relationships = modelClass.relationshipsObject || null;
       this._relationshipsDefCache[modelName] = relationships;
     }
@@ -115,7 +116,7 @@ export class DSModelSchemaDefinitionService {
   }
 }
 
-export function getModelFactory(store: Store, cache, normalizedModelName: string): Model | null {
+export function getModelFactory(store: Store, cache, normalizedModelName: string): ModelFactory | null {
   let factory = cache[normalizedModelName];
 
   if (!factory) {

packages/store/src/-private/legacy-model-support/shim-model-class.ts

@@ -81,11 +81,11 @@ export default class ShimModelClass implements ModelSchema {
     });
   }
 
-  eachTransformedAttribute<T>(callback: (this: T | undefined, key: string, type?: string) => void, binding?: T) {
+  eachTransformedAttribute<T>(callback: (this: T | undefined, key: string, type: string | null) => void, binding?: T) {
     const attrDefs = this.__store.getSchemaDefinitionService().attributesDefinitionFor({ type: this.modelName });
     Object.keys(attrDefs).forEach((key) => {
       if (attrDefs[key]!.type) {
-        callback.call(binding, key, attrDefs[key]!.type);
+        callback.call(binding, key, attrDefs[key]?.type ?? null);
       }
     });
   }

packages/store/src/-private/store-service.ts

@@ -19,7 +19,7 @@ import {
 import { DEBUG, TESTING } from '@ember-data/env';
 import type CacheClass from '@ember-data/json-api';
 import type FetchManager from '@ember-data/legacy-compat/legacy-network-handler/fetch-manager';
-import type DSModelClass from '@ember-data/model';
+import type Model from '@ember-data/model';
 import { HAS_COMPAT_PACKAGE, HAS_GRAPH_PACKAGE, HAS_JSON_API_PACKAGE, HAS_MODEL_PACKAGE } from '@ember-data/packages';
 import type RequestManager from '@ember-data/request';
 import type { Future } from '@ember-data/request/-private/types';
@@ -78,6 +78,8 @@ import normalizeModelName from './utils/normalize-model-name';
 
 export { storeFor };
 
+type StaticModel = typeof Model;
+
 // hello world
 type CacheConstruct = typeof CacheClass;
 let _Cache: CacheConstruct | undefined;
@@ -471,7 +473,10 @@ class Store extends EmberObject {
 
       // ensure that `getOwner(this)` works inside a model instance
       setOwner(createOptions, getOwner(this)!);
-      return getModelFactory(this, this._modelFactoryCache, modelName).class.create(createOptions);
+      const factory = getModelFactory(this, this._modelFactoryCache, modelName);
+
+      assert(`No model was found for '${modelName}'`, factory);
+      return factory.class.create(createOptions);
     }
     assert(`You must implement the store's instantiateRecord hook for your custom model class.`);
   }
@@ -656,7 +661,7 @@ class Store extends EmberObject {
     */
   // TODO @deprecate in favor of schema APIs, requires adapter/serializer overhaul or replacement
 
-  modelFor(modelName: string): ShimModelClass | DSModelClass {
+  modelFor(modelName: string): ShimModelClass | StaticModel {
     if (DEBUG) {
       assertDestroyedStoreOnly(this, 'modelFor');
     }
@@ -670,7 +675,8 @@ class Store extends EmberObject {
       let maybeFactory = getModelFactory(this, this._modelFactoryCache, normalizedModelName);
 
       // for factorFor factory/class split
-      let klass = maybeFactory && maybeFactory.class ? maybeFactory.class : maybeFactory;
+      const klass = maybeFactory && maybeFactory.class ? maybeFactory.class : null;
+
       if (!klass || !klass.isModel || this._forceShim) {
         assert(
           `No model was found for '${modelName}' and no schema handles the type`,