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

feat: load default config when creating a project #471

Merged
merged 20 commits into from
Feb 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
15733dc
fix malformed JSON in default config (...), load default config when
Feb 7, 2024
9edd313
use `new URL(configPath, import.meta.url).pathname` to resolve config
Feb 7, 2024
615cfe6
rename .zip to .mapeoconfig, add tests for default config
Feb 7, 2024
bed0244
fix wrong path to default config
Feb 12, 2024
d5e62cc
Merge branch 'main' of github.com:digidem/mapeo-core-next into feat/l…
Feb 12, 2024
3fe385c
Merge branch 'main' of github.com:digidem/mapeo-core-next into feat/l…
Feb 14, 2024
076e5ba
fix test with wrong num of icons, add assert for number of variants
Feb 14, 2024
d5d0b77
add assertions to check correctly loading default config
Feb 15, 2024
dc078d9
`dataType.delete` now needs a `docId`, not a `versionId`
Feb 15, 2024
37806e8
add e2e tests that checks importing config:
Feb 15, 2024
8dc066a
fix order of execution in tests
Feb 15, 2024
6e1923e
add completeConfig zip to fixtures
Feb 15, 2024
dcb9410
add `defaultConfigPath` to managerBasic opts
Feb 19, 2024
e5ee544
* Use absolute path for configs
Feb 19, 2024
c16b9fd
Merge branch 'main' of github.com:digidem/mapeo-core-next into feat/l…
Feb 19, 2024
53f981f
await subtests in `test-e2e/manager-basic.js`
Feb 19, 2024
4755ae0
delete commented code
Feb 19, 2024
869199b
move manager-basic config subtests to their own test
Feb 20, 2024
8b4268c
change tests string and move loading of config into subtest
Feb 20, 2024
315acf9
Merge branch 'main' of github.com:digidem/mapeo-core-next into feat/l…
Feb 20, 2024
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
Binary file not shown.
14 changes: 14 additions & 0 deletions src/mapeo-manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ export class MapeoManager extends TypedEmitter {
#localDiscovery
#loggerBase
#l
#defaultConfigPath
/** @readonly */
#deviceType

Expand All @@ -103,6 +104,7 @@ export class MapeoManager extends TypedEmitter {
* @param {string | import('./types.js').CoreStorage} opts.coreStorage Folder for hypercore storage or a function that returns a RandomAccessStorage instance
* @param {import('fastify').FastifyInstance} opts.fastify Fastify server instance
* @param {import('./generated/rpc.js').DeviceInfo['deviceType']} [opts.deviceType] Device type, shared with local peers and project members
* @param {String} [opts.defaultConfigPath]
*/
constructor({
rootKey,
Expand All @@ -112,11 +114,13 @@ export class MapeoManager extends TypedEmitter {
coreStorage,
fastify,
deviceType,
defaultConfigPath,
}) {
super()
this.#keyManager = new KeyManager(rootKey)
this.#deviceId = getDeviceId(this.#keyManager)
this.#deviceType = deviceType
this.#defaultConfigPath = defaultConfigPath
const logger = (this.#loggerBase = new Logger({ deviceId: this.#deviceId }))
this.#l = Logger.create('manager', logger)
this.#dbFolder = dbFolder
Expand Down Expand Up @@ -389,6 +393,16 @@ export class MapeoManager extends TypedEmitter {
// TODO: Close the project instance instead of keeping it around
this.#activeProjects.set(projectPublicId, project)

// load default config
// TODO: see how to expose warnings to frontend
/* eslint-disable no-unused-vars */
let warnings
if (this.#defaultConfigPath) {
warnings = await project.importConfig({
configPath: this.#defaultConfigPath,
})
}

this.#l.log(
'created project %h, public id: %S',
projectKeypair.publicKey,
Expand Down
4 changes: 2 additions & 2 deletions src/mapeo-project.js
Original file line number Diff line number Diff line change
Expand Up @@ -721,8 +721,8 @@ function extractEditableProjectSettings(projectDoc) {
/** @param {DataType<any,any,any,any,any>} dataType */
async function deleteAll(dataType) {
const deletions = []
for (const { versionId } of await dataType.getMany()) {
deletions.push(dataType.delete(versionId))
for (const { docId } of await dataType.getMany()) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

fyi, this change is not actually in the scope of this PR, but is related to datatype.delete now taking a docId instead of versionId (I went ahead since it was a small change...)

Copy link
Member

Choose a reason for hiding this comment

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

This suggests we were lacking test coverage here. Can you create an issue to create an e2e test for changing config, to ensure that old config is correctly deleted.

deletions.push(dataType.delete(docId))
}
return Promise.all(deletions)
}
Expand Down
122 changes: 101 additions & 21 deletions test-e2e/manager-basic.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import { KeyManager } from '@mapeo/crypto'
import RAM from 'random-access-memory'
import { MapeoManager } from '../src/mapeo-manager.js'
import Fastify from 'fastify'
import { getExpectedConfig } from './utils.js'
import { kDataTypes } from '../src/mapeo-project.js'

const projectMigrationsFolder = new URL('../drizzle/project', import.meta.url)
.pathname
Expand All @@ -26,7 +28,7 @@ test('Managing created projects', async (t) => {
name: 'project 2',
})

t.test('initial information from listed projects', async (st) => {
await t.test('initial information from listed projects', async (st) => {
const listedProjects = await manager.listProjects()

st.is(listedProjects.length, 2)
Expand Down Expand Up @@ -56,22 +58,23 @@ test('Managing created projects', async (t) => {
t.ok(project1)
t.ok(project2)

t.test('initial settings from project instances', async (st) => {
await t.test('initial settings from project instances', async (st) => {
const settings1 = await project1.$getProjectSettings()
const settings2 = await project2.$getProjectSettings()

st.alike(settings1, {
name: undefined,
defaultPresets: undefined,
})

st.alike(settings2, {
name: 'project 2',
defaultPresets: undefined,
})
st.alike(
settings1,
{ name: undefined, defaultPresets: undefined },
'undefined name and default presets for project1'
)
st.alike(
settings2,
{ name: 'project 2', defaultPresets: undefined },
'matched name for project2 with undefined default presets'
)
})

t.test('after updating project settings', async (st) => {
await t.test('after updating project settings', async (st) => {
await project1.$setProjectSettings({
name: 'project 1',
})
Expand All @@ -82,15 +85,9 @@ test('Managing created projects', async (t) => {
const settings1 = await project1.$getProjectSettings()
const settings2 = await project2.$getProjectSettings()

st.alike(settings1, {
name: 'project 1',
defaultPresets: undefined,
})
st.is(settings1.name, 'project 1')

st.alike(settings2, {
name: 'project 2 updated',
defaultPresets: undefined,
})
st.is(settings2.name, 'project 2 updated')

const listedProjects = await manager.listProjects()

Expand All @@ -116,6 +113,89 @@ test('Managing created projects', async (t) => {
})
})

test('Consistent loading of config', async (t) => {
const manager = new MapeoManager({
rootKey: KeyManager.generateRootKey(),
projectMigrationsFolder,
clientMigrationsFolder,
dbFolder: ':memory:',
coreStorage: () => new RAM(),
fastify: Fastify(),
defaultConfigPath: 'config/defaultConfig.mapeoconfig',
})

const expectedDefault = await getExpectedConfig(
'config/defaultConfig.mapeoconfig'
)
const expectedMinimal = await getExpectedConfig(
'tests/fixtures/config/completeConfig.zip'
)
const projectId = await manager.createProject()
const project = await manager.getProject(projectId)
const projectSettings = await project.$getProjectSettings()

const projectPresets = await project.preset.getMany()
await t.test(
'load default config and check if correctly loaded',
async (st) => {
st.is(
//@ts-ignore
projectSettings.defaultPresets.point.length,
expectedDefault.presets.length,
'the default presets loaded is equal to the number of presets in the default config'
)

st.alike(
projectPresets.map((preset) => preset.name),
expectedDefault.presets.map((preset) => preset.value.name),
'project is loading the default presets correctly'
)

const projectFields = await project.field.getMany()
st.alike(
projectFields.map((field) => field.tagKey),
expectedDefault.fields.map((field) => field.value.tagKey),
'project is loading the default fields correctly'
)

const projectIcons = await project[kDataTypes].icon.getMany()
st.alike(
projectIcons.map((icon) => icon.name),
expectedDefault.icons.map((icon) => icon.name),
'project is loading the default icons correctly'
)
}
)

await t.test(
'load different config and check if correctly loaded',
async (st) => {
Comment on lines +170 to +172
Copy link
Member

Choose a reason for hiding this comment

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

I think things can get confusing when you have subtests that mutate the project, maybe move to a separate test and a separate project instances? I don't think this needs to be part of the same test.

const configPath = 'tests/fixtures/config/completeConfig.zip'
await project.importConfig({ configPath })
const projectPresets = await project.preset.getMany()
st.alike(
projectPresets.map((preset) => preset.name),
expectedMinimal.presets.map((preset) => preset.value.name),
'project presets explicitly loaded match expected config'
)

const projectFields = await project.field.getMany()
st.alike(
projectFields.map((field) => field.tagKey),
expectedMinimal.fields.map((field) => field.value.tagKey),
'project fields explicitly loaded match expected config'
)

// TODO: since we don't delete icons, this wouldn't match
// const projectIcons = await project1[kDataTypes].icon.getMany()
// st.alike(
// projectIcons.map((icon) => icon.name),
// loadedIcons.map((icon) => icon.name)
// )
}
)
})

test('Managing added projects', async (t) => {
const manager = new MapeoManager({
rootKey: KeyManager.generateRootKey(),
Expand Down Expand Up @@ -144,7 +224,7 @@ test('Managing added projects', async (t) => {
{ waitForSync: false }
)

t.test('initial information from listed projects', async (st) => {
await t.test('initial information from listed projects', async (st) => {
const listedProjects = await manager.listProjects()

st.is(listedProjects.length, 2)
Expand Down
24 changes: 24 additions & 0 deletions test-e2e/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import { temporaryDirectory } from 'tempy'
import fsPromises from 'node:fs/promises'
import { MEMBER_ROLE_ID } from '../src/roles.js'
import { kSyncState } from '../src/sync/sync-api.js'
import { readConfig } from '../src/config-import.js'

const FAST_TESTS = !!process.env.FAST_TESTS
const projectMigrationsFolder = new URL('../drizzle/project', import.meta.url)
Expand Down Expand Up @@ -345,6 +346,29 @@ export function sortBy(arr, key) {
})
}

/** @param {String} path */
export async function getExpectedConfig(path) {
const config = await readConfig(path)
let presets = []
let fields = []
let icons = []
for (let preset of config.presets()) {
presets.push(preset)
}
for (let field of config.fields()) {
fields.push(field)
}
for await (let icon of config.icons()) {
icons.push(icon)
}

return {
presets,
fields,
icons,
}
}

/** @param {import('@mapeo/schema').MapeoDoc[]} docs */
export function sortById(docs) {
return sortBy(docs, 'docId')
Expand Down
34 changes: 33 additions & 1 deletion tests/config-import.js
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ test('config import - presets', async (t) => {
let config = await readConfig('./tests/fixtures/config/invalidPreset.zip')

/* eslint-disable-next-line */
for (const field of config.presets()) {
for (const preset of config.presets()) {
}
t.is(config.warnings.length, 2, 'we got two errors when reading presets')
t.not(
Expand All @@ -208,3 +208,35 @@ test('config import - presets', async (t) => {
}
t.is(config.warnings.length, 0, `no warnings on the file`)
})

test('config import - load default config', async (t) => {
let config = await readConfig('./config/defaultConfig.mapeoconfig')
tomasciccola marked this conversation as resolved.
Show resolved Hide resolved
t.ok(config, 'valid config file')

let nFields = 0
/* eslint-disable-next-line */
for (const field of config.fields()) {
nFields++
}
t.is(nFields, 2, 'correct number of fields in default config')
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't seem right

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, I realize that I built the default config using the incorrect repo (this one). I think that if we're removing the default config on a follow-up PR (and having it built as a devdep) then we maybe leave this as is and update the tests on that PR when it actually points to the correct config?
Or do you have another reason to think that that doesn't seem right??

Copy link
Member

Choose a reason for hiding this comment

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

No that would be it, ok to resolve in a follow up

let nIcons = 0
let nVariants = 0
/* eslint-disable-next-line */
for await (const icon of config.icons()) {
nIcons++
/* eslint-disable-next-line */
for (let variant of icon.variants) {
nVariants++
}
}
t.is(nIcons, 41, 'correct number of icons in default config')
t.is(nVariants, 369, 'correct number of icon variants in default config')

let nPresets = 0
/* eslint-disable-next-line */
for (const preset of config.presets()) {
nPresets++
}
t.is(nPresets, 43, 'correct number of presets in default config')
t.is(config.warnings.length, 0, 'no warnings on config file')
})
Binary file added tests/fixtures/config/completeConfig.zip
Binary file not shown.
Loading