feat(data-modeling): implement data model storage that uses atlas user data COMPASS-9949 #7459
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,129 @@ | ||
| import { AtlasUserData } from '@mongodb-js/compass-user-data'; | ||
| import type { | ||
| DataModelStorage, | ||
| MongoDBDataModelDescription, | ||
| } from './data-model-storage'; | ||
| import { MongoDBDataModelDescriptionSchema } from './data-model-storage'; | ||
| import dataModelStorageInMemory from './data-model-storage-in-memory'; | ||
| import { | ||
| atlasServiceLocator, | ||
| type AtlasService, | ||
| } from '@mongodb-js/atlas-service/provider'; | ||
| import { createServiceProvider } from '@mongodb-js/compass-app-registry'; | ||
| import { DataModelStorageServiceProvider } from '../provider'; | ||
| import React, { useRef } from 'react'; | ||
| import { mongoLogId, useLogger } from '@mongodb-js/compass-logging/provider'; | ||
|
|
||
| class DataModelStorageAtlas implements DataModelStorage { | ||
| private readonly userData: AtlasUserData< | ||
| typeof MongoDBDataModelDescriptionSchema | ||
| >; | ||
| constructor(orgId: string, projectId: string, atlasService: AtlasService) { | ||
| this.userData = new AtlasUserData( | ||
| MongoDBDataModelDescriptionSchema, | ||
| 'dataModelDescriptions', | ||
|
There was a problem hiding this comment. Not that important as this will be probably refactored too, just looks like we're repeating a const here. |
||
| { | ||
| orgId, | ||
| projectId, | ||
| getResourceUrl(path) { | ||
| // TODO(COMPASS-9960): this is copied from compass-web entrypoint for | ||
| // brevity, but shouldn't be defined outside of AtlasUserData, this | ||
| // logic is literally the same between all user data instances and can | ||
| // be encapsulated inside of the AtlasUserData class implementation | ||
|
Comment on lines
+29
to
+32
There was a problem hiding this comment. This clean-up will touch a lot of unrelated code, so I'm splitting it to a separate PR |
||
| const [type, pathOrgId, pathProjectId, id] = | ||
| path?.split('/').filter(Boolean) || []; | ||
|
|
||
| if ( | ||
| !type || | ||
| !pathOrgId || | ||
| !pathProjectId || | ||
| type !== 'dataModelDescriptions' | ||
|
There was a problem hiding this comment. Also not that important just highlighting that this last check sounds like it could be misconfiguration rather than "outside of atlas cloud context" There was a problem hiding this comment. I'm not putting much effort into this because COMPASS-9960 will remove this whole block of code. Strictly speaking none of these can be missing because all these options are required in the user data constructor |
||
| ) { | ||
| throw new Error( | ||
| 'DataModelStorageAtlas is used outside of Atlas Cloud context' | ||
| ); | ||
| } | ||
|
|
||
| return atlasService.userDataEndpoint( | ||
| pathOrgId, | ||
| pathProjectId, | ||
| type, | ||
| id | ||
| ); | ||
| }, | ||
| authenticatedFetch: atlasService.authenticatedFetch.bind(atlasService), | ||
| } | ||
| ); | ||
| } | ||
| save(description: MongoDBDataModelDescription) { | ||
| return this.userData.write(description.id, description); | ||
| } | ||
| delete(id: MongoDBDataModelDescription['id']) { | ||
| return this.userData.delete(id); | ||
| } | ||
| async loadAll(): Promise<MongoDBDataModelDescription[]> { | ||
| try { | ||
| const res = await this.userData.readAll(); | ||
| return res.data; | ||
| } catch { | ||
| return []; | ||
| } | ||
| } | ||
| async load(id: string): Promise<MongoDBDataModelDescription | null> { | ||
| return ( | ||
| (await this.loadAll()).find((item) => { | ||
| return item.id === id; | ||
| }) ?? null | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| export const DataModelStorageServiceProviderWeb = createServiceProvider( | ||
| function DataModelStorageServiceProviderWeb({ | ||
| children, | ||
| orgId, | ||
| projectId, | ||
| }: { | ||
| /** | ||
| * Atlas organization id. Optional. If provided, data model storage will | ||
| * save the user data in Atlas Cloud, otherwise will fall back to in-memory | ||
| * storage | ||
| */ | ||
| orgId?: string; | ||
| /** | ||
| * Atlas project id. Optional. If provided, data model storage will | ||
| * save the user data in Atlas Cloud, otherwise will fall back to in-memory | ||
| * storage | ||
| */ | ||
| projectId?: string; | ||
| children?: React.ReactNode; | ||
| }) { | ||
| const storageRef = useRef<DataModelStorage>(); | ||
| const atlasService = atlasServiceLocator(); | ||
| const logger = useLogger('DATA-MODEL-STORAGE'); | ||
|
|
||
| if (!storageRef.current) { | ||
| if (orgId && projectId) { | ||
| storageRef.current = new DataModelStorageAtlas( | ||
| orgId, | ||
| projectId, | ||
| atlasService | ||
| ); | ||
| } else { | ||
| logger.log.warn( | ||
| mongoLogId(1_001_000_379), | ||
| 'DataModelStorageServiceProviderWeb', | ||
| 'Falling back to in memory storage because orgId or projectId is missing' | ||
| ); | ||
| // Fallback to in-memory if we're outside of Atlas Cloud | ||
| storageRef.current = dataModelStorageInMemory; | ||
|
There was a problem hiding this comment. If this fallback is for the sandbox, can we check if it's sandbox and otherwise fail visibly? There was a problem hiding this comment. This is not a fallback for the sandbox, I don't think it's a good idea to think about it in that way and would suggest to try not to do it 🙂 Compass-web needs to be able to work outside Atlas runtime seamlessly as much as possible. Arbitrary environment checks are going against that, introducing more complexity than needed, causing weird behavioral differences for different envs (similar to the case of preferences not working as expected in sandbox because it was gated by the environment check for the sandbox), and spreading configuration across the whole codebase instead of keeping it all in one place, so instead I strongly recomment just solving the problem in a more generic way: DM storage in web can fallback to in memory if that's how it is configured, it's the work of consumer to pass the options correctly. I can rename this file to data-model-storage-web to illustrate this better I think There was a problem hiding this comment. Ah I see, so the idea would be this is for other integrations? My concern is that we made a decision to not design around in-memory usage, at present the UI is very strongly suggesting that the diagrams are persisted. There was a problem hiding this comment. Renaming it is a good idea, thanks! There was a problem hiding this comment. I just hope it's somehow clear that other consumers should implement their own persistent storage, otherwise the feature doesn't work as expected There was a problem hiding this comment. Yeah, so the main thing here is that compass-web package should work if it's not used in Atlas without the need to "pretend" to be a sandbox or an e2e test environment through some global configuration like an env var. Maybe there's a better way for us to embed this assumption in the code, FWIW component props are the public interface here controlling this behavior pretty explicitly, but doesn't mean there isn't a better way 🤔 |
||
| } | ||
| } | ||
|
|
||
| return ( | ||
| <DataModelStorageServiceProvider storage={storageRef.current}> | ||
| {children} | ||
| </DataModelStorageServiceProvider> | ||
| ); | ||
| } | ||
| ); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| export * from './dist/services/data-model-storage-web.d'; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,2 @@ | ||
| 'use strict'; | ||
| module.exports = require('./dist/services/data-model-storage-web'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😭