Skip to content

Commit

Permalink
More structured indexing for user data objects.
Browse files Browse the repository at this point in the history
I think I clung to the idea of having two ways to do this too long. I did a bunch of testing with the old way (exposing integer references based on numeric database primary keys). Having a config option that if changed would break everything exisiting is also a symptom of maybe me clinging too hard for too long.

The result of dropping this option is a much cleaner API schema, simpler API objects, and better typing throughout. We can also be more certain of how things entering the Vault are stored - I think being more strucutured about this is good.

Ultimately, the future facing stuff (e.g. OAuth 2.0) is going to require UUIDs so that I can store things like refresh tokens in the store before the object has been fully created.
  • Loading branch information
jmchilton committed May 31, 2024
1 parent e4d45a7 commit d42236e
Show file tree
Hide file tree
Showing 27 changed files with 64 additions and 143 deletions.
16 changes: 6 additions & 10 deletions client/src/api/schema/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12651,8 +12651,6 @@ export interface components {
device?: string | null;
/** Hidden */
hidden: boolean;
/** Id */
id: number | string;
/** Name */
name?: string | null;
/** Object Store Id */
Expand Down Expand Up @@ -12730,8 +12728,6 @@ export interface components {
description: string | null;
/** Hidden */
hidden: boolean;
/** Id */
id: string | number;
/** Name */
name: string;
/** Purged */
Expand Down Expand Up @@ -14972,7 +14968,7 @@ export interface operations {
header?: {
"run-as"?: string | null;
};
/** @description The index for a persisted UserFileSourceStore object. */
/** @description The UUID index for a persisted UserFileSourceStore object. */
path: {
user_file_source_id: string;
};
Expand All @@ -14999,7 +14995,7 @@ export interface operations {
header?: {
"run-as"?: string | null;
};
/** @description The index for a persisted UserFileSourceStore object. */
/** @description The UUID index for a persisted UserFileSourceStore object. */
path: {
user_file_source_id: string;
};
Expand Down Expand Up @@ -15034,7 +15030,7 @@ export interface operations {
header?: {
"run-as"?: string | null;
};
/** @description The index for a persisted UserFileSourceStore object. */
/** @description The UUID index for a persisted UserFileSourceStore object. */
path: {
user_file_source_id: string;
};
Expand Down Expand Up @@ -20896,7 +20892,7 @@ export interface operations {
header?: {
"run-as"?: string | null;
};
/** @description The identifier used to index a persisted UserObjectStore object. */
/** @description The UUID used to indentify a persisted UserObjectStore object. */
path: {
user_object_store_id: string;
};
Expand All @@ -20923,7 +20919,7 @@ export interface operations {
header?: {
"run-as"?: string | null;
};
/** @description The identifier used to index a persisted UserObjectStore object. */
/** @description The UUID used to indentify a persisted UserObjectStore object. */
path: {
user_object_store_id: string;
};
Expand Down Expand Up @@ -20958,7 +20954,7 @@ export interface operations {
header?: {
"run-as"?: string | null;
};
/** @description The identifier used to index a persisted UserObjectStore object. */
/** @description The UUID used to indentify a persisted UserObjectStore object. */
path: {
user_object_store_id: string;
};
Expand Down
1 change: 0 additions & 1 deletion client/src/components/ConfigTemplates/test_fixtures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,6 @@ export const OBJECT_STORE_INSTANCE: UserConcreteObjectStore = {
secrets: ["oldsecret", "droppedsecret"],
quota: { enabled: false },
private: false,
id: 4,
uuid: "112f889f-72d7-4619-a8e8-510a8c685aa7",
active: true,
hidden: false,
Expand Down
4 changes: 2 additions & 2 deletions client/src/components/FileSources/Instances/EditInstance.vue
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import EditSecrets from "./EditSecrets.vue";
import InstanceForm from "@/components/ConfigTemplates/InstanceForm.vue";
interface Props {
instanceId: number | string;
instanceId: string;
}
const props = defineProps<Props>();
Expand All @@ -36,7 +36,7 @@ const loadingMessage = "Loading file source template and instance information";
async function onSubmit(formData: any) {
if (template.value) {
const payload = editFormDataToPayload(template.value, formData);
const args = { user_file_source_id: String(instance?.value?.id) };
const args = { user_file_source_id: String(instance?.value?.uuid) };
const { data: fileSource } = await update({ ...args, ...payload });
await onUpdate(fileSource);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ async function onUpdate(secretName: string, secretValue: string) {
secret_name: secretName,
secret_value: secretValue,
};
const args = { user_file_source_id: String(props.fileSource.id) };
const args = { user_file_source_id: String(props.fileSource.uuid) };
await update({ ...args, ...payload });
}
</script>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ interface Props {
}
const props = defineProps<Props>();
const routeEdit = computed(() => `/file_source_instances/${props.fileSource.id}/edit`);
const routeUpgrade = computed(() => `/file_source_instances/${props.fileSource.id}/upgrade`);
const routeEdit = computed(() => `/file_source_instances/${props.fileSource.uuid}/edit`);
const routeUpgrade = computed(() => `/file_source_instances/${props.fileSource.uuid}/upgrade`);
const isUpgradable = computed(() =>
fileSourceTemplatesStore.canUpgrade(props.fileSource.template_id, props.fileSource.template_version)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const loadingMessage = "Loading file source template and instance information";
async function onSubmit(formData: any) {
const payload = upgradeFormDataToPayload(props.latestTemplate, formData);
const args = { user_file_source_id: String(props.instance.id) };
const args = { user_file_source_id: String(props.instance.uuid) };
try {
const { data: fileSource } = await update({ ...args, ...payload });
await onUpgrade(fileSource);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import UpgradeForm from "./UpgradeForm.vue";
import LoadingSpan from "@/components/LoadingSpan.vue";
interface Props {
instanceId: number | string;
instanceId: string;
}
const props = defineProps<Props>();
Expand Down
2 changes: 1 addition & 1 deletion client/src/components/FileSources/Instances/instance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import type { FileSourceTemplateSummary, UserFileSourceModel } from "@/api/fileS
import { useFileSourceInstancesStore } from "@/stores/fileSourceInstancesStore";
import { useFileSourceTemplatesStore } from "@/stores/fileSourceTemplatesStore";

export function useInstanceAndTemplate(instanceIdRef: Ref<string | number>) {
export function useInstanceAndTemplate(instanceIdRef: Ref<string>) {
const fileSourceTemplatesStore = useFileSourceTemplatesStore();
const fileSourceInstancesStore = useFileSourceInstancesStore();
fileSourceInstancesStore.fetchInstances();
Expand Down
2 changes: 1 addition & 1 deletion client/src/components/FileSources/Instances/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ export const update = fetcher.path("/api/file_source_instances/{user_file_source

export async function hide(instance: UserFileSourceModel) {
const payload = { hidden: true };
const args = { user_file_source_id: String(instance?.id) };
const args = { user_file_source_id: String(instance?.uuid) };
const { data: fileSource } = await update({ ...args, ...payload });
return fileSource;
}
4 changes: 2 additions & 2 deletions client/src/components/ObjectStore/Instances/EditInstance.vue
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import EditSecrets from "./EditSecrets.vue";
import InstanceForm from "@/components/ConfigTemplates/InstanceForm.vue";
interface Props {
instanceId: number | string;
instanceId: string;
}
const props = defineProps<Props>();
Expand All @@ -36,7 +36,7 @@ const loadingMessage = "Loading storage location template and instance informati
async function onSubmit(formData: any) {
if (template.value) {
const payload = editFormDataToPayload(template.value, formData);
const args = { user_object_store_id: String(instance?.value?.id) };
const args = { user_object_store_id: String(instance?.value?.uuid) };
const { data: objectStore } = await update({ ...args, ...payload });
await onUpdate(objectStore);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ async function onUpdate(secretName: string, secretValue: string) {
secret_name: secretName,
secret_value: secretValue,
};
const args = { user_object_store_id: String(props.objectStore.id) };
const args = { user_object_store_id: String(props.objectStore.uuid) };
await update({ ...args, ...payload });
}
</script>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ interface Props {
}
const props = defineProps<Props>();
const routeEdit = computed(() => `/object_store_instances/${props.objectStore.id}/edit`);
const routeUpgrade = computed(() => `/object_store_instances/${props.objectStore.id}/upgrade`);
const routeEdit = computed(() => `/object_store_instances/${props.objectStore.uuid}/edit`);
const routeUpgrade = computed(() => `/object_store_instances/${props.objectStore.uuid}/upgrade`);
const isUpgradable = computed(() =>
objectStoreTemplatesStore.canUpgrade(props.objectStore.template_id, props.objectStore.template_version)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const loadingMessage = "Loading storage location template and instance informati
async function onSubmit(formData: any) {
const payload = upgradeFormDataToPayload(props.latestTemplate, formData);
const args = { user_object_store_id: String(props.instance.id) };
const args = { user_object_store_id: String(props.instance.uuid) };
try {
const { data: objectStore } = await update({ ...args, ...payload });
await onUpgrade(objectStore);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import UpgradeForm from "./UpgradeForm.vue";
import LoadingSpan from "@/components/LoadingSpan.vue";
interface Props {
instanceId: number | string;
instanceId: string;
}
const props = defineProps<Props>();
Expand Down
2 changes: 1 addition & 1 deletion client/src/components/ObjectStore/Instances/instance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { useObjectStoreTemplatesStore } from "@/stores/objectStoreTemplatesStore

import type { UserConcreteObjectStore } from "./types";

export function useInstanceAndTemplate(instanceIdRef: Ref<string | number>) {
export function useInstanceAndTemplate(instanceIdRef: Ref<string>) {
const objectStoreTemplatesStore = useObjectStoreTemplatesStore();
const objectStoreInstancesStore = useObjectStoreInstancesStore();
objectStoreInstancesStore.fetchInstances();
Expand Down
2 changes: 1 addition & 1 deletion client/src/components/ObjectStore/Instances/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export const update = fetcher.path("/api/object_store_instances/{user_object_sto

export async function hide(instance: UserConcreteObjectStore) {
const payload = { hidden: true };
const args = { user_object_store_id: String(instance?.id) };
const args = { user_object_store_id: String(instance?.uuid) };
const { data: objectStore } = await update({ ...args, ...payload });
return objectStore;
}
2 changes: 1 addition & 1 deletion client/src/stores/fileSourceInstancesStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export const useFileSourceInstancesStore = defineStore("fileSourceInstances", {
return !state.fetched;
},
getInstance: (state) => {
return (id: number | string) => state.instances.find((i) => i.id.toString() == id.toString());
return (uuid: string) => state.instances.find((i) => i.uuid == uuid);
},
},
actions: {
Expand Down
2 changes: 1 addition & 1 deletion client/src/stores/objectStoreInstancesStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export const useObjectStoreInstancesStore = defineStore("objectStoreInstances",
return !state.fetched;
},
getInstance: (state) => {
return (id: number | string) => state.instances.find((i) => i.id.toString() == id.toString());
return (uuid: string) => state.instances.find((i) => i.uuid == uuid);
},
},
actions: {
Expand Down
1 change: 0 additions & 1 deletion lib/galaxy/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,6 @@ def _configure_object_store(self, **kwds):
gid=self.config.gid,
object_store_cache_size=self.config.object_store_cache_size,
object_store_cache_path=self.config.object_store_cache_path,
user_config_templates_index_by=self.config.user_config_templates_index_by,
user_config_templates_use_saved_configuration=self.config.user_config_templates_use_saved_configuration,
)
self._register_singleton(UserObjectStoresAppConfig, app_config)
Expand Down
13 changes: 0 additions & 13 deletions lib/galaxy/config/schemas/config_schema.yml
Original file line number Diff line number Diff line change
Expand Up @@ -581,19 +581,6 @@ mapping:
desc: |
Configured user file source templates embedded into Galaxy's config.
user_config_templates_index_by:
type: str
default: 'uuid'
required: false
enum: ['uuid', 'id']
desc: |
Configure URIs for user object stores to use either the object ID ('id')
or UUIDs ('uuid'). Either is fine really, Galaxy doesn't typically expose
database objects by 'id' but there isn't any obvious disadvantage to doing
it in this case and it keeps user exposed URIs much smaller. The default of
UUID feels a little more like a typical way to do this within Galaxy though.
Do not change this value once user object stores have been created.
user_config_templates_use_saved_configuration:
type: str
default: 'fallback'
Expand Down
Loading

0 comments on commit d42236e

Please sign in to comment.