Skip to content

Commit

Permalink
Also fail internal list handlers on invalid queries + various Admin U…
Browse files Browse the repository at this point in the history
…I state management issues.

The former surfaced a few issues in the admin UI, around FilterBar and
search query handling, i.e.
 * the filter bar value getting out of sync
 * filter not being reset when switching tables

Also fix stale config issue and hide "sqlite_" internal tables.
  • Loading branch information
ignatz committed Jan 31, 2025
1 parent bec96be commit 5cec548
Show file tree
Hide file tree
Showing 10 changed files with 49 additions and 46 deletions.
10 changes: 1 addition & 9 deletions client/testfixture/config.textproto
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,6 @@ record_apis: [
schemas: [
{
name: "simple_schema"
schema:
"{"
" \"type\": \"object\","
" \"properties\": {"
" \"name\": { \"type\": \"string\" },"
" \"obj\": { \"type\": \"object\" }"
" },"
" \"required\": [\"name\"]"
"}"
schema: "{ \"type\": \"object\", \"properties\": { \"name\": { \"type\": \"string\" }, \"obj\": { \"type\": \"object\" } }, \"required\": [\"name\"]}"
}
]
17 changes: 7 additions & 10 deletions trailbase-core/js/admin/src/components/FilterBar.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import { createSignal } from "solid-js";

import { Button } from "@/components/ui/button";
import { TextField, TextFieldInput } from "@/components/ui/text-field";

Expand All @@ -9,12 +7,13 @@ export function FilterBar(props: {
initial?: string;
onSubmit: (filter: string) => void;
}) {
const [input, setInput] = createSignal(props.initial ?? "");

let ref: HTMLInputElement | undefined;
const onSubmit = () => {
const value = input();
const value = ref?.value;
console.debug("set filter: ", value);
props.onSubmit(value);
if (value !== undefined) {
props.onSubmit(value);
}
};

return (
Expand All @@ -26,12 +25,10 @@ export function FilterBar(props: {
>
<TextField class="w-full">
<TextFieldInput
ref={ref}
value={props.initial}
type="text"
placeholder={props.placeholder ?? "filter"}
onKeyUp={(e: KeyboardEvent) => {
const value = (e.currentTarget as HTMLInputElement).value;
setInput(value);
}}
/>
</TextField>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ import {
} from "@/components/FormFields";
import type { FormType, AnyFieldApi } from "@/components/FormFields";
import { SheetContainer } from "@/components/SafeSheet";
import { invalidateConfig } from "@/lib/config";

export function CreateAlterTableForm(props: {
close: () => void;
Expand Down Expand Up @@ -108,6 +109,7 @@ export function CreateAlterTableForm(props: {
}

if (!dryRun) {
invalidateConfig();
props.schemaRefetch().then(() => {
props.setSelected(value.name);
});
Expand Down
31 changes: 22 additions & 9 deletions trailbase-core/js/admin/src/components/tables/TablesPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ import {
TooltipTrigger,
} from "@/components/ui/tooltip";

import { createConfigQuery } from "@/lib/config";
import { createConfigQuery, invalidateConfig } from "@/lib/config";
import { adminFetch } from "@/lib/fetch";
import { urlSafeBase64ToUuid, showSaveFileDialog } from "@/lib/utils";
import { RecordApiConfig } from "@proto/config";
Expand Down Expand Up @@ -339,6 +339,8 @@ function TableHeaderRightHandButtons(props: {
await dropTable({
name: table().name,
});

invalidateConfig();
props.schemaRefetch();
}}
msg="Deleting a table will irreversibly delete all the data contained. Are you sure you'd like to continue?"
Expand Down Expand Up @@ -518,13 +520,13 @@ type TableStore = {
schemas: ListSchemasResponse;

// Filter & pagination
filter: string | undefined;
filter: string | null;
pagination: PaginationState;
};

type FetchArgs = {
tableName: string;
filter: string | undefined;
filter: string | null;
pageSize: number;
pageIndex: number;
cursors: string[];
Expand Down Expand Up @@ -700,7 +702,7 @@ function RowDataTable(props: {
</SheetContent>

<FilterBar
initial={props.state.store.filter}
initial={props.state.store.filter ?? undefined}
onSubmit={(value: string) => {
if (value === props.state.store.filter) {
refetch();
Expand Down Expand Up @@ -831,11 +833,11 @@ function TablePane(props: {
pageSize?: string;
}>();

function newStore(): TableStore {
function newStore({ filter }: { filter: string | null }): TableStore {
return {
selected: props.selectedTable,
schemas: props.schemas,
filter: searchParams.filter ?? "",
filter,
pagination: defaultPaginationState({
// NOTE: We index has to start at 0 since we're building the list of
// stable cursors as we incrementally page.
Expand All @@ -847,14 +849,25 @@ function TablePane(props: {

// Cursors are deliberately kept out of the store to avoid tracking.
let cursors: string[] = [];
const [store, setStore] = createStore<TableStore>(newStore());
const [store, setStore] = createStore<TableStore>(
newStore({ filter: searchParams.filter ?? null }),
);
createEffect(() => {
// When switching tables/views, recreate the state. This includes the main
// store but also the current search params and the untracked cursors.
if (store.selected.name !== props.selectedTable.name) {
// Recreate the state/store when we switch tables.
cursors = [];
setStore(newStore());

// The new table probably has different schema, thus filters must not
// carry over.
const newFilter = { filter: null };
setSearchParams(newFilter);

setStore(newStore(newFilter));
return;
}

// When the filter changes, we also update the search params to be in sync.
setSearchParams({
filter: store.filter,
});
Expand Down
10 changes: 6 additions & 4 deletions trailbase-core/js/admin/src/lib/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ function createClient(): QueryClient {
}
const queryClient = createClient();

export async function setConfig(config: Config) {
export async function setConfig(config: Config): Promise<void> {
const data = queryClient.getQueryData<GetConfigResponse>(defaultKey);
const hash = data?.hash;
if (!hash) {
Expand All @@ -24,11 +24,13 @@ export async function setConfig(config: Config) {
hash,
};
console.debug("Updating config:", request);
const response = await updateConfig(request);
await updateConfig(request);

queryClient.invalidateQueries();
invalidateConfig();
}

return response;
export function invalidateConfig() {
queryClient.invalidateQueries();
}

export function createConfigQuery() {
Expand Down
2 changes: 1 addition & 1 deletion trailbase-core/js/admin/src/lib/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -256,5 +256,5 @@ export function tableType(table: Table | View): TableType {
}

export function hiddenTable(table: Table | View): boolean {
return table.name.startsWith("_");
return table.name.startsWith("_") || table.name.startsWith("sqlite_");
}
3 changes: 2 additions & 1 deletion trailbase-core/src/admin/list_logs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,8 @@ pub async fn list_logs_handler(
limit,
order,
..
} = parse_query(raw_url_query.as_deref()).unwrap_or_default();
} = parse_query(raw_url_query.as_deref())
.map_err(|err| Error::Precondition(format!("Invalid query '{err}': {raw_url_query:?}")))?;

// NOTE: We cannot use state.table_metadata() here, since we're working on the logs database.
// We could cache, however this is just the admin logs handler.
Expand Down
5 changes: 2 additions & 3 deletions trailbase-core/src/admin/rows/list_rows.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,16 +32,15 @@ pub async fn list_rows_handler(
Path(table_name): Path<String>,
RawQuery(raw_url_query): RawQuery,
) -> Result<Json<ListRowsResponse>, Error> {
// TODO: we should probably return an error if the query parsing fails rather than quietly
// falling back to defaults.
let QueryParseResult {
params: filter_params,
cursor,
limit,
order,
offset,
..
} = parse_query(raw_url_query.as_deref()).unwrap_or_default();
} = parse_query(raw_url_query.as_deref())
.map_err(|err| Error::Precondition(format!("Invalid query '{err}': {raw_url_query:?}")))?;

let (virtual_table, table_or_view_metadata): (bool, Arc<dyn TableOrViewMetadata + Sync + Send>) = {
if let Some(table_metadata) = state.table_metadata().get(&table_name) {
Expand Down
3 changes: 2 additions & 1 deletion trailbase-core/src/admin/user/list_users.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ pub async fn list_users_handler(
limit,
order,
..
} = parse_query(raw_url_query.as_deref()).unwrap_or_default();
} = parse_query(raw_url_query.as_deref())
.map_err(|err| Error::Precondition(format!("Invalid query '{err}': {raw_url_query:?}")))?;

let Some(table_metadata) = state.table_metadata().get(USER_TABLE) else {
return Err(Error::Precondition(format!("Table {USER_TABLE} not found")));
Expand Down
12 changes: 4 additions & 8 deletions trailbase-core/src/records/list_records.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,20 +55,16 @@ pub async fn list_records_handler(
return Err(RecordError::Internal("missing pk column".into()));
};

let Ok(QueryParseResult {
let QueryParseResult {
params: filter_params,
cursor,
limit,
order,
count,
..
}) = parse_query(raw_url_query.as_deref())
else {
#[cfg(test)]
log::error!("{:?}", raw_url_query);

return Err(RecordError::BadRequest("Bad query"));
};
} = parse_query(raw_url_query.as_deref()).map_err(|_err| {
return RecordError::BadRequest("Invalid query");
})?;

// Where clause contains column filters and cursor depending on what's present.
let WhereClause {
Expand Down

0 comments on commit 5cec548

Please sign in to comment.