Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for idPrefix changes #55

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Support for idPrefix changes #55

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
15c6255
Waiting for bugfix in ronin
colodenn Feb 13, 2025
ab1f152
Only show which migration prompt when used with ronin apply
colodenn Feb 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 16 additions & 12 deletions src/commands/apply.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,18 +37,22 @@ export default async (
// Get all filenames of migrations in the migrations directory.
const migrations = fs.readdirSync(MIGRATIONS_PATH);

const migrationPrompt =
migrationFilePath ??
(await select({
message: 'Which migration do you want to apply?',
choices: migrations
// Sort in reverse lexical order
.sort((a, b) => b.localeCompare(a))
.map((migration) => ({
name: migration,
value: path.join(MIGRATIONS_PATH, migration),
})),
}));
let migrationPrompt: string | undefined;

if (!flags.apply) {
migrationPrompt =
migrationFilePath ??
(await select({
message: 'Which migration do you want to apply?',
choices: migrations
// Sort in reverse lexical order
.sort((a, b) => b.localeCompare(a))
.map((migration) => ({
name: migration,
value: path.join(MIGRATIONS_PATH, migration),
})),
}));
}

const protocol = await new Protocol(packages).load(migrationPrompt);
const statements = protocol.getSQLStatements(existingModels);
Expand Down
53 changes: 32 additions & 21 deletions src/utils/field.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,13 +118,15 @@ export const diffFields = async (
if (field.from.type === 'link') {
diff.push(
...createTempModelQuery(
modelSlug,
[
{ ...field.to, slug: field.from.slug },
...definedFields.filter((local) => local.slug !== field.to.slug),
],
indexes,
triggers,
{
slug: modelSlug,
fields: [
{ ...field.to, slug: field.from.slug },
...definedFields.filter((local) => local.slug !== field.to.slug),
],
triggers,
indexes,
},
[
renameFieldQuery(
`${RONIN_SCHEMA_TEMP_SUFFIX}${modelSlug}`,
Expand Down Expand Up @@ -176,10 +178,12 @@ export const diffFields = async (

diff.push(
...createTempModelQuery(
modelSlug,
updatedFields || [],
[],
[],
{
slug: modelSlug,
fields: updatedFields || [],
indexes,
triggers,
},
queries,
existingFields,
),
Expand Down Expand Up @@ -284,7 +288,7 @@ const adjustFields = (
indexes: Array<ModelIndex>,
triggers: Array<ModelTrigger>,
): Array<string> => {
return createTempModelQuery(modelSlug, fields, indexes, triggers);
return createTempModelQuery({ slug: modelSlug, fields, indexes, triggers });
};

/**
Expand Down Expand Up @@ -338,20 +342,20 @@ export const createFields = async (
);

return createTempModelQuery(
modelSlug,
updatedFields || [],
[],
[],
{
slug: modelSlug,
fields: updatedFields || [],
},
queries,
existingFields,
);
}

return createTempModelQuery(
modelSlug,
definedFields || [],
[],
[],
{
slug: modelSlug,
fields: definedFields || [],
},
[],
existingFields,
);
Expand Down Expand Up @@ -419,7 +423,14 @@ const deleteFields = (
const diff: Array<string> = [];
for (const fieldToDrop of fieldsToDrop) {
if (fieldToDrop.unique) {
return createTempModelQuery(modelSlug, fields, [], [], [], fields);
return createTempModelQuery(
{
slug: modelSlug,
fields,
},
[],
fields,
);
}
diff.push(dropFieldQuery(modelSlug, fieldToDrop.slug));
}
Expand Down
22 changes: 14 additions & 8 deletions src/utils/migration.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { type BaseFlags, areArraysEqual } from '@/src/utils/misc';
import {
createIndexQuery,
createModelQuery,
createTempModelQuery,
createTriggerQuery,
dropIndexQuery,
dropModelQuery,
Expand Down Expand Up @@ -252,14 +253,19 @@ export const adjustModelMeta = (

for (const model of definedModels) {
const currentModel = databaseModelsMap.get(model.slug);
if (!(model.name && model.idPrefix)) continue;
if (
currentModel &&
(model.name !== currentModel.name || model.idPrefix !== currentModel.idPrefix)
) {
newModels.push(
`alter.model("${model.slug}").to({name: "${model.name}", idPrefix: "${model.idPrefix}"})`,
);

if (currentModel) {
// The `name` and the `idPrefix` are generated in the compiler thus they are
// not always present. So if the defined model has no name or idPrefix we skip
// the model.
if (model.idPrefix && model.idPrefix !== currentModel.idPrefix) {
// If the prefix changes we need to recreate the model.
// All records inserted will use the new prefix. All old ids are not updated.
newModels.push(...createTempModelQuery(model));
} else if (model.name && model.name !== currentModel.name) {
// Otherwise, just update the name.
newModels.push(`alter.model("${model.slug}").to({name: "${model.name}"})`);
}
}
}

Expand Down
25 changes: 14 additions & 11 deletions src/utils/queries.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,23 +126,26 @@ export const dropFieldQuery = (modelSlug: string, fieldSlug: string): string =>
* ```
*/
export const createTempModelQuery = (
modelSlug: string,
fields: Array<ModelField>,
_indexes: Array<ModelIndex>,
triggers: Array<ModelTrigger>,
model: Model,
customQueries?: Array<string>,
includeFields?: Array<ModelField>,
): Array<string> => {
const queries: Array<string> = [];

const tempModelSlug = `${RONIN_SCHEMA_TEMP_SUFFIX}${modelSlug}`;
const tempModelSlug = `${RONIN_SCHEMA_TEMP_SUFFIX}${model.slug}`;

// Create a copy of the model
queries.push(createModelQuery(tempModelSlug, { fields }));
queries.push(
createModelQuery(tempModelSlug, {
fields: model.fields,
name: model.name,
idPrefix: model.idPrefix,
}),
);

// Move all the data to the copied model
queries.push(
`add.${tempModelSlug}.with(() => get.${modelSlug}(${
`add.${tempModelSlug}.with(() => get.${model.slug}(${
includeFields
? JSON.stringify({ selecting: includeFields.map((field) => field.slug) })
: ''
Expand All @@ -154,13 +157,13 @@ export const createTempModelQuery = (
}

// Delete the original model
queries.push(dropModelQuery(modelSlug));
queries.push(dropModelQuery(model.slug));

// Rename the copied model to the original model
queries.push(`alter.model("${tempModelSlug}").to({slug: "${modelSlug}"})`);
queries.push(`alter.model("${tempModelSlug}").to({slug: "${model.slug}"})`);

for (const trigger of triggers) {
queries.push(createTriggerQuery(modelSlug, trigger));
for (const trigger of model.triggers || []) {
queries.push(createTriggerQuery(model.slug, trigger));
}

return queries;
Expand Down
67 changes: 65 additions & 2 deletions tests/utils/apply.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@
TestU,
} from '@/fixtures/index';
import { getRowCount, getSQLTables, getTableRows, runMigration } from '@/fixtures/utils';
import { formatSqliteStatement } from '@/src/utils/format';
import { getLocalPackages } from '@/src/utils/misc';
import type { Model } from '@ronin/syntax/schema';
import { model, string } from 'ronin/schema';
const packages = await getLocalPackages();
const { Transaction } = packages.compiler;

Expand Down Expand Up @@ -187,17 +190,25 @@
});
});

test('meta properties', async () => {
const { models, db } = await runMigration([TestC], [TestA]);
test.only('meta properties', async () => {
const { models, db, modelDiff, statements } = await runMigration(
[TestC],
[TestA],
);

const rowCounts: Record<string, number> = {};
for (const model of models) {
if (model.pluralSlug) {
rowCounts[model.pluralSlug] = await getRowCount(db, model.pluralSlug);
}
}
console.log(modelDiff);
for (const statement of statements) {
console.log(formatSqliteStatement(statement.statement));
}
console.log(JSON.stringify(models, null, 2));
expect(models).toHaveLength(1);
expect(models[0].name).toBe('ThisIsACoolModel');

Check failure on line 211 in tests/utils/apply.test.ts

View workflow job for this annotation

GitHub Actions / validate / Testing

error: expect(received).toBe(expected) 

Expected: "ThisIsACoolModel"
Received: "Test"

      at /home/runner/work/cli/cli/tests/utils/apply.test.ts:211:34
expect(rowCounts).toEqual({
tests: 0,
});
Expand Down Expand Up @@ -241,6 +252,58 @@
});
});
});

describe('update', () => {
test.only('id prefix', async () => {
const definedModel = model({
slug: 'test',
idPrefix: 'test',
fields: {
name: string(),
},
}) as unknown as Model;

const existingModel = model({
slug: 'test',
idPrefix: 'corny',
fields: {
name: string(),
},
}) as unknown as Model;

const insert = {
add: {
test: {
with: {
name: 'Ilayda',
},
},
},
};

const transaction = new Transaction([insert], {
models: [definedModel, existingModel],
inlineParams: true,
});

const { modelDiff, db, statements, models } = await runMigration(
[definedModel],
[existingModel],
{},
transaction.statements.map((statement) => statement),
);

console.log(models);

await db.query(transaction.statements);

const rows = await getTableRows(db, definedModel);
console.log(rows);

expect(rows[0].id).toContain('corny_');
expect(rows[1].id).toContain('test_');
});
});
});
});

Expand Down
27 changes: 19 additions & 8 deletions tests/utils/migration.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,12 @@ describe('migration', () => {
// It is not recognized as a model.
const modelDiff = await diffModels([TestC], [TestA]);

expect(modelDiff).toHaveLength(1);
expect(modelDiff).toHaveLength(4);
expect(modelDiff).toStrictEqual([
'alter.model("test").to({name: "ThisIsACoolModel", idPrefix: "TICM"})',
"create.model({slug:'RONIN_TEMP_test',fields:[{slug:'age', required:true, unique:true, type:'string'}, {slug:'active', type:'boolean'}], name:'ThisIsACoolModel', idPrefix:'TICM'})",
'add.RONIN_TEMP_test.with(() => get.test())',
'drop.model("test")',
'alter.model("RONIN_TEMP_test").to({slug: "test"})',
]);
});

Expand Down Expand Up @@ -442,14 +445,22 @@ describe('migration', () => {

const queries = adjustModelMeta(definedModels, existingModels);

expect(queries).toHaveLength(2);
expect(queries).toHaveLength(8);
expect(queries).toStrictEqual([
'alter.model("test1").to({name: "Test Model 1", idPrefix: "TM1"})',
'alter.model("test2").to({name: "Test Model 2", idPrefix: "TM2"})',
"create.model({slug:'RONIN_TEMP_test1',name:'Test Model 1', idPrefix:'TM1'})",
'add.RONIN_TEMP_test1.with(() => get.test1())',
'drop.model("test1")',
'alter.model("RONIN_TEMP_test1").to({slug: "test1"})',
"create.model({slug:'RONIN_TEMP_test2',name:'Test Model 2', idPrefix:'TM2'})",
'add.RONIN_TEMP_test2.with(() => get.test2())',
'drop.model("test2")',
'alter.model("RONIN_TEMP_test2").to({slug: "test2"})',
]);
});

test('skips models without name and idPrefix', () => {
// This is not possible! The CLI can't detect that we removed the idPrefix. Because
// the idPrefix is autogenerated by the compiler.
const definedModels = [
{
slug: 'test1',
Expand All @@ -471,9 +482,9 @@ describe('migration', () => {
];

const queries = adjustModelMeta(definedModels, existingModels);

expect(queries).toHaveLength(0);
expect(queries).toStrictEqual([]);
console.log(queries);
expect(queries).toHaveLength(1);
expect(queries).toStrictEqual(['alter.model("test1").to({name: "Test Model 1"})']);
});

test('returns empty array when no changes needed', () => {
Expand Down
6 changes: 3 additions & 3 deletions tests/utils/queries.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ describe('queries', () => {
required: true,
},
];
const result = createTempModelQuery('user', fields, [], []);
const result = createTempModelQuery({ slug: 'user', fields });
expect(result).toEqual([
"create.model({slug:'RONIN_TEMP_user',fields:[{slug:'username', type:'string', name:'Username', unique:true, required:true}]})",
'add.RONIN_TEMP_user.with(() => get.user())',
Expand All @@ -123,7 +123,7 @@ describe('queries', () => {
},
];
const customQueries: Array<string> = ['get.model("user")'];
const result = createTempModelQuery('user', fields, [], [], customQueries);
const result = createTempModelQuery({ slug: 'user', fields }, customQueries);
expect(result).toEqual([
"create.model({slug:'RONIN_TEMP_user',fields:[{slug:'username', type:'string', name:'Username', unique:true, required:true}]})",
'add.RONIN_TEMP_user.with(() => get.user())',
Expand All @@ -150,7 +150,7 @@ describe('queries', () => {
effects: [],
},
];
const result = createTempModelQuery('user', fields, [], triggers);
const result = createTempModelQuery({ slug: 'user', fields, triggers });
expect(result).toEqual([
"create.model({slug:'RONIN_TEMP_user',fields:[{slug:'username', type:'string', name:'Username', unique:true, required:true}]})",
'add.RONIN_TEMP_user.with(() => get.user())',
Expand Down
Loading