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

chore: vault tests refactors and lint #2374

Merged
merged 57 commits into from
Aug 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
57 commits
Select commit Hold shift + click to select a range
2abc426
chore: enable vault tests by default
Jul 27, 2023
d51ac08
chore: sleep reason
moughxyz Jul 28, 2023
d96f71b
chore: debug
moughxyz Jul 28, 2023
af73853
chore: top level only
moughxyz Jul 28, 2023
0e0dec5
chore: no only
moughxyz Jul 28, 2023
b24776a
chore: debug
moughxyz Jul 28, 2023
a75a0a3
chore: trigger run
moughxyz Jul 28, 2023
a84157c
chore: trigger run
moughxyz Jul 28, 2023
84be31d
chore: trigger run
moughxyz Jul 28, 2023
ab3eb4d
chore: trigger run
moughxyz Jul 28, 2023
ea3c0d9
chore: trigger run
moughxyz Jul 28, 2023
6532c6f
chore: trigger run
moughxyz Jul 28, 2023
90cb0bd
chore: trigger run
moughxyz Jul 28, 2023
3386f46
chore: debug
moughxyz Jul 28, 2023
854703b
refactor: stub function
moughxyz Jul 28, 2023
17b7fe4
chore: error handling
moughxyz Jul 28, 2023
919a2b6
refactor: unify keypair change handling
moughxyz Jul 28, 2023
368f08c
chore: lint
moughxyz Jul 29, 2023
0579991
refactor: exclude certain message types from being resent
moughxyz Jul 29, 2023
1a76616
refactor: more granular control of when sync occurs
moughxyz Jul 29, 2023
c695e25
chore: tests
moughxyz Jul 29, 2023
d5d8369
chore: unskip tests
moughxyz Jul 30, 2023
27a0a7d
fix: handle messages only once
moughxyz Jul 30, 2023
73659ce
refactor: rename user event to notification
moughxyz Jul 30, 2023
13d777e
fix: test timing
moughxyz Jul 30, 2023
ca95a95
chore: disable exclusive
moughxyz Jul 30, 2023
7b915e2
refactor: test utils
moughxyz Jul 30, 2023
a2c1778
refactor: use case
moughxyz Jul 31, 2023
11470d0
chore: test improve
moughxyz Jul 31, 2023
7fd0d7a
chore: merge main
moughxyz Jul 31, 2023
f74bddf
chore: fix test
moughxyz Aug 1, 2023
105c8e8
chore: enable shared vault files tests
Aug 1, 2023
275ef5e
fix: test
moughxyz Aug 1, 2023
e5f960b
fix: await promise
moughxyz Aug 1, 2023
ec313bc
fix: test
moughxyz Aug 1, 2023
d87fa73
chore: test timeout
moughxyz Aug 1, 2023
c7ec4ea
chore: vaults context
moughxyz Aug 1, 2023
9d15cb0
refactor: test
moughxyz Aug 1, 2023
0f5d7b6
fix: test
moughxyz Aug 1, 2023
f9f7310
refactor: tests
moughxyz Aug 1, 2023
e6afa51
refactor: tests
moughxyz Aug 1, 2023
f707bb5
fix: test timeouts
moughxyz Aug 1, 2023
fe00a8c
refactor: tests
moughxyz Aug 1, 2023
b724a20
refactor: tests
moughxyz Aug 1, 2023
6fa9579
chore: refactor test and add new
moughxyz Aug 1, 2023
92468d8
chore: fix specs
moughxyz Aug 1, 2023
597a37b
chore: fix test memory leak
moughxyz Aug 1, 2023
da60938
feat: logger class
moughxyz Aug 1, 2023
b6f1619
chore: merge main
moughxyz Aug 1, 2023
fba0303
chore: flag
moughxyz Aug 1, 2023
5a405c8
chore: fix test
moughxyz Aug 1, 2023
2b38223
Merge branch 'main' of github.com:standardnotes/app into enalbe_vault…
moughxyz Aug 1, 2023
4e54b41
chore: flag
moughxyz Aug 1, 2023
750ae08
fix: test
moughxyz Aug 1, 2023
0e59698
fix: test deinit
moughxyz Aug 1, 2023
440ac80
chore: lint
moughxyz Aug 1, 2023
44f2b50
chore: lint tests
moughxyz Aug 1, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion packages/api/src/Domain/Http/FetchRequestHandler.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@ import { FetchRequestHandler } from './FetchRequestHandler'
import { HttpErrorResponseBody, HttpRequest } from '@standardnotes/responses'

import { ErrorMessage } from '../Error'
import { LoggerInterface } from '@standardnotes/utils'

describe('FetchRequestHandler', () => {
const snjsVersion = 'snjsVersion'
const appVersion = 'appVersion'
const environment = Environment.Web
const requestHandler = new FetchRequestHandler(snjsVersion, appVersion, environment)
const logger: LoggerInterface = {} as jest.Mocked<LoggerInterface>
const requestHandler = new FetchRequestHandler(snjsVersion, appVersion, environment, logger)

it('should create a request', () => {
const httpRequest: HttpRequest = {
Expand Down
4 changes: 3 additions & 1 deletion packages/api/src/Domain/Http/FetchRequestHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,14 @@ import { RequestHandlerInterface } from './RequestHandlerInterface'
import { Environment } from '@standardnotes/models'
import { isString } from 'lodash'
import { ErrorMessage } from '../Error'
import { LoggerInterface } from '@standardnotes/utils'

export class FetchRequestHandler implements RequestHandlerInterface {
constructor(
protected readonly snjsVersion: string,
protected readonly appVersion: string,
protected readonly environment: Environment,
private logger: LoggerInterface,
) {}

async handleRequest<T>(httpRequest: HttpRequest): Promise<HttpResponse<T>> {
Expand Down Expand Up @@ -122,7 +124,7 @@ export class FetchRequestHandler implements RequestHandlerInterface {
}
}
} catch (error) {
console.error(error)
this.logger.error(JSON.stringify(error))
}

if (httpStatus >= HttpStatusCode.Success && httpStatus < HttpStatusCode.InternalServerError) {
Expand Down
10 changes: 8 additions & 2 deletions packages/api/src/Domain/Http/HttpService.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { joinPaths, sleep } from '@standardnotes/utils'
import { LoggerInterface, joinPaths, sleep } from '@standardnotes/utils'
import { Environment } from '@standardnotes/models'
import { LegacySession, Session, SessionToken } from '@standardnotes/domain-core'
import {
Expand All @@ -21,6 +21,7 @@ export class HttpService implements HttpServiceInterface {
private session?: Session | LegacySession
private __latencySimulatorMs?: number
private declare host: string
loggingEnabled = false

private inProgressRefreshSessionPromise?: Promise<boolean>
private updateMetaCallback!: (meta: HttpResponseMeta) => void
Expand All @@ -32,8 +33,9 @@ export class HttpService implements HttpServiceInterface {
private environment: Environment,
private appVersion: string,
private snjsVersion: string,
private logger: LoggerInterface,
) {
this.requestHandler = new FetchRequestHandler(this.snjsVersion, this.appVersion, this.environment)
this.requestHandler = new FetchRequestHandler(this.snjsVersion, this.appVersion, this.environment, this.logger)
}

setCallbacks(
Expand Down Expand Up @@ -150,6 +152,10 @@ export class HttpService implements HttpServiceInterface {

const response = await this.requestHandler.handleRequest<T>(httpRequest)

if (this.loggingEnabled && isErrorResponse(response)) {
this.logger.error('Request failed', httpRequest, response)
}

if (response.meta && !httpRequest.external) {
this.updateMetaCallback?.(response.meta)
}
Expand Down
5 changes: 0 additions & 5 deletions packages/mobile/.mocharc.yml

This file was deleted.

4 changes: 2 additions & 2 deletions packages/responses/src/Domain/Item/RawSyncData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { ApiEndpointParam } from './ApiEndpointParam'
import { ConflictParams } from './ConflictParams'
import { ServerItemResponse } from './ServerItemResponse'
import { SharedVaultServerHash } from '../SharedVaults/SharedVaultServerHash'
import { UserEventServerHash } from '../UserEvent/UserEventServerHash'
import { NotificationServerHash } from '../Notification/NotificationServerHash'
import { AsymmetricMessageServerHash } from '../AsymmetricMessage/AsymmetricMessageServerHash'

export type RawSyncData = {
Expand All @@ -16,7 +16,7 @@ export type RawSyncData = {
unsaved?: ConflictParams[]
shared_vaults?: SharedVaultServerHash[]
shared_vault_invites?: SharedVaultInviteServerHash[]
notifications?: UserEventServerHash[]
notifications?: NotificationServerHash[]
messages?: AsymmetricMessageServerHash[]
status?: number
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
export type UserEventServerHash = {
export type NotificationServerHash = {
uuid: string
user_uuid: string
type: string
Expand Down
2 changes: 1 addition & 1 deletion packages/responses/src/Domain/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,4 +66,4 @@ export * from './User/PostSubscriptionTokensResponse'
export * from './User/SettingData'
export * from './User/UpdateSettingResponse'

export * from './UserEvent/UserEventServerHash'
export * from './Notification/NotificationServerHash'
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
KeySystemRootKeyContentSpecialized,
TrustedContactInterface,
} from '@standardnotes/models'
import { Result } from '@standardnotes/domain-core'

describe('AsymmetricMessageService', () => {
let sync: jest.Mocked<SyncServiceInterface>
Expand Down Expand Up @@ -61,6 +62,7 @@ describe('AsymmetricMessageService', () => {
encryption,
mutator,
sessions,
sync,
messageServer,
createOrEditContact,
findContact,
Expand Down Expand Up @@ -115,6 +117,45 @@ describe('AsymmetricMessageService', () => {
})
})

describe('handleTrustedMessageResult', () => {
it('should not double handle the same message', async () => {
/**
* Because message retrieval is based on a syncToken, and the server aligns syncTokens to items sent back
* rather than messages, we may receive the same message twice. We want to keep track of processed messages
* and avoid double processing.
*/

const message: AsymmetricMessageServerHash = {
uuid: 'message',
recipient_uuid: '1',
sender_uuid: '2',
encrypted_message: 'encrypted_message',
created_at_timestamp: 2,
updated_at_timestamp: 2,
}

const decryptedMessagePayload: AsymmetricMessageTrustedContactShare = {
type: AsymmetricMessagePayloadType.ContactShare,
data: {
recipientUuid: '1',
trustedContact: {} as TrustedContactInterface,
},
}

service.getTrustedMessagePayload = service.getUntrustedMessagePayload = jest
.fn()
.mockReturnValue(Result.ok(decryptedMessagePayload))

service.handleTrustedContactShareMessage = jest.fn()
await service.handleTrustedMessageResult(message, decryptedMessagePayload)
expect(service.handleTrustedContactShareMessage).toHaveBeenCalledTimes(1)

service.handleTrustedContactShareMessage = jest.fn()
await service.handleTrustedMessageResult(message, decryptedMessagePayload)
expect(service.handleTrustedContactShareMessage).toHaveBeenCalledTimes(0)
})
})

it('should process incoming messages oldest first', async () => {
const messages: AsymmetricMessageServerHash[] = [
{
Expand All @@ -139,7 +180,7 @@ describe('AsymmetricMessageService', () => {

service.getTrustedMessagePayload = service.getUntrustedMessagePayload = jest
.fn()
.mockReturnValue(trustedPayloadMock)
.mockReturnValue(Result.ok(trustedPayloadMock))

const handleTrustedContactShareMessageMock = jest.fn()
service.handleTrustedContactShareMessage = handleTrustedContactShareMessageMock
Expand Down Expand Up @@ -171,7 +212,7 @@ describe('AsymmetricMessageService', () => {
service.handleTrustedContactShareMessage = jest.fn()
service.getTrustedMessagePayload = service.getUntrustedMessagePayload = jest
.fn()
.mockReturnValue(decryptedMessagePayload)
.mockReturnValue(Result.ok(decryptedMessagePayload))

await service.handleRemoteReceivedAsymmetricMessages([message])

Expand Down Expand Up @@ -200,7 +241,7 @@ describe('AsymmetricMessageService', () => {
service.handleTrustedSenderKeypairChangedMessage = jest.fn()
service.getTrustedMessagePayload = service.getUntrustedMessagePayload = jest
.fn()
.mockReturnValue(decryptedMessagePayload)
.mockReturnValue(Result.ok(decryptedMessagePayload))

await service.handleRemoteReceivedAsymmetricMessages([message])

Expand Down Expand Up @@ -228,7 +269,7 @@ describe('AsymmetricMessageService', () => {
service.handleTrustedSharedVaultRootKeyChangedMessage = jest.fn()
service.getTrustedMessagePayload = service.getUntrustedMessagePayload = jest
.fn()
.mockReturnValue(decryptedMessagePayload)
.mockReturnValue(Result.ok(decryptedMessagePayload))

await service.handleRemoteReceivedAsymmetricMessages([message])

Expand Down Expand Up @@ -258,7 +299,7 @@ describe('AsymmetricMessageService', () => {
service.handleTrustedVaultMetadataChangedMessage = jest.fn()
service.getTrustedMessagePayload = service.getUntrustedMessagePayload = jest
.fn()
.mockReturnValue(decryptedMessagePayload)
.mockReturnValue(Result.ok(decryptedMessagePayload))

await service.handleRemoteReceivedAsymmetricMessages([message])

Expand All @@ -284,7 +325,7 @@ describe('AsymmetricMessageService', () => {

service.getTrustedMessagePayload = service.getUntrustedMessagePayload = jest
.fn()
.mockReturnValue(decryptedMessagePayload)
.mockReturnValue(Result.ok(decryptedMessagePayload))

await expect(service.handleRemoteReceivedAsymmetricMessages([message])).rejects.toThrow(
'Shared vault invites payloads are not handled as part of asymmetric messages',
Expand Down Expand Up @@ -313,7 +354,7 @@ describe('AsymmetricMessageService', () => {
service.handleTrustedContactShareMessage = jest.fn()
service.getTrustedMessagePayload = service.getUntrustedMessagePayload = jest
.fn()
.mockReturnValue(decryptedMessagePayload)
.mockReturnValue(Result.ok(decryptedMessagePayload))

await service.handleRemoteReceivedAsymmetricMessages([message])

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { SyncServiceInterface } from './../Sync/SyncServiceInterface'
import { SessionsClientInterface } from './../Session/SessionsClientInterface'
import { MutatorClientInterface } from './../Mutator/MutatorClientInterface'
import { AsymmetricMessageServerHash, ClientDisplayableError, isClientDisplayableError } from '@standardnotes/responses'
import { AsymmetricMessageServerHash } from '@standardnotes/responses'
import { SyncEvent, SyncEventReceivedAsymmetricMessagesData } from '../Event/SyncEvent'
import { InternalEventBusInterface } from '../Internal/InternalEventBusInterface'
import { InternalEventHandlerInterface } from '../Internal/InternalEventHandlerInterface'
Expand All @@ -20,7 +21,6 @@ import {
VaultListingInterface,
} from '@standardnotes/models'
import { HandleRootKeyChangedMessage } from './UseCase/HandleRootKeyChangedMessage'
import { SessionEvent } from '../Session/SessionEvent'
import { AsymmetricMessageServer } from '@standardnotes/api'
import { GetOutboundMessages } from './UseCase/GetOutboundMessages'
import { GetInboundMessages } from './UseCase/GetInboundMessages'
Expand All @@ -31,15 +31,19 @@ import { FindContact } from '../Contacts/UseCase/FindContact'
import { CreateOrEditContact } from '../Contacts/UseCase/CreateOrEditContact'
import { ReplaceContactData } from '../Contacts/UseCase/ReplaceContactData'
import { EncryptionProviderInterface } from '../Encryption/EncryptionProviderInterface'
import { Result } from '@standardnotes/domain-core'

export class AsymmetricMessageService
extends AbstractService
implements AsymmetricMessageServiceInterface, InternalEventHandlerInterface
{
private handledMessages = new Set<string>()

constructor(
private encryption: EncryptionProviderInterface,
private mutator: MutatorClientInterface,
private sessions: SessionsClientInterface,
private sync: SyncServiceInterface,
private messageServer: AsymmetricMessageServer,
private _createOrEditContact: CreateOrEditContact,
private _findContact: FindContact,
Expand Down Expand Up @@ -73,30 +77,27 @@ export class AsymmetricMessageService

async handleEvent(event: InternalEventInterface): Promise<void> {
switch (event.type) {
case SessionEvent.UserKeyPairChanged:
void this.messageServer.deleteAllInboundMessages()
break
case SyncEvent.ReceivedAsymmetricMessages:
void this.handleRemoteReceivedAsymmetricMessages(event.payload as SyncEventReceivedAsymmetricMessagesData)
break
}
}

public async getOutboundMessages(): Promise<AsymmetricMessageServerHash[] | ClientDisplayableError> {
public async getOutboundMessages(): Promise<Result<AsymmetricMessageServerHash[]>> {
return this._getOutboundMessagesUseCase.execute()
}

public async getInboundMessages(): Promise<AsymmetricMessageServerHash[] | ClientDisplayableError> {
public async getInboundMessages(): Promise<Result<AsymmetricMessageServerHash[]>> {
return this._getInboundMessagesUseCase.execute()
}

public async downloadAndProcessInboundMessages(): Promise<void> {
const messages = await this.getInboundMessages()
if (isClientDisplayableError(messages)) {
if (messages.isFailed()) {
return
}

await this.handleRemoteReceivedAsymmetricMessages(messages)
await this.handleRemoteReceivedAsymmetricMessages(messages.getValue())
}

sortServerMessages(messages: AsymmetricMessageServerHash[]): AsymmetricMessageServerHash[] {
Expand Down Expand Up @@ -143,11 +144,11 @@ export class AsymmetricMessageService
getServerMessageType(message: AsymmetricMessageServerHash): AsymmetricMessagePayloadType | undefined {
const result = this.getUntrustedMessagePayload(message)

if (!result) {
if (result.isFailed()) {
return undefined
}

return result.type
return result.getValue().type
}

async handleRemoteReceivedAsymmetricMessages(messages: AsymmetricMessageServerHash[]): Promise<void> {
Expand All @@ -159,18 +160,26 @@ export class AsymmetricMessageService

for (const message of sortedMessages) {
const trustedPayload = this.getTrustedMessagePayload(message)
if (!trustedPayload) {
if (trustedPayload.isFailed()) {
continue
}

await this.handleTrustedMessageResult(message, trustedPayload)
await this.handleTrustedMessageResult(message, trustedPayload.getValue())
}

void this.sync.sync()
}

private async handleTrustedMessageResult(
async handleTrustedMessageResult(
message: AsymmetricMessageServerHash,
payload: AsymmetricMessagePayload,
): Promise<void> {
if (this.handledMessages.has(message.uuid)) {
return
}

this.handledMessages.add(message.uuid)

if (payload.type === AsymmetricMessagePayloadType.ContactShare) {
await this.handleTrustedContactShareMessage(message, payload)
} else if (payload.type === AsymmetricMessagePayloadType.SenderKeypairChanged) {
Expand All @@ -186,23 +195,23 @@ export class AsymmetricMessageService
await this.deleteMessageAfterProcessing(message)
}

getUntrustedMessagePayload(message: AsymmetricMessageServerHash): AsymmetricMessagePayload | undefined {
getUntrustedMessagePayload(message: AsymmetricMessageServerHash): Result<AsymmetricMessagePayload> {
const result = this._getUntrustedPayload.execute({
privateKey: this.encryption.getKeyPair().privateKey,
message,
})

if (result.isFailed()) {
return undefined
return Result.fail(result.getError())
}

return result.getValue()
return result
}

getTrustedMessagePayload(message: AsymmetricMessageServerHash): AsymmetricMessagePayload | undefined {
getTrustedMessagePayload(message: AsymmetricMessageServerHash): Result<AsymmetricMessagePayload> {
const contact = this._findContact.execute({ userUuid: message.sender_uuid })
if (contact.isFailed()) {
return undefined
return Result.fail(contact.getError())
}

const result = this._getTrustedPayload.execute({
Expand All @@ -213,10 +222,10 @@ export class AsymmetricMessageService
})

if (result.isFailed()) {
return undefined
return Result.fail(result.getError())
}

return result.getValue()
return result
}

async deleteMessageAfterProcessing(message: AsymmetricMessageServerHash): Promise<void> {
Expand Down