diff --git a/client/packages/cli/src/renderSchemaPlan.ts b/client/packages/cli/src/renderSchemaPlan.ts index ff28408564..e64af3193e 100644 --- a/client/packages/cli/src/renderSchemaPlan.ts +++ b/client/packages/cli/src/renderSchemaPlan.ts @@ -51,25 +51,40 @@ const renderLinkUpdate = ( reverseLabel = ` <-> ${oldAttr['reverse-identity']![1]}.${oldAttr['reverse-identity']![2]}`; - if (!oldAttr['on-delete'] && tx.partialAttr['on-delete']) { - details.push( - `(SET ON DELETE ${oldAttr['reverse-identity']![1]} CASCADE DELETE ${oldAttr['forward-identity'][1]})`, - ); - } - if (!oldAttr['on-delete-reverse'] && tx.partialAttr['on-delete-reverse']) { - details.push( - `(SET ON DELETE ${oldAttr['forward-identity']![1]} CASCADE DELETE ${oldAttr['reverse-identity']![1]})`, - ); - } - if (oldAttr['on-delete'] && !oldAttr['on-delete-reverse']) { - details.push( - `(REMOVE CASCADE DELETE ON ${oldAttr['reverse-identity']![1]})`, - ); + const prevOnDelete = oldAttr['on-delete']; + const newOnDelete = tx.partialAttr['on-delete']; + + if (prevOnDelete != newOnDelete) { + if (prevOnDelete) { + const action = prevOnDelete.toUpperCase(); + details.push( + `(REMOVE ${action} DELETE ON ${oldAttr['reverse-identity']![1]})`, + ); + } + if (newOnDelete) { + const action = newOnDelete.toUpperCase(); + details.push( + `(SET ON DELETE ${oldAttr['reverse-identity']![1]} ${action} DELETE ${oldAttr['forward-identity'][1]})`, + ); + } } - if (oldAttr['on-delete-reverse'] && !oldAttr['on-delete-reverse']) { - details.push( - `(REMOVE REVERSE CASCADE DELETE ON ${oldAttr['forward-identity'][1]})`, - ); + + const prevOnDeleteReverse = oldAttr['on-delete-reverse']; + const newOnDeleteReverse = tx.partialAttr['on-delete-reverse']; + + if (prevOnDeleteReverse != newOnDeleteReverse) { + if (prevOnDeleteReverse) { + const action = prevOnDeleteReverse.toUpperCase(); + details.push( + `(REMOVE REVERSE ${action} DELETE ON ${oldAttr['reverse-identity']![1]})`, + ); + } + if (newOnDeleteReverse) { + const action = newOnDeleteReverse.toUpperCase(); + details.push( + `(SET ON DELETE ${oldAttr['forward-identity']![1]} ${action} DELETE ${oldAttr['reverse-identity']![1]})`, + ); + } } } diff --git a/client/packages/components/src/components/explorer/edit-namespace-dialog.tsx b/client/packages/components/src/components/explorer/edit-namespace-dialog.tsx index 10eea892c8..70ea12d75d 100644 --- a/client/packages/components/src/components/explorer/edit-namespace-dialog.tsx +++ b/client/packages/components/src/components/explorer/edit-namespace-dialog.tsx @@ -34,6 +34,7 @@ import { ToggleGroup, } from '@lib/components/ui'; import { + OnDelete, RelationshipKinds, relationshipConstraints, relationshipConstraintsInverse, @@ -360,8 +361,8 @@ function AddAttrForm({ const [isRequired, setIsRequired] = useState(false); const [isIndex, setIsIndex] = useState(false); const [isUniq, setIsUniq] = useState(false); - const [isCascade, setIsCascade] = useState(false); - const [isCascadeReverse, setIsCascadeReverse] = useState(false); + const [onDelete, setOnDelete] = useState(null); + const [onDeleteReverse, setOnDeleteReverse] = useState(null); const [checkedDataType, setCheckedDataType] = useState(null); const [attrType, setAttrType] = useState<'blob' | 'ref'>('blob'); @@ -374,9 +375,9 @@ function AddAttrForm({ const [attrName, setAttrName] = useState(''); const [reverseAttrName, setReverseAttrName] = useState(namespace.name); - const isCascadeAllowed = + const isOnDeleteAllowed = relationship === 'one-one' || relationship === 'one-many'; - const isCascadeReverseAllowed = + const isOnDeleteReverseAllowed = relationship === 'one-one' || relationship === 'many-one'; const linkValidation = validateLink({ @@ -428,9 +429,10 @@ function AddAttrForm({ 'value-type': 'ref', 'index?': false, 'required?': isRequired, - 'on-delete': isCascadeAllowed && isCascade ? 'cascade' : undefined, - 'on-delete-reverse': - isCascadeReverseAllowed && isCascadeReverse ? 'cascade' : undefined, + 'on-delete': isOnDeleteAllowed ? onDelete : undefined, + 'on-delete-reverse': isOnDeleteReverseAllowed + ? onDeleteReverse + : undefined, }; const ops = [['add-attr', attr]]; @@ -578,12 +580,12 @@ function AddAttrForm({ setAttrName={setAttrName} setReverseAttrName={setReverseAttrName} setRelationship={setRelationship} - isCascadeAllowed={isCascadeAllowed} - isCascade={isCascade} - setIsCascade={setIsCascade} - isCascadeReverseAllowed={isCascadeReverseAllowed} - isCascadeReverse={isCascadeReverse} - setIsCascadeReverse={setIsCascadeReverse} + isOnDeleteAllowed={isOnDeleteAllowed} + onDelete={onDelete} + setOnDelete={setOnDelete} + isOnDeleteReverseAllowed={isOnDeleteReverseAllowed} + onDeleteReverse={onDeleteReverse} + setOnDeleteReverse={setOnDeleteReverse} isRequired={isRequired} setIsRequired={setIsRequired} constraints={constraints} @@ -834,12 +836,12 @@ function RelationshipConfigurator({ setAttrName, setReverseAttrName, setRelationship, - isCascade, - setIsCascade, - isCascadeAllowed, - isCascadeReverse, - setIsCascadeReverse, - isCascadeReverseAllowed, + onDelete, + setOnDelete, + isOnDeleteAllowed, + onDeleteReverse, + setOnDeleteReverse, + isOnDeleteReverseAllowed, isRequired, setIsRequired, constraints, @@ -854,13 +856,13 @@ function RelationshipConfigurator({ setReverseAttrName: (n: string) => void; setRelationship: (n: RelationshipKinds) => void; - isCascadeAllowed: boolean; - isCascade: boolean; - setIsCascade: (n: boolean) => void; + isOnDeleteAllowed: boolean; + onDelete: OnDelete; + setOnDelete: (n: OnDelete) => void; - isCascadeReverseAllowed: boolean; - isCascadeReverse: boolean; - setIsCascadeReverse: (n: boolean) => void; + isOnDeleteReverseAllowed: boolean; + onDeleteReverse: OnDelete; + setOnDeleteReverse: (n: OnDelete) => void; isRequired: boolean; setIsRequired: (n: boolean) => void; @@ -952,9 +954,11 @@ function RelationshipConfigurator({
+ setOnDelete(onDelete === 'cascade' ? null : 'cascade') + } title={constraints.attr.message} label={ @@ -970,12 +974,36 @@ function RelationshipConfigurator({ } />
+
+ + setOnDelete(onDelete === 'restrict' ? null : 'restrict') + } + title={constraints.attr.message} + label={ + +
+ + Restrict Delete {reverseNamespaceName} → {namespaceName} + +
+ When a {reverseNamespaceName} entity is deleted, + the transaction will be blocked if all linked{' '} + {namespaceName} are not deleted or unlinked + + } + /> +
+ setOnDeleteReverse(onDeleteReverse === 'cascade' ? null : 'cascade') + } title={constraints.attr.message} label={ @@ -992,6 +1020,32 @@ function RelationshipConfigurator({ />
+
+ + setOnDeleteReverse( + onDeleteReverse === 'restrict' ? null : 'restrict', + ) + } + title={constraints.attr.message} + label={ + +
+ + Restrict Delete {namespaceName} → {reverseNamespaceName} + +
+ When a {namespaceName} entity is deleted, the + transaction will be blocked if all linked{' '} + {reverseNamespaceName} are not deleted or + unlinked + + } + /> +
+
Constraints
@@ -1578,10 +1632,10 @@ function EditAttrForm({ const explorerProps = useExplorerProps(); - const [isCascade, setIsCascade] = useState(() => attr.onDelete === 'cascade'); + const [onDelete, setOnDelete] = useState(() => attr.onDelete); - const [isCascadeReverse, setIsCascadeReverse] = useState( - () => attr.onDeleteReverse === 'cascade', + const [onDeleteReverse, setOnDeleteReverse] = useState( + () => attr.onDeleteReverse, ); const [isRequired, setIsRequired] = useState(attr.isRequired || false); @@ -1598,9 +1652,9 @@ function EditAttrForm({ return () => stopFetchLoop.current?.(); }, [stopFetchLoop]); - const isCascadeAllowed = + const isOnDeleteAllowed = relationship === 'one-one' || relationship === 'one-many'; - const isCascadeReverseAllowed = + const isOnDeleteReverseAllowed = relationship === 'one-one' || relationship === 'many-one'; const linkValidation = validateLink({ @@ -1645,9 +1699,10 @@ function EditAttrForm({ attr.linkConfig.reverse.namespace, reverseAttrName, ], - 'on-delete': isCascadeAllowed && isCascade ? 'cascade' : null, - 'on-delete-reverse': - isCascadeReverseAllowed && isCascadeReverse ? 'cascade' : null, + 'on-delete': isOnDeleteAllowed ? onDelete : null, + 'on-delete-reverse': isOnDeleteReverseAllowed + ? onDeleteReverse + : null, }, ], ]; @@ -1772,12 +1827,12 @@ function EditAttrForm({ setAttrName={setAttrName} setReverseAttrName={setReverseAttrName} setRelationship={setRelationship} - isCascadeAllowed={isCascadeAllowed} - isCascade={isCascade} - setIsCascade={setIsCascade} - isCascadeReverseAllowed={isCascadeReverseAllowed} - isCascadeReverse={isCascadeReverse} - setIsCascadeReverse={setIsCascadeReverse} + isOnDeleteAllowed={isOnDeleteAllowed} + onDelete={onDelete} + setOnDelete={setOnDelete} + isOnDeleteReverseAllowed={isOnDeleteReverseAllowed} + onDeleteReverse={onDeleteReverse} + setOnDeleteReverse={setOnDeleteReverse} isRequired={isRequired} setIsRequired={setIsRequired} constraints={constraints} diff --git a/client/packages/components/src/types.ts b/client/packages/components/src/types.ts index 69ed76f369..933f29cd6d 100644 --- a/client/packages/components/src/types.ts +++ b/client/packages/components/src/types.ts @@ -222,6 +222,8 @@ export type DBIdent = export type CheckedDataType = 'string' | 'number' | 'boolean' | 'date'; +export type OnDelete = 'cascade' | 'restrict' | null | undefined; + export interface DBAttr { id: string; 'forward-identity': DBIdent; @@ -235,8 +237,8 @@ export interface DBAttr { 'inferred-types'?: Array<'string' | 'number' | 'boolean' | 'json'>; catalog?: 'user' | 'system'; 'checked-data-type'?: CheckedDataType; - 'on-delete'?: 'cascade'; - 'on-delete-reverse'?: 'cascade'; + 'on-delete'?: OnDelete; + 'on-delete-reverse'?: OnDelete; metadata?: any; } diff --git a/client/packages/core/src/attrTypes.ts b/client/packages/core/src/attrTypes.ts index 115a92d352..830176c53f 100644 --- a/client/packages/core/src/attrTypes.ts +++ b/client/packages/core/src/attrTypes.ts @@ -8,7 +8,7 @@ export type InstantDBInferredType = 'number' | 'string' | 'boolean' | 'json'; export type InstantDBCheckedDataType = 'number' | 'string' | 'boolean' | 'date'; -export type InstantDBAttrOnDelete = 'cascade'; +export type InstantDBAttrOnDelete = 'cascade' | 'restrict'; export type InstantDBAttr = { id: string; diff --git a/client/packages/core/src/schemaTypes.ts b/client/packages/core/src/schemaTypes.ts index d6c7aa03d9..e0afd71816 100644 --- a/client/packages/core/src/schemaTypes.ts +++ b/client/packages/core/src/schemaTypes.ts @@ -164,13 +164,13 @@ export type LinkDef< label: FwdAttr; has: FwdCardinality; required?: RequirementKind; - onDelete?: 'cascade'; + onDelete?: 'cascade' | 'restrict'; }; reverse: { on: RevEntity; label: RevAttr; has: RevCardinality; - onDelete?: 'cascade'; + onDelete?: 'cascade' | 'restrict'; }; }; diff --git a/client/packages/platform/__tests__/src/migrations.test.ts b/client/packages/platform/__tests__/src/migrations.test.ts index fd1dfd5317..2a29345080 100644 --- a/client/packages/platform/__tests__/src/migrations.test.ts +++ b/client/packages/platform/__tests__/src/migrations.test.ts @@ -484,6 +484,105 @@ test('update link delete cascade', async () => { expect((result[0] as any).partialAttr['on-delete']).toBe('cascade'); }); +test('update link delete restrict', async () => { + const result = await diffSchemas( + i.schema({ + entities: { + albums: i.entity({ + name: i.string(), + }), + songs: i.entity({ + name: i.string(), + }), + }, + links: { + songAlbum: { + forward: { on: 'albums', has: 'one', label: 'songs' }, + reverse: { on: 'songs', has: 'one', label: 'albums' }, + }, + }, + }), + i.schema({ + entities: { + albums: i.entity({ + name: i.string(), + }), + songs: i.entity({ + name: i.string(), + }), + }, + links: { + songAlbum: { + forward: { + on: 'albums', + has: 'one', + onDelete: 'restrict', + label: 'songs', + }, + reverse: { on: 'songs', has: 'one', label: 'albums' }, + }, + }, + }), + createChooser([]), + systemCatalogIdentNames, + ); + console.log(result); + expectTxType(result, 'update-attr', 1); + expect((result[0] as any).partialAttr['on-delete']).toBe('restrict'); +}); + +test('update link delete cascade to restrict', async () => { + const result = await diffSchemas( + i.schema({ + entities: { + albums: i.entity({ + name: i.string(), + }), + songs: i.entity({ + name: i.string(), + }), + }, + links: { + songAlbum: { + forward: { + on: 'albums', + has: 'one', + onDelete: 'cascade', + label: 'songs', + }, + reverse: { on: 'songs', has: 'one', label: 'albums' }, + }, + }, + }), + i.schema({ + entities: { + albums: i.entity({ + name: i.string(), + }), + songs: i.entity({ + name: i.string(), + }), + }, + links: { + songAlbum: { + forward: { + on: 'albums', + has: 'one', + onDelete: 'restrict', + label: 'songs', + }, + reverse: { on: 'songs', has: 'one', label: 'albums' }, + }, + }, + }), + createChooser([]), + systemCatalogIdentNames, + ); + console.log(result); + expectTxType(result, 'update-attr', 1); + expect((result[0] as any).partialAttr['on-delete']).toBe('restrict'); +}); + test('system catalog attrs are ignored when adding entities', async () => { const result = await diffSchemas( i.schema({ diff --git a/client/packages/platform/src/api.ts b/client/packages/platform/src/api.ts index b59cc8c510..5a0ce8084a 100644 --- a/client/packages/platform/src/api.ts +++ b/client/packages/platform/src/api.ts @@ -420,7 +420,11 @@ function apiSchemaAttrToLinkDef(attr: InstantDBAttr) { label: flabel, required: attr['required?'] || undefined, onDelete: - attr['on-delete'] === 'cascade' ? ('cascade' as 'cascade') : undefined, + attr['on-delete'] === 'cascade' + ? ('cascade' as 'cascade') + : attr['on-delete'] === 'restrict' + ? ('restrict' as 'restrict') + : undefined, }, reverse: { on: re, @@ -429,7 +433,9 @@ function apiSchemaAttrToLinkDef(attr: InstantDBAttr) { onDelete: attr['on-delete-reverse'] === 'cascade' ? ('cascade' as 'cascade') - : undefined, + : attr['on-delete-reverse'] === 'restrict' + ? ('restrict' as 'restrict') + : undefined, }, }; } diff --git a/client/packages/platform/src/schema.ts b/client/packages/platform/src/schema.ts index 2a7b03285e..be9c83c1f1 100644 --- a/client/packages/platform/src/schema.ts +++ b/client/packages/platform/src/schema.ts @@ -493,14 +493,14 @@ export const validateSchema = ( ); } - if (link.forward.has === 'many' && link.forward.onDelete === 'cascade') { + if (link.forward.has === 'many' && link.forward.onDelete) { throw new SchemaValidationError( - `${link.forward.on}${link.forward.label} -> ${link.reverse.on}${link.reverse.label} has onDelete: "cascade" with has: "many"`, + `${link.forward.on}${link.forward.label} -> ${link.reverse.on}${link.reverse.label} has onDelete: "${link.forward.onDelete}" with has: "many"`, ); } - if (link.reverse.has === 'many' && link.reverse.onDelete === 'cascade') { + if (link.reverse.has === 'many' && link.reverse.onDelete) { throw new SchemaValidationError( - `${link.forward.on}${link.forward.label} -> ${link.reverse.on}${link.reverse.label} has onDelete: "cascade" with has: "many"`, + `${link.forward.on}${link.forward.label} -> ${link.reverse.on}${link.reverse.label} has onDelete: "${link.reverse.onDelete}" with has: "many"`, ); } diff --git a/client/www/lib/types.ts b/client/www/lib/types.ts index ad08f536e5..805724f85f 100644 --- a/client/www/lib/types.ts +++ b/client/www/lib/types.ts @@ -165,6 +165,8 @@ export type DBIdent = export type CheckedDataType = 'string' | 'number' | 'boolean' | 'date'; +export type OnDelete = 'cascade' | 'restrict'; + export interface DBAttr { id: string; 'forward-identity': DBIdent; @@ -178,8 +180,8 @@ export interface DBAttr { 'inferred-types'?: Array<'string' | 'number' | 'boolean' | 'json'>; catalog?: 'user' | 'system'; 'checked-data-type'?: CheckedDataType; - 'on-delete'?: 'cascade'; - 'on-delete-reverse'?: 'cascade'; + 'on-delete'?: OnDelete; + 'on-delete-reverse'?: OnDelete; metadata?: any; } @@ -226,8 +228,8 @@ export interface SchemaAttr { catalog?: 'user' | 'system'; checkedDataType?: CheckedDataType; sortable: boolean; - onDelete?: 'cascade'; - onDeleteReverse?: 'cascade'; + onDelete?: OnDelete; + onDeleteReverse?: OnDelete; } export type OAuthAppClientSecret = { diff --git a/client/www/pages/docs/modeling-data.md b/client/www/pages/docs/modeling-data.md index c900ab0565..6a794deead 100644 --- a/client/www/pages/docs/modeling-data.md +++ b/client/www/pages/docs/modeling-data.md @@ -343,9 +343,13 @@ Our micro-blog example has the following relationship types: - **One-to-many** between `comments` and `profiles` - **Many-to-many** between `posts` and `tags` -### Cascade Delete +### On Delete -Links defined with `has: "one"` can set `onDelete: "cascade"`. In this case, when the profile entity is deleted, all post entities will be deleted too: +Links defined with `has: "one"` can set `onDelete` to either `cascade` or `restrict`. + +#### `cascade` + +In this case, when the profile entity is deleted, all post entities will be deleted too: ```typescript postAuthor: { @@ -357,17 +361,44 @@ postAuthor: { db.tx.profiles[user_id].delete(); ``` -Without `onDelete: "cascade"`, deleting a profile would simply delete the links but not delete the underlying posts. +Without `onDelete`, deleting a profile would simply delete the links but not delete the underlying posts. If you prefer to model links in other direction, you can do it, too: -``` +```typescript postAuthor: { forward: { on: "profiles", has: "many", label: "authoredPosts" }, reverse: { on: "posts", has: "one", label: "author", onDelete: "cascade" }, } ``` +#### `restrict` + +In this case, when the profile entity is deleted, the post entities must be deleted first, or the transaction will fail: + +```typescript +postAuthor: { + forward: { on: "posts", has: "one", label: "author", onDelete: "restrict" }, + reverse: { on: "profiles", has: "many", label: "authoredPosts" }, +} + +// this will prevent deleting the profile if there are posts +await db.transact(db.tx.profiles[user_id].delete()); +// throws validation error + +// If all posts are deleted in the same transaction, then it will succeed +await db.transact([db.tx.profiles[user_id].delete(), db.tx.posts[linked_post_id].delete()]) +``` + +If you prefer to model links in other direction, you can do it, too: + +```typescript +postAuthor: { + forward: { on: "profiles", has: "many", label: "authoredPosts" }, + reverse: { on: "posts", has: "one", label: "author", onDelete: "restrict" }, +} +``` + ## Publishing your schema Now that you have your schema, you can use the CLI to `push` it to your app: diff --git a/server/resources/migrations/100_on-delete-restrict.down.sql b/server/resources/migrations/100_on-delete-restrict.down.sql new file mode 100644 index 0000000000..8a1293f058 --- /dev/null +++ b/server/resources/migrations/100_on-delete-restrict.down.sql @@ -0,0 +1,11 @@ +alter type attr_on_delete rename to attr_on_delete_old; + +create type attr_on_delete as enum ('cascade'); + +update attrs set on_delete = null where on_delete::text = 'restrict'; +update attrs set on_delete_reverse = null where on_delete_reverse::text = 'restrict'; + +alter table attrs alter column on_delete type attr_on_delete using on_delete::text::attr_on_delete; +alter table attrs alter column on_delete_reverse type attr_on_delete using on_delete_reverse::text::attr_on_delete; + +drop type attr_on_delete_old; diff --git a/server/resources/migrations/100_on-delete-restrict.up.sql b/server/resources/migrations/100_on-delete-restrict.up.sql new file mode 100644 index 0000000000..68e5e587ec --- /dev/null +++ b/server/resources/migrations/100_on-delete-restrict.up.sql @@ -0,0 +1 @@ +alter type attr_on_delete add value 'restrict'; diff --git a/server/src/instant/config_edn.clj b/server/src/instant/config_edn.clj index b5d588f6a2..06bb9c7ae3 100644 --- a/server/src/instant/config_edn.clj +++ b/server/src/instant/config_edn.clj @@ -116,7 +116,7 @@ (when override ;; Can't use tracer because it requires config to be decoded before ;; it is initialized - (log/infof "Using config at resources/config/override.edn")) + (log/info "Using config at resources/config/override.edn")) (-> (or override (io/resource (format "config/%s.edn" (name env)))) slurp diff --git a/server/src/instant/db/transaction.clj b/server/src/instant/db/transaction.clj index 8eb0f46d56..27e089b85b 100644 --- a/server/src/instant/db/transaction.clj +++ b/server/src/instant/db/transaction.clj @@ -400,13 +400,13 @@ (let [attrs+etypes (->> attrs (filter #(= :ref (:value-type %))) - (filter #(= :cascade (:on-delete %))) + (filter #(:on-delete %)) (mapv #(vector (:id %) (-> % :forward-identity second) (-> % :reverse-identity second)))) reverse-attrs+etypes (->> attrs (filter #(= :ref (:value-type %))) - (filter #(= :cascade (:on-delete-reverse %))) + (filter #(:on-delete-reverse %)) (mapv #(vector (:id %) (-> % :forward-identity second) (-> % :reverse-identity second)))) query+args @@ -431,13 +431,15 @@ jsonb_array_elements(cast(?reverse-attrs+etypes AS jsonb)) AS elem ), - entids (entity_id, etype, parent_id) AS ( + entids (entity_id, etype, parent_id, parent_etype, attr_id, direction) AS ( -- Starting entities (the parents being deleted) SELECT cast(elem ->> 0 AS uuid) as entity_id, cast(elem ->> 1 AS text) as etype, cast(elem ->> 0 AS uuid) as parent_id, -- parent_id is itself for root entities - cast(elem ->> 1 as text) as parent_etype -- parent_etype is itself for root entities + cast(elem ->> 1 as text) as parent_etype, -- parent_etype is itself for root entities + cast(null as uuid) as attr_id, + cast(null as text) as direction FROM jsonb_array_elements(cast(?ids+etypes AS jsonb)) AS elem @@ -452,7 +454,8 @@ entity_id, etype, parent_id, - parent_etype + parent_etype, + attr_id FROM entids ) @@ -462,7 +465,9 @@ triples.entity_id AS entity_id, attrs_forward.forward_etype AS etype, entids_inner.parent_id AS parent_id, -- inherit parent from the entity that triggered cascade - entids_inner.parent_etype as parent_etype + entids_inner.parent_etype as parent_etype, + attrs_forward.id as attr_id, + 'forward' as direction FROM entids_inner JOIN triples @@ -481,7 +486,9 @@ json_uuid_to_uuid(triples.value) AS entity_id, attrs_reverse.reverse_etype AS etype, entids_inner.parent_id AS parent_id, -- inherit parent from the entity that triggered cascade - entids_inner.parent_etype as parent_etype + entids_inner.parent_etype as parent_etype, + attrs_reverse.id as attr_id, + 'reverse' as direction FROM entids_inner JOIN triples @@ -496,7 +503,7 @@ ) SELECT - entity_id, etype, parent_id, parent_etype + entity_id, etype, parent_id, parent_etype, attr_id, direction FROM entids" {"?app-id" app-id @@ -508,25 +515,94 @@ (for [{:keys [op eid value]} tx-step-maps :when (and (= :rule-params op) (uuid? eid))] [eid value])) + original-deleted-entities (set ids+etypes) + + {:keys [first-step add-triple retract-triple]} + (coll/reduce-tr (fn [acc step] + (case (:op step) + (:add-triple :retract-triple) + (let [attr (attr-model/seek-by-id (:aid step) attrs)] + (if-not (= :ref (:value-type attr)) + acc + (cond-> acc + + (or (nil? (:first-step acc)) + (= :retract-triple (:first-step acc)) + (not= :add-triple (:op step))) + (assoc! (:op step) + (conj! (get acc (:op step)) + [(:eid step) + (:aid step) + (uuid/coerce (:value step))])) + + (not (:first-step acc)) + (assoc! :first-step (:op step))))) + acc)) + {:first-step nil + :add-triple (transient #{}) + :retract-triple (transient #{})} + tx-step-maps) + + original-deleted-triples + (case first-step + nil #{} + ;; add-triple is first, so they won't overwrite our retracts + :add-triple (persistent! retract-triple) + :retract-triple (as-> retract-triple % + (apply disj! % (persistent! add-triple)) + (persistent! %))) + + ;; Only get the cascaded entities (where parent_id != entity_id) - cascaded-entities (remove #(and (= (:parent_id %) (:entity_id %)) - (= (:parent_etype %) (:etype %))) - res)] - (concat - tx-step-maps - ;; Add delete operations for cascaded entities only - (for [{:keys [entity_id etype]} cascaded-entities] - {:op :delete-entity - :eid entity_id - :etype etype}) - ;; Add rule-params for cascaded entities that have a parent with rule-params - (for [{:keys [entity_id etype parent_id]} cascaded-entities - :let [rule-params (get parent-id->rule-params parent_id)] - :when rule-params] - {:op :rule-params - :eid entity_id - :etype etype - :value rule-params})))))) + cascaded-entities + (keep (fn [{:keys [parent_id entity_id parent_etype etype attr_id direction] :as ent}] + (when-not (and (= parent_id entity_id) + (= parent_etype etype)) + (let [on-delete (case direction + "forward" :on-delete + "reverse" :on-delete-reverse) + attr (attr-model/seek-by-id attr_id attrs) + cascade-rule (-> attr + (get on-delete))] + (case cascade-rule + :restrict + (when (and (= cascade-rule :restrict) + (not (or (contains? original-deleted-entities [entity_id etype]) + (contains? original-deleted-triples (case on-delete + :on-delete + [entity_id attr_id parent_id] + :on-delete-reverse + [parent_id attr_id entity_id]))))) + (ex/throw-validation-err! + :on-delete-restrict + (map vectorize-tx-step tx-step-maps) + [{:message (format (clojure.string/join + " " + ["This transaction violates an on-delete constraint." + "The `%s` entity cannot be deleted unless its" + "linked `%s` entity is deleted first."]) + parent_etype + etype)}])) + #_else + ent)))) + res) + + steps (concat + tx-step-maps + ;; Add delete operations for cascaded entities only + (for [{:keys [entity_id etype]} cascaded-entities] + {:op :delete-entity + :eid entity_id + :etype etype}) + ;; Add rule-params for cascaded entities that have a parent with rule-params + (for [{:keys [entity_id etype parent_id]} cascaded-entities + :let [rule-params (get parent-id->rule-params parent_id)] + :when rule-params] + {:op :rule-params + :eid entity_id + :etype etype + :value rule-params}))] + steps)))) (defn validate-value-lookup-etypes "Check that in the case of diff --git a/server/src/instant/model/schema.clj b/server/src/instant/model/schema.clj index da2e94db7d..d96dafdc80 100644 --- a/server/src/instant/model/schema.clj +++ b/server/src/instant/model/schema.clj @@ -1,5 +1,6 @@ (ns instant.model.schema - (:require [instant.db.model.attr :as attr-model] + (:require [clojure.string] + [instant.db.model.attr :as attr-model] [instant.jdbc.aurora :as aurora] [instant.db.datalog :as d] [instant.db.indexing-jobs :as indexing-jobs] @@ -239,9 +240,10 @@ "Check your full schema in the dashboard for a link with the same label names: " "https://www.instantdb.com/dash?s=main&t=explorer")) -(defn cascade-message [[etype label]] +(defn on-delete-message [action [etype label]] (str etype "->" label ": " - "Cascade delete is only possible on links with `has: 'one'`. " + (clojure.string/capitalize (name action)) + " delete is only possible on links with `has: 'one'`. " "Check your full schema in the dashboard: " "https://www.instantdb.com/dash?s=main&t=explorer")) @@ -308,14 +310,14 @@ (and (= :ref (:value-type attr)) (= :many (:cardinality attr)) - (= :cascade (:on-delete attr))) - (cascade-message fwd-name) + (:on-delete attr)) + (on-delete-message (:on-delete attr) fwd-name) (and (= :ref (:value-type attr)) (not (:unique? attr)) - (= :cascade (:on-delete-reverse attr))) - (cascade-message rev-name))] + (:on-delete-reverse attr)) + (on-delete-message (:on-delete-reverse attr) rev-name))] :when message] {:in [:schema] :message message}))] diff --git a/server/src/instant/util/test.clj b/server/src/instant/util/test.clj index 04b3b52fec..8f79a97dfc 100644 --- a/server/src/instant/util/test.clj +++ b/server/src/instant/util/test.clj @@ -109,7 +109,9 @@ :index? (assoc m :index? true) :required? (assoc m :required? true) :on-delete (assoc m :on-delete :cascade) - :on-delete-reverse (assoc m :on-delete-reverse :cascade))) + :on-delete-restrict (assoc m :on-delete :restrict) + :on-delete-reverse (assoc m :on-delete-reverse :cascade) + :on-delete-reverse-restrict (assoc m :on-delete-reverse :restrict))) {:id (random-uuid) :forward-identity [(random-uuid) (namespace fwd) (name fwd)] :reverse-identity (when rvr diff --git a/server/test/instant/db/transaction_test.clj b/server/test/instant/db/transaction_test.clj index 9cfd0816d6..6a29ae54e9 100644 --- a/server/test/instant/db/transaction_test.clj +++ b/server/test/instant/db/transaction_test.clj @@ -4835,10 +4835,103 @@ (is (= #{} (test-util/find-entids-by-ids app-id attr->id ids)))))))) +(deftest on-delete-restrict + (with-empty-app + (fn [{app-id :id}] + ;; user <- book + (let [attr->id (test-util/make-attrs + app-id + [[:user/name :unique? :index?] + [:book/title :unique? :index?] + [[:book/author :user/books] :on-delete-restrict]]) + ids #{(suid "a") (suid "b1") (suid "b2") (suid "b3")} + attr-model (attr-model/get-by-app-id app-id)] + + (test-util/insert-entities + app-id attr->id + [{:db/id (suid "a") :user/name "Leo Tolstoy"} + {:db/id (suid "b1") :book/title "War and Peace" :book/author (suid "a")} + {:db/id (suid "b2") :book/title "Anna Karenina" :book/author (suid "a")} + {:db/id (suid "b3") :book/title "Death of Ivan Ilyich" :book/author (suid "a")}]) + (is (= #{(suid "a") (suid "b1") (suid "b2") (suid "b3")} + (test-util/find-entids-by-ids app-id attr->id ids))) + + (testing "deleting book doesn’t delete user" + (tx/transact! (aurora/conn-pool :write) attr-model app-id [[:delete-entity (suid "b1") "book"]]) + (is (= #{(suid "a") (suid "b2") (suid "b3")} + (test-util/find-entids-by-ids app-id attr->id ids)))) + + (testing "deleting user is blocked if you don't delete the book" + (is (thrown-with-msg? clojure.lang.ExceptionInfo + #"violates an on-delete constraint" + (tx/transact! (aurora/conn-pool :write) attr-model app-id [[:delete-entity (suid "a") "user"]]))) + (is (= #{(suid "a") (suid "b2") (suid "b3")} + (test-util/find-entids-by-ids app-id attr->id ids)))) + + (testing "deleting user works if you also delete the book" + (tx/transact! (aurora/conn-pool :write) attr-model app-id [[:delete-entity (suid "a") "user"] + [:delete-entity (suid "b2") "book"] + [:delete-entity (suid "b3") "book"]]) + (is (= #{} + (test-util/find-entids-by-ids app-id attr->id ids)))))))) + +(deftest on-delete-restrict-with-unlinking + (with-empty-app + (fn [{app-id :id}] + ;; user <- book + (let [attr->id (test-util/make-attrs + app-id + [[:user/name :unique? :index?] + [:book/title :unique? :index?] + [[:book/author :user/books] :on-delete-restrict]]) + ids #{(suid "a") (suid "b1") (suid "b2") (suid "b3")} + attr-model (attr-model/get-by-app-id app-id)] + + (test-util/insert-entities + app-id attr->id + [{:db/id (suid "a") :user/name "Leo Tolstoy"} + {:db/id (suid "b1") :book/title "War and Peace" :book/author (suid "a")} + {:db/id (suid "b2") :book/title "Anna Karenina" :book/author (suid "a")} + {:db/id (suid "b3") :book/title "Death of Ivan Ilyich" :book/author (suid "a")}]) + (is (= #{(suid "a") (suid "b1") (suid "b2") (suid "b3")} + (test-util/find-entids-by-ids app-id attr->id ids))) + + (testing "deleting book doesn’t delete user" + (tx/transact! (aurora/conn-pool :write) attr-model app-id [[:delete-entity (suid "b1") "book"]]) + (is (= #{(suid "a") (suid "b2") (suid "b3")} + (test-util/find-entids-by-ids app-id attr->id ids)))) + + (testing "deleting user is blocked if you don't delete the book" + (is (thrown-with-msg? clojure.lang.ExceptionInfo + #"violates an on-delete constraint" + (tx/transact! (aurora/conn-pool :write) attr-model app-id [[:delete-entity (suid "a") "user"]]))) + (is (= #{(suid "a") (suid "b2") (suid "b3")} + (test-util/find-entids-by-ids app-id attr->id ids)))) + + (testing "deleting user works if you unlink the book" + (is (thrown-with-msg? clojure.lang.ExceptionInfo + #"violates an on-delete constraint" + (tx/transact! (aurora/conn-pool :write) attr-model app-id + [[:retract-triple (suid "b2") (:book/author attr->id) (suid "a")] + [:add-triple (suid "b2") (:book/author attr->id) (suid "a")] + [:retract-triple (suid "b3") (:book/author attr->id) (suid "a")] + [:delete-entity (suid "a") "user"]]))) + + (is (= #{(suid "a") (suid "b2") (suid "b3")} + (test-util/find-entids-by-ids app-id attr->id ids)))) + + (testing "deleting user works if you unlink the book" + (tx/transact! (aurora/conn-pool :write) attr-model app-id [[:add-triple (suid "b2") (:book/author attr->id) (suid "a")] + [:retract-triple (suid "b2") (:book/author attr->id) (suid "a")] + [:retract-triple (suid "b3") (:book/author attr->id) (suid "a")] + [:delete-entity (suid "a") "user"]]) + (is (= #{(suid "b2") (suid "b3")} + (test-util/find-entids-by-ids app-id attr->id ids)))))))) + (deftest on-delete-cascade-reverse (with-empty-app (fn [{app-id :id}] - ;;; user -> book + ;; user -> book (let [attr->id (test-util/make-attrs app-id [[:user/name :unique? :index?] @@ -4866,10 +4959,104 @@ (is (= #{} (test-util/find-entids-by-ids app-id attr->id ids)))))))) +(deftest on-delete-restrict-reverse + (with-empty-app + (fn [{app-id :id}] + ;; user -> book + (let [attr->id (test-util/make-attrs + app-id + [[:user/name :unique? :index?] + [:book/title :unique? :index?] + [[:user/books :book/author] :many :unique? :on-delete-reverse-restrict]]) + ids #{(suid "a") (suid "b1") (suid "b2") (suid "b3")} + attr-model (attr-model/get-by-app-id app-id)] + + (test-util/insert-entities + app-id attr->id + [{:db/id (suid "a") :user/name "Leo Tolstoy" :user/books [(suid "b1") (suid "b2") (suid "b3")]} + {:db/id (suid "b1") :book/title "War and Peace"} + {:db/id (suid "b2") :book/title "Anna Karenina"} + {:db/id (suid "b3") :book/title "Death of Ivan Ilyich"}]) + (is (= #{(suid "a") (suid "b1") (suid "b2") (suid "b3")} + (test-util/find-entids-by-ids app-id attr->id ids))) + + (testing "deleting book doesn’t delete user" + (tx/transact! (aurora/conn-pool :write) attr-model app-id [[:delete-entity (suid "b1") "book"]]) + (is (= #{(suid "a") (suid "b2") (suid "b3")} + (test-util/find-entids-by-ids app-id attr->id ids)))) + + (testing "deleting user is blocked if you don't delete the book" + (is (thrown-with-msg? clojure.lang.ExceptionInfo + #"violates an on-delete constraint" + (tx/transact! (aurora/conn-pool :write) attr-model app-id [[:delete-entity (suid "a") "user"]]))) + (is (= #{(suid "a") (suid "b2") (suid "b3")} + (test-util/find-entids-by-ids app-id attr->id ids)))) + + (testing "deleting user works if you also delete the book" + (tx/transact! (aurora/conn-pool :write) attr-model app-id [[:delete-entity (suid "a") "user"] + [:delete-entity (suid "b2") "book"] + [:delete-entity (suid "b3") "book"]]) + (is (= #{} + (test-util/find-entids-by-ids app-id attr->id ids)))))))) + +(deftest on-delete-restrict-reverse-with-unlinking + (with-empty-app + (fn [{app-id :id}] + ;; user <- book + (let [attr->id (test-util/make-attrs + app-id + [[:user/name :unique? :index?] + [:book/title :unique? :index?] + [[:user/books :book/author] :many :unique? :on-delete-reverse-restrict]]) + ids #{(suid "a") (suid "b1") (suid "b2") (suid "b3")} + attr-model (attr-model/get-by-app-id app-id)] + + (test-util/insert-entities + app-id attr->id + [{:db/id (suid "a") :user/name "Leo Tolstoy" :user/books [(suid "b1") (suid "b2") (suid "b3")]} + {:db/id (suid "b1") :book/title "War and Peace"} + {:db/id (suid "b2") :book/title "Anna Karenina"} + {:db/id (suid "b3") :book/title "Death of Ivan Ilyich"}]) + (is (= #{(suid "a") (suid "b1") (suid "b2") (suid "b3")} + (test-util/find-entids-by-ids app-id attr->id ids))) + + (testing "deleting book doesn’t delete user" + (tx/transact! (aurora/conn-pool :write) attr-model app-id [[:delete-entity (suid "b1") "book"]]) + (is (= #{(suid "a") (suid "b2") (suid "b3")} + (test-util/find-entids-by-ids app-id attr->id ids)))) + + (testing "deleting user is blocked if you don't delete the book" + (is (thrown-with-msg? clojure.lang.ExceptionInfo + #"violates an on-delete constraint" + (tx/transact! (aurora/conn-pool :write) attr-model app-id [[:delete-entity (suid "a") "user"]]))) + (is (= #{(suid "a") (suid "b2") (suid "b3")} + (test-util/find-entids-by-ids app-id attr->id ids)))) + + (testing "deleting user is blocked if you add the triple after retracting it" + (is (thrown-with-msg? + clojure.lang.ExceptionInfo + #"violates an on-delete constraint" + (tx/transact! (aurora/conn-pool :write) attr-model app-id + [[:retract-triple (suid "a") (:user/books attr->id) (suid "b2")] + [:add-triple (suid "a") (:user/books attr->id) (suid "b3")] + [:retract-triple (suid "a") (:user/books attr->id) (suid "b3")] + [:delete-entity (suid "a") "user"]]))) + (is (= #{(suid "a") (suid "b2") (suid "b3")} + (test-util/find-entids-by-ids app-id attr->id ids)))) + + (testing "deleting user works if you also unlink the book" + (tx/transact! (aurora/conn-pool :write) attr-model app-id + [[:add-triple (suid "a") (:user/books attr->id) (suid "b3")] + [:retract-triple (suid "a") (:user/books attr->id) (suid "b2")] + [:retract-triple (suid "a") (:user/books attr->id) (suid "b3")] + [:delete-entity (suid "a") "user"]]) + (is (= #{(suid "b2") (suid "b3")} + (test-util/find-entids-by-ids app-id attr->id ids)))))))) + (deftest on-delete-cascade-mixed (with-empty-app (fn [{app-id :id}] - ;;; A <- B -> C <- D -> E <- F + ;; A <- B -> C <- D -> E <- F (let [attr->id (test-util/make-attrs app-id [[:A/id :unique? :index?] @@ -4899,6 +5086,53 @@ (tx/transact! (aurora/conn-pool :write) attr-model app-id [[:delete-entity (suid "a") "A"]]) (is (= #{} (test-util/find-entids-by-ids app-id attr->id ids))))))) +(deftest on-delete-restrict-mixed + (with-empty-app + (fn [{app-id :id}] + ;; A <- B -> C <- D -> E <- F + (let [attr->id (test-util/make-attrs + app-id + [[:A/id :unique? :index?] + [:B/id :unique? :index?] + [[:B/a :A/bs] :on-delete] + [[:B/c :C/bs] :many :unique? :on-delete-reverse-restrict] + [:C/id :unique? :index?] + [:D/id :unique? :index?] + [[:D/c :C/ds] :on-delete] + [[:D/e :E/ds] :many :unique? :on-delete-reverse-restrict] + [:E/id :unique? :index?] + [:F/id :unique? :index?] + [[:F/e :E/fs] :on-delete]]) + ids #{(suid "a") (suid "b") (suid "c") (suid "d") (suid "e") (suid "f")} + attr-model (attr-model/get-by-app-id app-id)] + + (test-util/insert-entities + app-id attr->id + [{:db/id (suid "a") :A/id (suid "a")} + {:db/id (suid "b") :B/id (suid "b") :B/a (suid "a") :B/c (suid "c")} + {:db/id (suid "c") :C/id (suid "c")} + {:db/id (suid "d") :D/id (suid "d") :D/c (suid "c") :D/e (suid "e")} + {:db/id (suid "e") :E/id (suid "e")} + {:db/id (suid "f") :F/id (suid "f") :F/e (suid "e")}]) + (is (= ids (test-util/find-entids-by-ids app-id attr->id ids))) + + (is (thrown-with-msg? clojure.lang.ExceptionInfo + #"violates an on-delete constraint" + (tx/transact! (aurora/conn-pool :write) attr-model app-id [[:delete-entity (suid "a") "A"]]))) + (is (= ids (test-util/find-entids-by-ids app-id attr->id ids))) + + (is (thrown-with-msg? clojure.lang.ExceptionInfo + #"violates an on-delete constraint" + (tx/transact! (aurora/conn-pool :write) attr-model app-id [[:delete-entity (suid "a") "A"] + [:delete-entity (suid "c") "C"]]))) + (is (= ids (test-util/find-entids-by-ids app-id attr->id ids))) + + (tx/transact! (aurora/conn-pool :write) attr-model app-id [[:delete-entity (suid "a") "A"] + [:delete-entity (suid "c") "C"] + [:delete-entity (suid "e") "E"]]) + + (is (= #{} (test-util/find-entids-by-ids app-id attr->id ids))))))) + (deftest on-delete-cascade-cycle (with-empty-app (fn [{app-id :id}] @@ -4920,6 +5154,36 @@ (is (= #{} (test-util/find-entids-by-ids app-id attr->id ids))))))) +(deftest on-delete-restrict-cycle + (with-empty-app + (fn [{app-id :id}] + (let [attr->id (test-util/make-attrs + app-id + [[:users/name :unique? :index?] + [[:users/friend :users/friend-of] :unique? :on-delete-restrict]]) + ids #{(suid "a") (suid "b")} + attr-model (attr-model/get-by-app-id app-id)] + + (test-util/insert-entities + app-id attr->id + [{:db/id (suid "a") :users/name "Ivan" :users/friend (suid "b")} + {:db/id (suid "b") :users/name "Oleg" :users/friend (suid "a")}]) + (is (= #{(suid "a") (suid "b")} + (test-util/find-entids-by-ids app-id attr->id ids))) + + (is (thrown-with-msg? clojure.lang.ExceptionInfo + #"violates an on-delete constraint" + (tx/transact! (aurora/conn-pool :write) attr-model app-id [[:delete-entity (suid "a") "users"]]))) + + (is (= #{(suid "a") (suid "b")} + (test-util/find-entids-by-ids app-id attr->id ids))) + + (tx/transact! (aurora/conn-pool :write) attr-model app-id [[:delete-entity (suid "a") "users"] + [:delete-entity (suid "b") "users"]]) + + (is (= #{} + (test-util/find-entids-by-ids app-id attr->id ids))))))) + (deftest on-delete-cascade-etypes (with-empty-app (fn [{app-id :id}] diff --git a/server/test/instant/reactive/session_test.clj b/server/test/instant/reactive/session_test.clj index fc5e2b5b91..324aa0a2da 100644 --- a/server/test/instant/reactive/session_test.clj +++ b/server/test/instant/reactive/session_test.clj @@ -1275,7 +1275,7 @@ {:keys [offset]} (blocking-send-msg :start-stream-ok socket-2 {:op :start-stream :client-id "stream-1" - :reconnect-token (str reconnect-token)}) + :reconnect-token reconnect-token}) _ (is (= offset 8)) _ (send-msg socket-2 {:op :append-stream @@ -1287,7 +1287,7 @@ (testing "if someone steals our session, we can't write to it" (blocking-send-msg :start-stream-ok socket-3 {:op :start-stream :client-id "stream-1" - :reconnect-token (str reconnect-token)}) + :reconnect-token reconnect-token}) (blocking-send-msg :error socket-2 {:op :append-stream :client-id "stream-1" :chunks ["GHI"]