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.

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

Round-robin mail details calls when exporting #8421

Open
wants to merge 2 commits into
base: dev-mail
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions src/common/api/worker/facades/lazy/MailExportFacade.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ export class MailExportFacade {
)
}

async loadMailDetails(mails: readonly Mail[]): Promise<MailWithMailDetails[]> {
return this.mailExportTokenFacade.loadWithToken((token) => this.bulkMailLoader.loadMailDetails(mails, this.options(token)))
async loadMailDetails(mails: readonly Mail[], baseUrl: string): Promise<MailWithMailDetails[]> {
return this.mailExportTokenFacade.loadWithToken((token) => this.bulkMailLoader.loadMailDetails(mails, { baseUrl, ...this.options(token) }))
}

async loadAttachments(mails: readonly Mail[], baseUrl: string): Promise<TutanotaFile[]> {
Expand Down
21 changes: 16 additions & 5 deletions src/common/api/worker/rest/EntityRestClient.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { RestClient, SuspensionBehavior } from "./RestClient"
import type { RestClient, SuspensionBehavior } from "./RestClient"
import type { CryptoFacade } from "../crypto/CryptoFacade"
import { _verifyType, HttpMethod, MediaType, resolveTypeReference } from "../../common/EntityFunctions"
import { SessionKeyNotFoundError } from "../../common/error/SessionKeyNotFoundError"
Expand Down Expand Up @@ -287,7 +287,7 @@ export class EntityRestClient implements EntityRestInterface {
}
let json: string
if (typeModel.type === Type.BlobElement) {
json = await this.loadMultipleBlobElements(listId, queryParams, headers, path, typeRef, opts.suspensionBehavior)
json = await this.loadMultipleBlobElements(listId, queryParams, headers, path, typeRef, opts)
} else {
json = await this.restClient.request(path, HttpMethod.GET, {
queryParams,
Expand All @@ -308,7 +308,7 @@ export class EntityRestClient implements EntityRestInterface {
headers: Dict | undefined,
path: string,
typeRef: TypeRef<any>,
suspensionBehavior?: SuspensionBehavior,
opts: EntityRestClientLoadOptions = {},
): Promise<string> {
if (archiveId == null) {
throw new Error("archiveId must be set to load BlobElementTypes")
Expand All @@ -321,16 +321,27 @@ export class EntityRestClient implements EntityRestInterface {
queryParams,
)
const allParams = await this.blobAccessTokenFacade.createQueryParams(blobServerAccessInfo, additionalRequestParams, typeRef)

let serversToTry = blobServerAccessInfo.servers
if (opts.baseUrl) {
const preferredServer = blobServerAccessInfo.servers.find((server) => server.url === opts.baseUrl)

if (preferredServer) {
// preferredServer takes precedence over the rest
serversToTry = [preferredServer, ...blobServerAccessInfo.servers.filter((server) => server.url !== opts.baseUrl)]
}
}

return tryServers(
blobServerAccessInfo.servers,
serversToTry,
async (serverUrl) =>
this.restClient.request(path, HttpMethod.GET, {
queryParams: allParams,
headers: {}, // prevent CORS request due to non standard header usage
responseType: MediaType.Json,
baseUrl: serverUrl,
noCORS: true,
suspensionBehavior,
suspensionBehavior: opts.suspensionBehavior,
}),
`can't load instances from server `,
)
Expand Down
2 changes: 1 addition & 1 deletion src/mail-app/native/main/MailExportController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ export class MailExportController {
break
}

const downloadedMailDetails = await this.mailExportFacade.loadMailDetails(downloadedMails)
const downloadedMailDetails = await this.mailExportFacade.loadMailDetails(downloadedMails, this.getServerUrl())
const attachmentInfo = await this.mailExportFacade.loadAttachments(downloadedMails, this.getServerUrl())
for (const { mail, mailDetails } of downloadedMailDetails) {
if (this._state().type !== "exporting") {
Expand Down
3 changes: 2 additions & 1 deletion src/mail-app/workerUtils/index/BulkMailLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ export class BulkMailLoader {
}

async loadMailDetails(mails: readonly Mail[], options: EntityRestClientLoadOptions = {}): Promise<MailWithMailDetails[]> {
const { baseUrl: _, ...optsWithoutBaseUrl } = options
const result: Array<MailWithMailDetails> = []
// mailDetails stored as blob
let mailDetailsBlobMails = mails.filter((m) => !isDraft(m))
Expand Down Expand Up @@ -96,7 +97,7 @@ export class BulkMailLoader {
encryptingKeyVersion: Number(mail._ownerKeyVersion ?? 0),
}
}
const mailDetailsDrafts = await this.loadInChunks(MailDetailsDraftTypeRef, listId, ids, ownerEncSessionKeyProvider, options)
const mailDetailsDrafts = await this.loadInChunks(MailDetailsDraftTypeRef, listId, ids, ownerEncSessionKeyProvider, optsWithoutBaseUrl)
result.push(
...mailDetailsDrafts.map((draftDetails) => {
const mail = assertNotNull(mailDetailsDraftMails.find((m) => isSameId(m.mailDetailsDraft, draftDetails._id)))
Expand Down
10 changes: 8 additions & 2 deletions test/tests/api/worker/facades/MailExportFacadeTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,15 @@ o.spec("MailExportFacade", () => {
{ mail: mail1, mailDetails: details1 },
{ mail: mail2, mailDetails: details2 },
]
when(bulkMailLoader.loadMailDetails([mail1, mail2], { extraHeaders: tokenHeaders, suspensionBehavior: SuspensionBehavior.Throw })).thenResolve(expected)
when(
bulkMailLoader.loadMailDetails([mail1, mail2], {
baseUrl: "baseUrl",
extraHeaders: tokenHeaders,
suspensionBehavior: SuspensionBehavior.Throw,
}),
).thenResolve(expected)

const result = await facade.loadMailDetails([mail1, mail2])
const result = await facade.loadMailDetails([mail1, mail2], "baseUrl")

o(result).deepEquals(expected)
})
Expand Down
21 changes: 17 additions & 4 deletions test/tests/native/main/MailExportControllerTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ o.spec("MailExportController", function () {
const attachmentData = new Uint8Array([1, 2, 3])
const dataFile = createDataFile("test", "application/octet-stream", attachmentData)
when(mailExportFacade.loadFixedNumberOfMailsWithCache(mailBag.mails, startId, matchers.anything())).thenResolve([mail])
when(mailExportFacade.loadMailDetails([mail])).thenResolve([{ mail, mailDetails }])
when(mailExportFacade.loadMailDetails([mail], matchers.anything())).thenResolve([{ mail, mailDetails }])
when(mailExportFacade.loadAttachments([mail], matchers.anything())).thenResolve([attachmentInfo])
when(mailExportFacade.loadAttachmentData(mail, [attachmentInfo])).thenResolve([dataFile])

Expand Down Expand Up @@ -217,7 +217,13 @@ o.spec("MailExportController", function () {
const archivedMailBag2 = mailboxDetail.mailbox.archivedMailBags[1]
const { mail: mail3, mailBundle: mailBundle3 } = prepareMailData(archivedMailBag2, GENERATED_MAX_ID, 3)
when(mailExportFacade.loadFixedNumberOfMailsWithCache(archivedMailBag2.mails, getElementId(mail3), "baseUrl")).thenResolve([])
when(mailExportFacade.getExportServers(mailboxDetail.mailGroup)).thenResolve([{ _id: "id", url: "baseUrl", _type: BlobServerUrlTypeRef }])
when(mailExportFacade.getExportServers(mailboxDetail.mailGroup)).thenResolve([
{
_id: "id",
url: "baseUrl",
_type: BlobServerUrlTypeRef,
},
])

await controller.startExport(mailboxDetail)

Expand All @@ -241,14 +247,21 @@ o.spec("MailExportController", function () {
await controller.startExport(mailboxDetail)

verify(mailExportFacade.loadFixedNumberOfMailsWithCache(currentMailBag.mails, GENERATED_MAX_ID, "baseUrl2"))
verify(mailExportFacade.loadAttachments([mail1], "baseUrl3"))
verify(mailExportFacade.loadMailDetails([mail1], "baseUrl3"))
verify(mailExportFacade.loadAttachments([mail1], "baseUrl1"))
verify(mailExportFacade.loadAttachmentData(mail1, matchers.anything()))
})
})

o.spec("handle errors", function () {
o.test("SuspensionError", async () => {
when(mailExportFacade.getExportServers(mailboxDetail.mailGroup)).thenResolve([{ _id: "id", url: "baseUrl", _type: BlobServerUrlTypeRef }])
when(mailExportFacade.getExportServers(mailboxDetail.mailGroup)).thenResolve([
{
_id: "id",
url: "baseUrl",
_type: BlobServerUrlTypeRef,
},
])
let wasThrown = false
when(mailExportFacade.loadFixedNumberOfMailsWithCache(matchers.anything(), matchers.anything(), matchers.anything())).thenDo(() => {
if (wasThrown) {
Expand Down