Skip to content

Commit

Permalink
tsparser: detect conflicting paths in endpoints (encoredev#1465)
Browse files Browse the repository at this point in the history
Example error:
```
error: api endpoints with conflicting paths defined
  --> /home/fredr/projects/encore-apps/teeeest/repro/repro.ts:29:22
   |
29 |   export const same2 = api(
   |  ______________________^
30 | |   { expose: true, method: "POST", path: "/same/:hello" },
31 | |   async ({ hello }: { hello: string }) => {},
32 | | );
   | |_^
   |
note: previously defined here
  --> /home/fredr/projects/encore-apps/teeeest/repro/repro.ts:25:22
   |
25 |   export const same1 = api(
   |  ______________________^
26 | |   { expose: true, method: "POST", path: "/same/:id" },
27 | |   async ({ id }: { id: string }) => {},
28 | | );
   | |_^

```
  • Loading branch information
fredr authored Oct 10, 2024
1 parent 4bbf64e commit e471951
Show file tree
Hide file tree
Showing 7 changed files with 79 additions and 5 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ class SvcServiceClient {
"some-query": params.queryValue,
})

return await this.baseClient.createStreamOut(`/in/withResponse`, {headers, query})
return await this.baseClient.createStreamOut(`/in/withResponseAndHandshake`, {headers, query})
}

async inWithoutHandshake() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ export namespace svc {
"some-query": params.queryValue,
})

return await this.baseClient.createStreamOut(`/in/withResponse`, {headers, query})
return await this.baseClient.createStreamOut(`/in/withResponseAndHandshake`, {headers, query})
}

public async inWithoutHandshake(): Promise<StreamOut<InMsg, void>> {
Expand Down
2 changes: 1 addition & 1 deletion internal/clientgen/testdata/tsapp/input_stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,6 @@ export const inWithResponse = api.streamIn<InMsg, OutMsg>(
);

export const inWithResponseAndHandshake = api.streamIn<Handshake, InMsg, OutMsg>(
{ expose: true, path: "/in/withResponse" },
{ expose: true, path: "/in/withResponseAndHandshake" },
async (handshake: Handshake, stream) => {},
);
1 change: 1 addition & 0 deletions tsparser/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ tracing = "0.1.40"
tracing-subscriber = { version = "0.3.18", features = ["env-filter"] }
junction = "1.2.0"
thiserror = "1.0.64"
matchit = "0.7.3"

[build-dependencies]
prost-build = { version = "0.12.1" }
Expand Down
74 changes: 73 additions & 1 deletion tsparser/src/app/mod.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,88 @@
use std::collections::HashMap;

use anyhow::Result;
use matchit::InsertError;
use swc_common::errors::HANDLER;

use crate::encore::parser::meta::v1;
use crate::legacymeta::compute_meta;
use crate::parser::parser::{ParseContext, ParseResult};
use crate::parser::resources::apis::api::{Method, Methods};
use crate::parser::resources::Resource;
use crate::parser::respath::Path;
use crate::parser::Range;

#[derive(Debug)]
pub struct AppDesc {
pub parse: ParseResult,
pub meta: v1::Data,
}

struct Router {
methods: HashMap<Method, matchit::Router<Range>>,
}

impl Router {
fn new() -> Self {
Router {
methods: HashMap::new(),
}
}
}

impl Router {
fn try_add(&mut self, methods: &Methods, path: &Path, range: Range) {
let methods = match methods {
Methods::All => Method::all().to_vec(),
Methods::Some(vec) => vec.to_vec(),
};

for method in methods {
let method_router = self.methods.entry(method).or_default();
if let Err(e) = method_router.insert(path.to_string(), range) {
match e {
InsertError::Conflict { with } => {
let prev_range = *method_router.at(&with).unwrap().value;
HANDLER.with(|handler| {
handler
.struct_span_err(
range,
"api endpoints with conflicting paths defined",
)
.span_note(prev_range, "previously defined here")
.emit()
})
}
_ => HANDLER.with(|handler| handler.span_err(range, &e.to_string())),
}
}
}
}
}

impl AppDesc {
fn validate(&self) {
self.validate_apis()
}

fn validate_apis(&self) {
for service in self.parse.services.iter() {
let mut router = Router::new();
for bind in &service.binds {
if let Resource::APIEndpoint(endpoint) = &bind.resource {
let encoding = &endpoint.encoding;
router.try_add(&encoding.methods, &encoding.path, endpoint.range);
}
}
}
}
}

pub fn validate_and_describe(pc: &ParseContext, parse: ParseResult) -> Result<AppDesc> {
let meta = compute_meta(pc, &parse)?;
Ok(AppDesc { parse, meta })
let desc = AppDesc { parse, meta };

desc.validate();

Ok(desc)
}
2 changes: 1 addition & 1 deletion tsparser/src/parser/resources/apis/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ impl Methods {
}
}

#[derive(Debug, Clone, Copy, PartialOrd, Ord, PartialEq, Eq)]
#[derive(Debug, Clone, Copy, PartialOrd, Ord, PartialEq, Eq, Hash)]
pub enum Method {
Get,
Post,
Expand Down

0 comments on commit e471951

Please sign in to comment.