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

refactor!: migrate from .afj to .credo #2161

Closed
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
1 change: 1 addition & 0 deletions packages/core/src/storage/FileSystem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,5 @@ export interface FileSystem {
read(path: string): Promise<string>
delete(path: string): Promise<void>
downloadToFile(url: string, path: string, options?: DownloadToFileOptions): Promise<void>
migrateWalletToCredoFolder(walletId: string): Promise<void>
}
Original file line number Diff line number Diff line change
Expand Up @@ -757,7 +757,7 @@ exports[`UpdateAssistant | v0.2 - v0.3.1 should correctly update the did records
"createdAt": "2022-09-08T19:35:53.872Z",
"id": "STORAGE_VERSION_RECORD_ID",
"metadata": {},
"storageVersion": "0.5",
"storageVersion": "0.6",
"updatedAt": "2023-01-21T22:50:20.522Z",
},
},
Expand Down Expand Up @@ -885,7 +885,7 @@ exports[`UpdateAssistant | v0.2 - v0.3.1 should correctly update the proofs reco
"createdAt": "2022-09-08T19:35:53.872Z",
"id": "STORAGE_VERSION_RECORD_ID",
"metadata": {},
"storageVersion": "0.5",
"storageVersion": "0.6",
"updatedAt": "2023-01-21T22:50:20.522Z",
},
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ exports[`UpdateAssistant | v0.4 - v0.5 should correctly add 'type' tag to w3c re
"createdAt": "2023-03-18T18:35:02.888Z",
"id": "STORAGE_VERSION_RECORD_ID",
"metadata": {},
"storageVersion": "0.5",
"storageVersion": "0.6",
"updatedAt": "2024-02-05T22:50:20.522Z",
},
},
Expand Down Expand Up @@ -313,7 +313,7 @@ exports[`UpdateAssistant | v0.4 - v0.5 should correctly add role to credential e
"createdAt": "2023-03-18T18:35:02.888Z",
"id": "STORAGE_VERSION_RECORD_ID",
"metadata": {},
"storageVersion": "0.5",
"storageVersion": "0.6",
"updatedAt": "2024-02-05T22:50:20.522Z",
},
},
Expand Down Expand Up @@ -829,7 +829,7 @@ exports[`UpdateAssistant | v0.4 - v0.5 should correctly add role to proof exchan
"createdAt": "2023-03-18T18:35:02.888Z",
"id": "STORAGE_VERSION_RECORD_ID",
"metadata": {},
"storageVersion": "0.5",
"storageVersion": "0.6",
"updatedAt": "2024-02-05T22:50:20.522Z",
},
},
Expand Down
6 changes: 6 additions & 0 deletions packages/core/src/storage/migration/updates.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type { VersionString } from '../../utils/version'
import { updateV0_3ToV0_3_1 } from './updates/0.3-0.3.1'
import { updateV0_3_1ToV0_4 } from './updates/0.3.1-0.4'
import { updateV0_4ToV0_5 } from './updates/0.4-0.5'
import { updateV0_5ToV0_6 } from './updates/0.5-0.6'

export const INITIAL_STORAGE_VERSION = '0.1'

Expand Down Expand Up @@ -53,6 +54,11 @@ export const supportedUpdates = [
toVersion: '0.5',
doUpdate: updateV0_4ToV0_5,
},
{
fromVersion: '0.5',
toVersion: '0.6',
doUpdate: updateV0_5ToV0_6,
},
] as const

// Current version is last toVersion from the supported updates
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import type { BaseAgent } from '../../../../agent/BaseAgent'

import { migrateToCredoFolder } from './migrateToCredoFolder'

export async function updateV0_5ToV0_6<Agent extends BaseAgent>(agent: Agent): Promise<void> {
await migrateToCredoFolder(agent)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import type { BaseAgent } from '../../../../agent/BaseAgent'
import type { FileSystem } from '../../../FileSystem'

import { InjectionSymbols } from '../../../../constants'
import { CredoError } from '../../../../error'

/**
* Migrates the sqlite folder location from .afj to .credo in the storage directory in node and react native.
*
*/
export async function migrateToCredoFolder<Agent extends BaseAgent>(agent: Agent) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

due to the sensivity in doing migrations it's always good to test these migrations extensively. For all other migrations and version changes we have tests (per migration method, and for a version migration in general), can you add these?

const walletId = agent.config.walletConfig?.id

if (!walletId) {
throw new CredoError('Wallet id is required to migrate the wallet to .credo')
}

// Adding type assertion to get the storage config
const storageConfig = agent.config.walletConfig?.storage as {
config?: { inMemory?: boolean }
}

// If no storage config is provided, we set default as sqlite
// https://github.com/openwallet-foundation/credo-ts/blob/main/packages/askar/src/utils/askarWalletConfig.ts#L35
// and we only migrate the data folder if the storage config is not set to inMemory
if (!storageConfig || (storageConfig.config && !storageConfig.config?.inMemory)) {
return
}

agent.config.logger.info('Migrating data from .afj to .credo')

const fileSystem = agent.dependencyManager.resolve<FileSystem>(InjectionSymbols.FileSystem)

await fileSystem.migrateWalletToCredoFolder(walletId)
agent.config.logger.info('Migration completed successfully')
}
60 changes: 52 additions & 8 deletions packages/node/src/NodeFileSystem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import https from 'https'
import { tmpdir, homedir } from 'os'
import { dirname } from 'path'

const { access, readFile, writeFile, mkdir, rm, unlink, copyFile } = promises
const { access, readFile, writeFile, mkdir, rm, unlink, copyFile, cp } = promises

export class NodeFileSystem implements FileSystem {
public readonly dataPath
Expand All @@ -19,16 +19,61 @@ export class NodeFileSystem implements FileSystem {
* Create new NodeFileSystem class instance.
*
* @param baseDataPath The base path to use for reading and writing data files used within the framework.
* Files will be created under baseDataPath/.afj directory. If not specified, it will be set to homedir()
* Files will be created under baseDataPath/.credo directory. If not specified, it will be set to homedir()
* @param baseCachePath The base path to use for reading and writing cache files used within the framework.
* Files will be created under baseCachePath/.afj directory. If not specified, it will be set to homedir()
* Files will be created under baseCachePath/.credo directory. If not specified, it will be set to homedir()
* @param baseTempPath The base path to use for reading and writing temporary files within the framework.
* Files will be created under baseTempPath/.afj directory. If not specified, it will be set to tmpdir()
* Files will be created under baseTempPath/.credo directory. If not specified, it will be set to tmpdir()
*/
public constructor(options?: { baseDataPath?: string; baseCachePath?: string; baseTempPath?: string }) {
this.dataPath = options?.baseDataPath ? `${options?.baseDataPath}/.afj` : `${homedir()}/.afj/data`
this.cachePath = options?.baseCachePath ? `${options?.baseCachePath}/.afj` : `${homedir()}/.afj/cache`
this.tempPath = `${options?.baseTempPath ?? tmpdir()}/.afj`
this.dataPath = options?.baseDataPath ? `${options?.baseDataPath}/.credo` : `${homedir()}/.credo/data`
this.cachePath = options?.baseCachePath ? `${options?.baseCachePath}/.credo` : `${homedir()}/.credo/cache`
this.tempPath = `${options?.baseTempPath ?? tmpdir()}/.credo`
}

/**
* Migrate data from .afj to .credo if .afj exists.
* Copy the contents from old directory (.afj) to new directory (.credo).
*/
public async migrateWalletToCredoFolder(walletId: string) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the file system should stay very generic. So instead of adding this method migrateWalletToCredoFolder in the file system (which then needs to be implemented by every file system), I'd move it to the migration method. This can then call the needed methods exists and copyDirectory (i think this method can also copy files now? Should we create one generic copy method that can do both directories (using recursive property) and files?)

try {
// We only migrate the specific wallet folder because other wallets might be using the same .afj folder
// which are used by different agents
const oldWalletPath = this.dataPath.replace('.credo/data', `.afj/data/wallet/${walletId}`)
const cacheAfjPath = this.cachePath.replace('.credo', '.afj')
const tempAfjPath = this.tempPath.replace('.credo', '.afj')

const pathsToMigrate = [
{
from: oldWalletPath,
// We manually construct the path to the wallet folder because we only want to migrate the specific wallet
to: this.dataPath + '/wallet/' + walletId,
},
{
from: cacheAfjPath,
to: this.cachePath,
},
{
from: tempAfjPath,
to: this.tempPath,
},
]

for await (const path of pathsToMigrate) {
// Migrate if the old paths exist
if (await this.exists(path.from)) {
await this.copyDirectory(path.from, path.to)
}
}
Comment on lines +62 to +67
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we delete the old files afterwards? Or how do we clean up?

} catch (error) {
throw new CredoError(`Error during migration from .afj to .credo`, {
cause: error,
})
}
}

public async copyDirectory(sourcePath: string, destinationPath: string) {
await cp(sourcePath, destinationPath, { recursive: true })
}

public async exists(path: string) {
Expand All @@ -49,7 +94,6 @@ export class NodeFileSystem implements FileSystem {
}

public async write(path: string, data: string): Promise<void> {
// Make sure parent directories exist
await mkdir(dirname(path), { recursive: true })

return writeFile(path, data, { encoding: 'utf-8' })
Expand Down
13 changes: 13 additions & 0 deletions packages/node/tests/NodeFileSystem.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { TypedArrayEncoder } from '@credo-ts/core'
import nock, { cleanAll, enableNetConnect } from 'nock'
import { homedir } from 'os'
import path from 'path'

import { NodeFileSystem } from '../src/NodeFileSystem'
Expand Down Expand Up @@ -38,5 +39,17 @@ describe('@credo-ts/file-system-node', () => {
)
})
})

describe('migrateWalletToCredoFolder', () => {
test('should copy the files from .afj to .credo', async () => {
const filePath = `${homedir()}/.afj/data/wallet/test/testwallet.db`

await fileSystem.write(filePath, 'some-random-content')

await fileSystem.migrateWalletToCredoFolder('test')

expect(await fileSystem.exists(`${fileSystem.dataPath}/wallet/test/testwallet.db`)).toBe(true)
})
})
})
})
78 changes: 70 additions & 8 deletions packages/react-native/src/ReactNativeFileSystem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,26 +13,88 @@ export class ReactNativeFileSystem implements FileSystem {
* Create new ReactNativeFileSystem class instance.
*
* @param baseDataPath The base path to use for reading and writing data files used within the framework.
* Files will be created under baseDataPath/.afj directory. If not specified, it will be set to
* Files will be created under baseDataPath/.credo directory. If not specified, it will be set to
* RNFS.DocumentDirectoryPath
* @param baseCachePath The base path to use for reading and writing cache files used within the framework.
* Files will be created under baseCachePath/.afj directory. If not specified, it will be set to
* Files will be created under baseCachePath/.credo directory. If not specified, it will be set to
* RNFS.CachesDirectoryPath
* @param baseTempPath The base path to use for reading and writing temporary files within the framework.
* Files will be created under baseTempPath/.afj directory. If not specified, it will be set to
* Files will be created under baseTempPath/.credo directory. If not specified, it will be set to
* RNFS.TemporaryDirectoryPath
*
* @see https://github.com/itinance/react-native-fs#constants
*/
public constructor(options?: { baseDataPath?: string; baseCachePath?: string; baseTempPath?: string }) {
this.dataPath = `${options?.baseDataPath ?? RNFS.DocumentDirectoryPath}/.afj`
this.dataPath = `${options?.baseDataPath ?? RNFS.DocumentDirectoryPath}/.credo`
// In Android, TemporaryDirectoryPath falls back to CachesDirectoryPath
this.cachePath = options?.baseCachePath
? `${options?.baseCachePath}/.afj`
: `${RNFS.CachesDirectoryPath}/.afj${Platform.OS === 'android' ? '/cache' : ''}`
? `${options?.baseCachePath}/.credo`
: `${RNFS.CachesDirectoryPath}/.credo${Platform.OS === 'android' ? '/cache' : ''}`
this.tempPath = options?.baseTempPath
? `${options?.baseTempPath}/.afj`
: `${RNFS.TemporaryDirectoryPath}/.afj${Platform.OS === 'android' ? '/temp' : ''}`
? `${options?.baseTempPath}/.credo`
: `${RNFS.TemporaryDirectoryPath}/.credo${Platform.OS === 'android' ? '/temp' : ''}`
}

/**
* Migrate data from .afj to .credo if .afj exists.
* Copy the contents from old directory (.afj) to new directory (.credo).
*/
public async migrateWalletToCredoFolder() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here, this can be moved to migration logic so we don't have to duplicate

try {
const oldDataPath = this.dataPath.replace('.credo', `.afj`)
const cacheAfjPath = this.cachePath.replace('.credo', '.afj')
const tempAfjPath = this.tempPath.replace('.credo', '.afj')

const pathsToMigrate = [
{
from: oldDataPath,
to: this.dataPath,
},
{
from: cacheAfjPath,
to: this.cachePath,
},
{
from: tempAfjPath,
to: this.tempPath,
},
]

for await (const path of pathsToMigrate) {
// Migrate if the old paths exist
if (await this.exists(path.from)) {
await this.copyDirectory(path.from, path.to)
}
}
} catch (error) {
throw new CredoError(`Error during migration from .afj to .credo`, {
cause: error,
})
}
}

public async copyDirectory(sourcePath: string, destinationPath: string) {
try {
// Ensure the target directory exists
await RNFS.mkdir(destinationPath)

// Get the contents of the source directory
const contents = await RNFS.readDir(sourcePath)

for (const item of contents) {
const newPath = `${destinationPath}/${item.name}`

if (item.isDirectory()) {
// Recursively copy subdirectories
await this.copyDirectory(item.path, newPath)
} else {
// Copy files to the new location
await RNFS.copyFile(item.path, newPath)
}
}
} catch (error) {
throw new CredoError(`Error copying directory from ${sourcePath} to ${destinationPath}`, { cause: error })
}
}

public async exists(path: string): Promise<boolean> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ describe('TenantAgentContextProvider', () => {
key: 'test-wallet-key',
},
},
storageVersion: '0.5',
storageVersion: '0.6',
})

const tenantAgentContext = jest.fn() as unknown as AgentContext
Expand Down Expand Up @@ -86,7 +86,7 @@ describe('TenantAgentContextProvider', () => {
key: 'test-wallet-key',
},
},
storageVersion: '0.5',
storageVersion: '0.6',
})

const tenantAgentContext = jest.fn() as unknown as AgentContext
Expand Down Expand Up @@ -131,7 +131,7 @@ describe('TenantAgentContextProvider', () => {
key: 'test-wallet-key',
},
},
storageVersion: '0.5',
storageVersion: '0.6',
})

const tenantAgentContext = jest.fn() as unknown as AgentContext
Expand Down
8 changes: 4 additions & 4 deletions packages/tenants/tests/tenants-storage-update.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,9 +74,9 @@ describe('Tenants Storage Update', () => {
await agent.modules.tenants.getTenantAgent({ tenantId: '1d45d3c2-3480-4375-ac6f-47c322f091b0' })
).endSession()

// Expect tenant storage version to be 0.5
// Expect tenant storage version to be 0.6
const updatedTenant = await agent.modules.tenants.getTenantById('1d45d3c2-3480-4375-ac6f-47c322f091b0')
expect(updatedTenant.storageVersion).toBe('0.5')
expect(updatedTenant.storageVersion).toBe('0.6')

await agent.wallet.delete()
await agent.shutdown()
Expand Down Expand Up @@ -204,9 +204,9 @@ describe('Tenants Storage Update', () => {
// Should have closed session after upgrade
expect(tenantSessionCoordinator.getSessionCountForTenant(tenant.id)).toBe(0)

// Expect tenant storage version to be 0.5
// Expect tenant storage version to be 0.6
const updatedTenant = await agent.modules.tenants.getTenantById('1d45d3c2-3480-4375-ac6f-47c322f091b0')
expect(updatedTenant.storageVersion).toBe('0.5')
expect(updatedTenant.storageVersion).toBe('0.6')

// Getting tenant should now work
await expect(
Expand Down
Loading