From 00693c6a66846f946eae8d95f83225e504859701 Mon Sep 17 00:00:00 2001 From: Tyler Butler Date: Wed, 29 Mar 2023 12:39:24 -0700 Subject: [PATCH] Revert "Separate api for getting tenant gitManager. (#14290)" This reverts commit d368f26923fb0379b00451d68b990b4c8e53a9b2. --- .../lambdas/src/deli/lambdaFactory.ts | 7 +- .../lambdas/src/scribe/lambdaFactory.ts | 5 +- .../lambdas/src/test/scribe/lambda.spec.ts | 45 ++--- .../src/alfred/services/deltaService.ts | 16 +- .../packages/services-core/src/tenant.ts | 13 +- .../packages/services-shared/src/storage.ts | 162 ++++++++---------- .../packages/services/src/tenant.ts | 124 ++++++-------- .../test-utils/src/testDocumentStorage.ts | 43 +++-- .../test-utils/src/testTenantManager.ts | 10 +- 9 files changed, 186 insertions(+), 239 deletions(-) diff --git a/server/routerlicious/packages/lambdas/src/deli/lambdaFactory.ts b/server/routerlicious/packages/lambdas/src/deli/lambdaFactory.ts index dac7108b78ba..130ca25fb2fa 100644 --- a/server/routerlicious/packages/lambdas/src/deli/lambdaFactory.ts +++ b/server/routerlicious/packages/lambdas/src/deli/lambdaFactory.ts @@ -97,10 +97,9 @@ export class DeliLambdaFactory extends EventEmitter implements IPartitionLambdaF let gitManager: IGitManager; let document: IDocument; - try { - // Lookup the last sequence number stored - // TODO - is this storage specific to the orderer in place? Or can I generalize the output context? - document = await this.collection.findOne({ documentId, tenantId }); + try { + const tenant = await this.tenantManager.getTenant(tenantId, documentId); + gitManager = tenant.gitManager; // Check if the document was deleted prior. if (!isDocumentValid(document)) { diff --git a/server/routerlicious/packages/lambdas/src/scribe/lambdaFactory.ts b/server/routerlicious/packages/lambdas/src/scribe/lambdaFactory.ts index 7116d5292923..ca3be91ace4d 100644 --- a/server/routerlicious/packages/lambdas/src/scribe/lambdaFactory.ts +++ b/server/routerlicious/packages/lambdas/src/scribe/lambdaFactory.ts @@ -99,8 +99,9 @@ export class ScribeLambdaFactory extends EventEmitter implements IPartitionLambd this.serviceConfiguration, ); - try { - document = await this.documentCollection.findOne({ documentId, tenantId }); + try { + const tenant = await this.tenantManager.getTenant(tenantId, documentId); + gitManager = tenant.gitManager; if (!isDocumentValid(document)) { // Document sessions can be joined (via Alfred) after a document is functionally deleted. diff --git a/server/routerlicious/packages/lambdas/src/test/scribe/lambda.spec.ts b/server/routerlicious/packages/lambdas/src/test/scribe/lambda.spec.ts index 0b65692e1acc..1c36a4b0df11 100644 --- a/server/routerlicious/packages/lambdas/src/test/scribe/lambda.spec.ts +++ b/server/routerlicious/packages/lambdas/src/test/scribe/lambda.spec.ts @@ -73,33 +73,24 @@ describe("Routerlicious", () => { messageFactory = new MessageFactory(testDocumentId, testClientId, testTenantId); kafkaMessageFactory = new KafkaMessageFactory(); - const testData = [ - { - documentId: testDocumentId, - tenantId: testTenantId, - sequenceNumber: 0, - logOffset: undefined, - }, - ]; - const dbFactory = new TestDbFactory(_.cloneDeep({ documents: testData })); - testMongoManager = new MongoManager(dbFactory); - const database = await testMongoManager.getDatabase(); - testDocumentCollection = database.collection("documents"); - testMessageCollection = new TestCollection([]); - testKafka = new TestKafka(); - testProducer = testKafka.createProducer(); - testTenantManager = new TestTenantManager(); - testGitManager = (await testTenantManager.getTenantGitManager( - testTenantId, - testDocumentId, - )) as GitManager; - const createTreeEntry: ICreateTreeEntry[] = []; - const requestBody: ICreateTreeParams = { - tree: createTreeEntry, - }; - tree = await testGitManager.createGitTree(requestBody); - testGitManager.addTree(tree); - const testDeltaManager = new TestDeltaManager(); + const testData = [{ documentId: testDocumentId, tenantId: testTenantId, sequenceNumber: 0, logOffset: undefined }]; + const dbFactory = new TestDbFactory(_.cloneDeep({ documents: testData })); + testMongoManager = new MongoManager(dbFactory); + const database = await testMongoManager.getDatabase(); + testDocumentCollection = database.collection("documents"); + testMessageCollection = new TestCollection([]); + testKafka = new TestKafka(); + testProducer = testKafka.createProducer(); + testTenantManager = new TestTenantManager(); + const tenant = await testTenantManager.getTenant(testTenantId, testDocumentId); + testGitManager = tenant.gitManager as GitManager; + const createTreeEntry: ICreateTreeEntry[] = []; + const requestBody: ICreateTreeParams = { + tree: createTreeEntry, + }; + tree = await testGitManager.createGitTree(requestBody); + testGitManager.addTree(tree); + const testDeltaManager = new TestDeltaManager(); let factory = new ScribeLambdaFactory( testMongoManager, diff --git a/server/routerlicious/packages/routerlicious-base/src/alfred/services/deltaService.ts b/server/routerlicious/packages/routerlicious-base/src/alfred/services/deltaService.ts index 9624cff4c55c..4370b8f1607b 100644 --- a/server/routerlicious/packages/routerlicious-base/src/alfred/services/deltaService.ts +++ b/server/routerlicious/packages/routerlicious-base/src/alfred/services/deltaService.ts @@ -79,14 +79,14 @@ export class DeltaService implements IDeltaService { return dbDeltas.map((delta) => delta.operation); } - public async getDeltasFromSummaryAndStorage( - collectionName: string, - tenantId: string, - documentId: string, - from?: number, - to?: number, - ) { - const gitManager = await this.tenantManager.getTenantGitManager(tenantId, documentId); + public async getDeltasFromSummaryAndStorage( + collectionName: string, + tenantId: string, + documentId: string, + from?: number, + to?: number) { + const tenant = await this.tenantManager.getTenant(tenantId, documentId); + const gitManager = tenant.gitManager; const existingRef = await gitManager.getRef(encodeURIComponent(documentId)); if (!existingRef) { diff --git a/server/routerlicious/packages/services-core/src/tenant.ts b/server/routerlicious/packages/services-core/src/tenant.ts index d50cfc0e715a..71d1cdaae742 100644 --- a/server/routerlicious/packages/services-core/src/tenant.ts +++ b/server/routerlicious/packages/services-core/src/tenant.ts @@ -86,15 +86,10 @@ export interface ITenantManager { */ getTenant(tenantId: string, documentId: string): Promise; - /** - * Retrieves GitManager instance for the given tenant - */ - getTenantGitManager(tenantId: string, documentId: string): Promise; - - /** - * Verifies that the given auth token is valid. A rejected promise indicates an invalid token. - */ - verifyToken(tenantId: string, token: string): Promise; + /** + * Verifies that the given auth token is valid. A rejected promise indicates an invalid token. + */ + verifyToken(tenantId: string, token: string): Promise; /** * Retrieves the key for the given tenant. This is a privileged op and should be used with care. diff --git a/server/routerlicious/packages/services-shared/src/storage.ts b/server/routerlicious/packages/services-shared/src/storage.ts index 5928d10fe8c3..ee67e3129c2f 100644 --- a/server/routerlicious/packages/services-shared/src/storage.ts +++ b/server/routerlicious/packages/services-shared/src/storage.ts @@ -116,20 +116,21 @@ export class DocumentStorage implements IDocumentStorage { }; } - public async createDocument( - tenantId: string, - documentId: string, - appTree: ISummaryTree, - sequenceNumber: number, - term: number, - initialHash: string, - ordererUrl: string, - historianUrl: string, - deltaStreamUrl: string, - values: [string, ICommittedProposal][], - enableDiscovery: boolean = false, - ): Promise { - const gitManager = await this.tenantManager.getTenantGitManager(tenantId, documentId); + public async createDocument( + tenantId: string, + documentId: string, + appTree: ISummaryTree, + sequenceNumber: number, + term: number, + initialHash: string, + ordererUrl: string, + historianUrl: string, + deltaStreamUrl: string, + values: [string, ICommittedProposal][], + enableDiscovery: boolean = false, + ): Promise { + const tenant = await this.tenantManager.getTenant(tenantId, documentId); + const gitManager = tenant.gitManager; const lumberjackProperties = { [BaseTelemetryProperties.tenantId]: tenantId, @@ -278,41 +279,31 @@ export class DocumentStorage implements IDocumentStorage { }; } - public async getVersions( - tenantId: string, - documentId: string, - count: number, - ): Promise { - const gitManager = await this.tenantManager.getTenantGitManager(tenantId, documentId); + public async getVersions(tenantId: string, documentId: string, count: number): Promise { + const tenant = await this.tenantManager.getTenant(tenantId, documentId); + const gitManager = tenant.gitManager; return gitManager.getCommits(documentId, count); } - public async getVersion(tenantId: string, documentId: string, sha: string): Promise { - const gitManager = await this.tenantManager.getTenantGitManager(tenantId, documentId); + public async getVersion(tenantId: string, documentId: string, sha: string): Promise { + const tenant = await this.tenantManager.getTenant(tenantId, documentId); + const gitManager = tenant.gitManager; return gitManager.getCommit(sha); } - public async getFullTree( - tenantId: string, - documentId: string, - ): Promise<{ cache: IGitCache; code: string }> { - const gitManager = await this.tenantManager.getTenantGitManager(tenantId, documentId); - const versions = await gitManager.getCommits(documentId, 1); - if (versions.length === 0) { - return { - cache: { - blobs: [], - commits: [], - refs: { [documentId]: null as unknown as string }, - trees: [], - }, - code: null as unknown as string, - }; - } + public async getFullTree(tenantId: string, documentId: string): Promise<{ cache: IGitCache; code: string; }> { + const tenant = await this.tenantManager.getTenant(tenantId, documentId); + const versions = await tenant.gitManager.getCommits(documentId, 1); + if (versions.length === 0) { + return { + cache: { blobs: [], commits: [], refs: { [documentId]: (null as unknown) as string }, trees: [] }, + code: (null as unknown) as string, + }; + } - const fullTree = await gitManager.getFullTree(versions[0].sha); + const fullTree = await tenant.gitManager.getFullTree(versions[0].sha); let code: string = null as unknown as string; if (fullTree.quorumValues) { @@ -412,51 +403,48 @@ export class DocumentStorage implements IDocumentStorage { } } - private async readFromSummary(tenantId: string, documentId: string): Promise { - const gitManager = await this.tenantManager.getTenantGitManager(tenantId, documentId); - const existingRef = await gitManager.getRef(encodeURIComponent(documentId)); - if (existingRef) { - // Fetch ops from logTail and insert into deltas collection. - // TODO: Make the rest endpoint handle this case. - const opsContent = await gitManager.getContent( - existingRef.object.sha, - ".logTail/logTail", - ); - const ops = JSON.parse( - Buffer.from( - opsContent.content, - Buffer.isEncoding(opsContent.encoding) ? opsContent.encoding : undefined, - ).toString(), - ) as ISequencedDocumentMessage[]; - const dbOps: ISequencedOperationMessage[] = ops.map((op: ISequencedDocumentMessage) => { - return { - documentId, - operation: op, - tenantId, - type: SequencedOperationType, - mongoTimestamp: new Date(op.timestamp), - }; - }); - const opsCollection = await this.databaseManager.getDeltaCollection( - tenantId, - documentId, - ); - await opsCollection.insertMany(dbOps, false).catch(async (error) => { - // Duplicate key errors are ignored - if (error.code !== 11000) { - // Needs to be a full rejection here - return Promise.reject(error); - } - }); - winston.info(`Inserted ${dbOps.length} ops into deltas DB`); - const lumberjackProperties = { - [BaseTelemetryProperties.tenantId]: tenantId, - [BaseTelemetryProperties.documentId]: documentId, - }; - Lumberjack.info(`Inserted ${dbOps.length} ops into deltas DB`, lumberjackProperties); - return true; - } else { - return false; - } - } + private async readFromSummary(tenantId: string, documentId: string): Promise { + const tenant = await this.tenantManager.getTenant(tenantId, documentId); + const gitManager = tenant.gitManager; + const existingRef = await gitManager.getRef(encodeURIComponent(documentId)); + if (existingRef) { + // Fetch ops from logTail and insert into deltas collection. + // TODO: Make the rest endpoint handle this case. + const opsContent = await gitManager.getContent(existingRef.object.sha, ".logTail/logTail"); + const ops = JSON.parse( + Buffer.from( + opsContent.content, + Buffer.isEncoding(opsContent.encoding) ? opsContent.encoding : undefined, + ).toString(), + ) as ISequencedDocumentMessage[]; + const dbOps: ISequencedOperationMessage[] = ops.map((op: ISequencedDocumentMessage) => { + return { + documentId, + operation: op, + tenantId, + type: SequencedOperationType, + mongoTimestamp: new Date(op.timestamp), + }; + }); + const opsCollection = await this.databaseManager.getDeltaCollection(tenantId, documentId); + await opsCollection + .insertMany(dbOps, false) + .catch(async (error) => { + // Duplicate key errors are ignored + if (error.code !== 11000) { + // Needs to be a full rejection here + return Promise.reject(error); + } + }); + winston.info(`Inserted ${dbOps.length} ops into deltas DB`); + const lumberjackProperties = { + [BaseTelemetryProperties.tenantId]: tenantId, + [BaseTelemetryProperties.documentId]: documentId, + }; + Lumberjack.info(`Inserted ${dbOps.length} ops into deltas DB`, lumberjackProperties); + return true; + } else { + return false; + } + } } diff --git a/server/routerlicious/packages/services/src/tenant.ts b/server/routerlicious/packages/services/src/tenant.ts index ebd7f8f0cabc..1d029b740e84 100644 --- a/server/routerlicious/packages/services/src/tenant.ts +++ b/server/routerlicious/packages/services/src/tenant.ts @@ -4,12 +4,11 @@ */ import { - GitManager, - Historian, - ICredentials, - BasicRestWrapper, - getAuthorizationTokenFromCredentials, - IGitManager, + GitManager, + Historian, + ICredentials, + BasicRestWrapper, + getAuthorizationTokenFromCredentials, } from "@fluidframework/server-services-client"; import { generateToken, getCorrelationId } from "@fluidframework/server-services-utils"; import * as core from "@fluidframework/server-services-core"; @@ -20,9 +19,9 @@ export class Tenant implements core.ITenant { return this.config.id; } - public get gitManager(): IGitManager { - return this.manager; - } + public get gitManager(): GitManager { + return this.manager; + } public get storage(): core.ITenantStorage { return this.config.storage; @@ -32,10 +31,8 @@ export class Tenant implements core.ITenant { return this.config.orderer; } - constructor( - private readonly config: core.ITenantConfig, - private readonly manager: IGitManager, - ) {} + constructor(private readonly config: core.ITenantConfig, private readonly manager: GitManager) { + } } /** @@ -53,66 +50,47 @@ export class TenantManager implements core.ITenantManager { return result; } - public async getTenant( - tenantId: string, - documentId: string, - includeDisabledTenant = false, - ): Promise { - const restWrapper = new BasicRestWrapper(); - const [details, gitManager] = await Promise.all([ - restWrapper.get(`${this.endpoint}/api/tenants/${tenantId}`, { - includeDisabledTenant, - }), - this.getTenantGitManager(tenantId, documentId, includeDisabledTenant), - ]); - - const tenant = new Tenant(details, gitManager); - - return tenant; - } - - public async getTenantGitManager( - tenantId: string, - documentId: string, - includeDisabledTenant = false, - ): Promise { - const key = await this.getKey(tenantId, includeDisabledTenant); - - const defaultQueryString = { - token: fromUtf8ToBase64(`${tenantId}`), - }; - const getDefaultHeaders = () => { - const credentials: ICredentials = { - password: generateToken(tenantId, documentId, key, null), - user: tenantId, - }; - return { - Authorization: getAuthorizationTokenFromCredentials(credentials), - }; - }; - const defaultHeaders = getDefaultHeaders(); - const baseUrl = `${this.internalHistorianUrl}/repos/${encodeURIComponent(tenantId)}`; - const tenantRestWrapper = new BasicRestWrapper( - baseUrl, - defaultQueryString, - undefined, - undefined, - defaultHeaders, - undefined, - undefined, - getDefaultHeaders, - getCorrelationId, - ); - const historian = new Historian( - `${this.internalHistorianUrl}/repos/${encodeURIComponent(tenantId)}`, - true, - false, - tenantRestWrapper, - ); - const gitManager = new GitManager(historian); - - return gitManager; - } + public async getTenant(tenantId: string, documentId: string, includeDisabledTenant = false): Promise { + const restWrapper = new BasicRestWrapper(); + const [details, key] = await Promise.all([ + restWrapper.get(`${this.endpoint}/api/tenants/${tenantId}`, + { includeDisabledTenant }), + this.getKey(tenantId, includeDisabledTenant)]); + + const defaultQueryString = { + token: fromUtf8ToBase64(`${tenantId}`), + }; + const getDefaultHeaders = () => { + const credentials: ICredentials = { + password: generateToken(tenantId, documentId, key, null), + user: tenantId, + }; + return ({ + Authorization: getAuthorizationTokenFromCredentials(credentials), + }); + }; + const defaultHeaders = getDefaultHeaders(); + const baseUrl = `${this.internalHistorianUrl}/repos/${encodeURIComponent(tenantId)}`; + const tenantRestWrapper = new BasicRestWrapper( + baseUrl, + defaultQueryString, + undefined, + undefined, + defaultHeaders, + undefined, + undefined, + getDefaultHeaders, + getCorrelationId); + const historian = new Historian( + `${this.internalHistorianUrl}/repos/${encodeURIComponent(tenantId)}`, + true, + false, + tenantRestWrapper); + const gitManager = new GitManager(historian); + const tenant = new Tenant(details, gitManager); + + return tenant; + } public async verifyToken(tenantId: string, token: string): Promise { const restWrapper = new BasicRestWrapper(); diff --git a/server/routerlicious/packages/test-utils/src/testDocumentStorage.ts b/server/routerlicious/packages/test-utils/src/testDocumentStorage.ts index c310748474ae..87d1383b154c 100644 --- a/server/routerlicious/packages/test-utils/src/testDocumentStorage.ts +++ b/server/routerlicious/packages/test-utils/src/testDocumentStorage.ts @@ -60,20 +60,21 @@ export class TestDocumentStorage implements IDocumentStorage { return getOrCreateP; } - public async createDocument( - tenantId: string, - documentId: string, - summary: ISummaryTree, - sequenceNumber: number, - term: number, - initialHash: string, - ordererUrl: string, - historianUrl: string, - deltaStreamUrl: string, - values: [string, ICommittedProposal][], - enableDiscovery: boolean = false, - ): Promise { - const gitManager = await this.tenantManager.getTenantGitManager(tenantId, documentId); + public async createDocument( + tenantId: string, + documentId: string, + summary: ISummaryTree, + sequenceNumber: number, + term: number, + initialHash: string, + ordererUrl: string, + historianUrl: string, + deltaStreamUrl: string, + values: [string, ICommittedProposal][], + enableDiscovery: boolean = false, + ): Promise { + const tenant = await this.tenantManager.getTenant(tenantId, documentId); + const gitManager = tenant.gitManager; const blobsShaCache = new Set(); const handle = await writeSummaryTree(gitManager, summary, blobsShaCache, undefined); @@ -193,18 +194,16 @@ export class TestDocumentStorage implements IDocumentStorage { }; } - public async getVersions( - tenantId: string, - documentId: string, - count: number, - ): Promise { - const gitManager = await this.tenantManager.getTenantGitManager(tenantId, documentId); + public async getVersions(tenantId: string, documentId: string, count: number): Promise { + const tenant = await this.tenantManager.getTenant(tenantId, documentId); + const gitManager = tenant.gitManager; return gitManager.getCommits(documentId, count); } - public async getVersion(tenantId: string, documentId: string, sha: string): Promise { - const gitManager = await this.tenantManager.getTenantGitManager(tenantId, documentId); + public async getVersion(tenantId: string, documentId: string, sha: string): Promise { + const tenant = await this.tenantManager.getTenant(tenantId, documentId); + const gitManager = tenant.gitManager; return gitManager.getCommit(sha); } diff --git a/server/routerlicious/packages/test-utils/src/testTenantManager.ts b/server/routerlicious/packages/test-utils/src/testTenantManager.ts index 17d6444622a7..9fd812236698 100644 --- a/server/routerlicious/packages/test-utils/src/testTenantManager.ts +++ b/server/routerlicious/packages/test-utils/src/testTenantManager.ts @@ -71,11 +71,7 @@ export class TestTenantManager implements ITenantManager { return this.tenant; } - public async getTenantGitManager(tenantId: string, documentId: string): Promise { - return this.tenant.gitManager; - } - - public async getKey(tenantId: string): Promise { - return "test"; - } + public async getKey(tenantId: string): Promise { + return "test"; + } }