Skip to content

Commit

Permalink
fix: unexpected file changes when editing (#2786)
Browse files Browse the repository at this point in the history
* use cached projects

* don't need to call init after project loaded

* use a real-time refreshed files list, fix concurrency deletes

* update test

* create/saveAs/open project should call init

* clean up

* update

Co-authored-by: Chris Whitten <christopher.whitten@microsoft.com>
Co-authored-by: Ben Yackley <61990921+beyackle@users.noreply.github.com>
Co-authored-by: Andy Brown <asbrown002@gmail.com>
  • Loading branch information
4 people authored Apr 30, 2020
1 parent 07c701f commit 6d14ac5
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -159,15 +159,15 @@ describe('dialog operation', () => {
const mockReq = {
params: { projectId },
query: {},
body: { name: 'test.dialog', content: '' },
body: { name: 'test2.dialog', content: '' },
} as Request;
await ProjectController.createFile(mockReq, mockRes);
expect(mockRes.status).toHaveBeenCalledWith(200);
});

it('should remove dialog', async () => {
const mockReq = {
params: { name: 'test.dialog', projectId },
params: { name: 'test2.dialog', projectId },
query: {},
body: {},
} as Request;
Expand Down
1 change: 0 additions & 1 deletion Composer/packages/server/src/controllers/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,6 @@ async function getProjectById(req: Request, res: Response) {
const currentProject = await BotProjectService.getProjectById(projectId, user);

if (currentProject !== undefined && (await currentProject.exists())) {
await currentProject.init();
const project = currentProject.getProject();
res.status(200).json({
id: projectId,
Expand Down
10 changes: 5 additions & 5 deletions Composer/packages/server/src/models/bot/botProject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ export class BotProject {
};

public deleteFile = async (name: string) => {
if (Path.resolve(name) === 'Main') {
if (Path.basename(name, '.dialog') === this.name) {
throw new Error(`Main dialog can't be removed`);
}

Expand Down Expand Up @@ -334,7 +334,7 @@ export class BotProject {
try {
await this.fileStorage.rmDir(folderPath);
} catch (e) {
console.log(e);
// pass
}
}
};
Expand Down Expand Up @@ -411,19 +411,19 @@ export class BotProject {
if (index === -1) {
throw new Error(`no such file at ${relativePath}`);
}

const absolutePath = `${this.dir}/${relativePath}`;

// only write if the file has actually changed
if (this.files[index].content !== content) {
this.files[index].content = content;
await this.fileStorage.writeFile(absolutePath, content);
}

// TODO: we should get the lastModified from the writeFile operation
// instead of calling stat again which could be expensive
const stats = await this.fileStorage.stat(absolutePath);

this.files[index].content = content;

return stats.lastModified;
};

Expand All @@ -434,10 +434,10 @@ export class BotProject {
if (index === -1) {
throw new Error(`no such file at ${relativePath}`);
}
this.files.splice(index, 1);

const absolutePath = `${this.dir}/${relativePath}`;
await this.fileStorage.removeFile(absolutePath);
this.files.splice(index, 1);
};

// ensure dir exist, dir is a absolute dir path
Expand Down
24 changes: 19 additions & 5 deletions Composer/packages/server/src/services/project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import merge from 'lodash/merge';
import find from 'lodash/find';
import { importResolverGenerator, ResolverResource } from '@bfc/shared';
import extractMemoryPaths from '@bfc/indexers/lib/dialogUtils/extractMemoryPaths';
import { UserIdentity } from '@bfc/plugin-loader';

import { BotProject } from '../models/bot/botProject';
Expand Down Expand Up @@ -35,7 +36,7 @@ export class BotProjectService {

public static lgImportResolver(source: string, id: string, projectId: string): ResolverResource {
BotProjectService.initialize();
const project = BotProjectService.currentBotProjects.find(({ id }) => id === projectId);
const project = BotProjectService.getIndexedProjectById(projectId);
if (!project) throw new Error('project not found');
const resource = project.files.reduce((result: ResolverResource[], file) => {
const { name, content } = file;
Expand All @@ -50,7 +51,7 @@ export class BotProjectService {

public static luImportResolver(source: string, id: string, projectId: string): ResolverResource {
BotProjectService.initialize();
const project = BotProjectService.currentBotProjects.find(({ id }) => id === projectId);
const project = BotProjectService.getIndexedProjectById(projectId);
if (!project) throw new Error('project not found');
const resource = project.files.reduce((result: ResolverResource[], file) => {
const { name, content } = file;
Expand Down Expand Up @@ -88,9 +89,13 @@ export class BotProjectService {
'turn.repeatedIds',
'turn.activityProcessed',
];
const projectVariables = BotProjectService.getIndexedProjectById(projectId)
?.files.filter(file => file.name.endsWith('.dialog'))
.map(({ content }) => extractMemoryPaths(content));

const userDefined: string[] =
BotProjectService.currentBotProjects[projectId]?.dialogs.reduce((result: string[], dialog) => {
result = [...dialog.userDefinedVariables, ...result];
projectVariables?.reduce((result: string[], variables) => {
result = [...variables, ...result];
return result;
}, []) || [];
return [...defaultProperties, ...userDefined];
Expand Down Expand Up @@ -180,9 +185,18 @@ export class BotProjectService {
Store.set('projectLocationMap', BotProjectService.projectLocationMap);
};

public static getProjectById = async (projectId: string, user?: UserIdentity) => {
public static getIndexedProjectById(projectId): BotProject | undefined {
// use indexed project
const indexedCurrentProject = BotProjectService.currentBotProjects.find(({ id }) => id === projectId);
if (indexedCurrentProject) return indexedCurrentProject;
}

public static getProjectById = async (projectId: string, user?: UserIdentity): Promise<BotProject> => {
BotProjectService.initialize();

const cachedProject = BotProjectService.getIndexedProjectById(projectId);
if (cachedProject) return cachedProject;

if (!BotProjectService.projectLocationMap?.[projectId]) {
throw new Error('project not found in cache');
} else {
Expand Down

0 comments on commit 6d14ac5

Please sign in to comment.