mirror of
https://github.com/ModernRelay/omnigraph.git
synced 2026-06-09 01:35:18 +02:00
Polish schema endpoint: rename show, align field name, add tests
Review feedback on #23, applied on top of the original commit: - Rename the CLI subcommand from `schema get` to `schema show` to match the existing `run show` / `commit show` convention. A `#[command(alias = "get")]` preserves muscle memory for anyone who already typed `get`. - Rename `SchemaGetOutput` → `SchemaOutput` and its field `source` → `schema_source`, so the get response and the apply request use the same field name for the same concept. - Use `println!` instead of `print!` in the CLI so the shell prompt doesn't land on the last line of schema output. - Add three integration tests on `/schema`: happy path (no auth), 401 when bearer is required but missing, 403 when the policy grants the actor branch_create but not read. Follow-ups left for a separate PR: include `schema_ir_hash` and `schema_identity_version` in the response payload so clients can do drift detection and the server can set an ETag; and a fast-path local read that skips `Omnigraph::open()` when only the schema source is needed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
0c4df674fa
commit
be520f31f4
4 changed files with 104 additions and 16 deletions
|
|
@ -18,7 +18,7 @@ use omnigraph_server::api::{
|
|||
BranchCreateOutput, BranchCreateRequest, BranchDeleteOutput, BranchListOutput,
|
||||
BranchMergeOutput, BranchMergeRequest, ChangeOutput, ChangeRequest, CommitListOutput,
|
||||
CommitOutput, ErrorOutput, ExportRequest, IngestOutput, IngestRequest, ReadOutput, ReadRequest,
|
||||
RunListOutput, RunOutput, SchemaApplyOutput, SchemaApplyRequest, SchemaGetOutput, SnapshotOutput,
|
||||
RunListOutput, RunOutput, SchemaApplyOutput, SchemaApplyRequest, SchemaOutput, SnapshotOutput,
|
||||
SnapshotTableOutput, commit_output, ingest_output, read_output, run_output,
|
||||
schema_apply_output, snapshot_payload,
|
||||
};
|
||||
|
|
@ -303,8 +303,9 @@ enum SchemaCommand {
|
|||
#[arg(long)]
|
||||
json: bool,
|
||||
},
|
||||
/// Get the current accepted schema source
|
||||
Get {
|
||||
/// Show the current accepted schema source
|
||||
#[command(alias = "get")]
|
||||
Show {
|
||||
/// Repo URI
|
||||
uri: Option<String>,
|
||||
#[arg(long)]
|
||||
|
|
@ -2014,7 +2015,7 @@ async fn main() -> Result<()> {
|
|||
print_schema_apply_human(&output);
|
||||
}
|
||||
}
|
||||
SchemaCommand::Get {
|
||||
SchemaCommand::Show {
|
||||
uri,
|
||||
target,
|
||||
config,
|
||||
|
|
@ -2025,7 +2026,7 @@ async fn main() -> Result<()> {
|
|||
resolve_remote_bearer_token(&config, uri.as_deref(), target.as_deref())?;
|
||||
let uri = resolve_uri(&config, uri, target.as_deref())?;
|
||||
let output = if is_remote_uri(&uri) {
|
||||
remote_json::<SchemaGetOutput>(
|
||||
remote_json::<SchemaOutput>(
|
||||
&http_client,
|
||||
Method::GET,
|
||||
remote_url(&uri, "/schema"),
|
||||
|
|
@ -2035,14 +2036,14 @@ async fn main() -> Result<()> {
|
|||
.await?
|
||||
} else {
|
||||
let db = Omnigraph::open(&uri).await?;
|
||||
SchemaGetOutput {
|
||||
source: db.schema_source().to_string(),
|
||||
SchemaOutput {
|
||||
schema_source: db.schema_source().to_string(),
|
||||
}
|
||||
};
|
||||
if json {
|
||||
print_json(&output)?;
|
||||
} else {
|
||||
print!("{}", output.source);
|
||||
println!("{}", output.schema_source);
|
||||
}
|
||||
}
|
||||
},
|
||||
|
|
|
|||
|
|
@ -281,8 +281,8 @@ pub struct SchemaApplyOutput {
|
|||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, ToSchema)]
|
||||
pub struct SchemaGetOutput {
|
||||
pub source: String,
|
||||
pub struct SchemaOutput {
|
||||
pub schema_source: String,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize, ToSchema)]
|
||||
|
|
|
|||
|
|
@ -14,7 +14,7 @@ use api::{
|
|||
BranchMergeOutput, BranchMergeRequest, ChangeOutput, ChangeRequest, CommitListOutput,
|
||||
CommitListQuery, ErrorCode, ErrorOutput, ExportRequest, HealthOutput, IngestOutput,
|
||||
IngestRequest, ReadOutput, ReadRequest, RunListOutput, SchemaApplyOutput, SchemaApplyRequest,
|
||||
SchemaGetOutput, SnapshotQuery, ingest_output, schema_apply_output, snapshot_payload,
|
||||
SchemaOutput, SnapshotQuery, ingest_output, schema_apply_output, snapshot_payload,
|
||||
};
|
||||
use axum::body::{Body, Bytes};
|
||||
use axum::extract::DefaultBodyLimit;
|
||||
|
|
@ -803,7 +803,7 @@ async fn server_change(
|
|||
path = "/schema",
|
||||
tag = "schema",
|
||||
responses(
|
||||
(status = 200, description = "Current schema source", body = SchemaGetOutput),
|
||||
(status = 200, description = "Current schema source", body = SchemaOutput),
|
||||
(status = 401, description = "Unauthorized", body = ErrorOutput),
|
||||
(status = 403, description = "Forbidden", body = ErrorOutput),
|
||||
),
|
||||
|
|
@ -812,7 +812,7 @@ async fn server_change(
|
|||
async fn server_schema_get(
|
||||
State(state): State<AppState>,
|
||||
actor: Option<Extension<AuthenticatedActor>>,
|
||||
) -> std::result::Result<Json<SchemaGetOutput>, ApiError> {
|
||||
) -> std::result::Result<Json<SchemaOutput>, ApiError> {
|
||||
authorize_request(
|
||||
&state,
|
||||
actor.as_ref().map(|Extension(actor)| actor),
|
||||
|
|
@ -826,11 +826,11 @@ async fn server_schema_get(
|
|||
target_branch: None,
|
||||
},
|
||||
)?;
|
||||
let source = {
|
||||
let schema_source = {
|
||||
let db = Arc::clone(&state.db).read_owned().await;
|
||||
db.schema_source().to_string()
|
||||
};
|
||||
Ok(Json(SchemaGetOutput { source }))
|
||||
Ok(Json(SchemaOutput { schema_source }))
|
||||
}
|
||||
|
||||
#[utoipa::path(
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@ use omnigraph::db::{Omnigraph, ReadTarget};
|
|||
use omnigraph::loader::{LoadMode, load_jsonl};
|
||||
use omnigraph_server::api::{
|
||||
BranchCreateRequest, BranchMergeRequest, ChangeRequest, ErrorOutput, ExportRequest,
|
||||
IngestRequest, ReadRequest, SchemaApplyRequest,
|
||||
IngestRequest, ReadRequest, SchemaApplyRequest, SchemaOutput,
|
||||
};
|
||||
use omnigraph_server::{AppState, build_app};
|
||||
use serde_json::{Value, json};
|
||||
|
|
@ -1042,6 +1042,93 @@ async fn snapshot_route_returns_manifest_dataset_version() {
|
|||
assert!(snapshot_body["tables"].is_array());
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn schema_route_returns_current_source() {
|
||||
let (_temp, app) = app_for_loaded_repo().await;
|
||||
let (status, body) = json_response(
|
||||
&app,
|
||||
Request::builder()
|
||||
.uri("/schema")
|
||||
.method(Method::GET)
|
||||
.body(Body::empty())
|
||||
.unwrap(),
|
||||
)
|
||||
.await;
|
||||
|
||||
assert_eq!(status, StatusCode::OK);
|
||||
let output: SchemaOutput = serde_json::from_value(body).unwrap();
|
||||
assert!(output.schema_source.contains("node Person"));
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn schema_route_requires_bearer_token_when_auth_configured() {
|
||||
let (_temp, app) = app_for_loaded_repo_with_auth("demo-token").await;
|
||||
|
||||
let (missing_status, missing_body) = json_response(
|
||||
&app,
|
||||
Request::builder()
|
||||
.uri("/schema")
|
||||
.method(Method::GET)
|
||||
.body(Body::empty())
|
||||
.unwrap(),
|
||||
)
|
||||
.await;
|
||||
let missing_error: ErrorOutput = serde_json::from_value(missing_body).unwrap();
|
||||
assert_eq!(missing_status, StatusCode::UNAUTHORIZED);
|
||||
assert_eq!(
|
||||
missing_error.code,
|
||||
Some(omnigraph_server::api::ErrorCode::Unauthorized)
|
||||
);
|
||||
|
||||
let (ok_status, ok_body) = json_response(
|
||||
&app,
|
||||
Request::builder()
|
||||
.uri("/schema")
|
||||
.method(Method::GET)
|
||||
.header("authorization", "Bearer demo-token")
|
||||
.body(Body::empty())
|
||||
.unwrap(),
|
||||
)
|
||||
.await;
|
||||
assert_eq!(ok_status, StatusCode::OK);
|
||||
let output: SchemaOutput = serde_json::from_value(ok_body).unwrap();
|
||||
assert!(!output.schema_source.is_empty());
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn schema_route_denied_when_actor_lacks_read_permission() {
|
||||
let temp = init_loaded_repo().await;
|
||||
let repo = repo_path(temp.path());
|
||||
let policy_path = temp.path().join("policy.yaml");
|
||||
// Policy grants branch_create only — no read action for act-bruno.
|
||||
fs::write(&policy_path, INGEST_CREATE_ONLY_POLICY_YAML).unwrap();
|
||||
let state = AppState::open_with_bearer_tokens_and_policy(
|
||||
repo.to_string_lossy().to_string(),
|
||||
vec![("act-bruno".to_string(), "team-token".to_string())],
|
||||
Some(&policy_path),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
let app = build_app(state);
|
||||
|
||||
let (status, body) = json_response(
|
||||
&app,
|
||||
Request::builder()
|
||||
.uri("/schema")
|
||||
.method(Method::GET)
|
||||
.header("authorization", "Bearer team-token")
|
||||
.body(Body::empty())
|
||||
.unwrap(),
|
||||
)
|
||||
.await;
|
||||
let error: ErrorOutput = serde_json::from_value(body).unwrap();
|
||||
assert_eq!(status, StatusCode::FORBIDDEN);
|
||||
assert_eq!(
|
||||
error.code,
|
||||
Some(omnigraph_server::api::ErrorCode::Forbidden)
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread")]
|
||||
async fn policy_blocks_change_on_protected_main_but_allows_unprotected_branch() {
|
||||
let temp = init_loaded_repo().await;
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue