Skip to content

Commit

Permalink
fix(grpc): import proto files relative to parent proto (#1705)
Browse files Browse the repository at this point in the history
  • Loading branch information
ssddOnTop authored Apr 19, 2024
1 parent 03f9829 commit ed7bdc6
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 17 deletions.
4 changes: 2 additions & 2 deletions src/grpc/tests/proto/cycle.proto
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@ syntax = "proto3";

package cycle;

import "src/grpc/tests/proto/nested0.proto";
import "src/grpc/tests/proto/duplicate.proto";
import "nested0.proto";
import "duplicate.proto";
4 changes: 2 additions & 2 deletions src/grpc/tests/proto/duplicate.proto
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@ syntax = "proto3";

package duplicate;

import "src/grpc/tests/proto/greetings.proto";
import "src/grpc/tests/proto/news.proto";
import "greetings.proto";
import "news.proto";
6 changes: 3 additions & 3 deletions src/grpc/tests/proto/nested0.proto
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@ syntax = "proto3";

package nested0;

import "src/grpc/tests/proto/greetings.proto";
import "src/grpc/tests/proto/nested1.proto";
import "src/grpc/tests/proto/news_no_pkg.proto";
import "greetings.proto";
import "nested1.proto";
import "news_no_pkg.proto";
4 changes: 2 additions & 2 deletions src/grpc/tests/proto/nested1.proto
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@ syntax = "proto3";

package nested1;

import "src/grpc/tests/proto/news.proto";
import "src/grpc/tests/proto/cycle.proto";
import "news.proto";
import "cycle.proto";
41 changes: 33 additions & 8 deletions src/proto_reader.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use std::collections::{HashMap, VecDeque};
use std::path::{Path, PathBuf};

use anyhow::Context;
use futures_util::future::join_all;
Expand Down Expand Up @@ -30,12 +31,14 @@ impl ProtoReader {
}

pub async fn read<T: AsRef<str>>(&self, path: T) -> anyhow::Result<ProtoMetadata> {
let file_read = self.read_proto(path.as_ref()).await?;
let file_read = self.read_proto(path.as_ref(), None).await?;
if file_read.package.is_none() {
anyhow::bail!("Package name is required");
}

let descriptors = self.resolve_descriptors(file_read).await?;
let descriptors = self
.resolve_descriptors(file_read, PathBuf::from(path.as_ref()).parent())
.await?;
let metadata = ProtoMetadata {
descriptor_set: FileDescriptorSet { file: descriptors },
path: path.as_ref().to_string(),
Expand All @@ -47,6 +50,7 @@ impl ProtoReader {
async fn resolve_descriptors(
&self,
parent_proto: FileDescriptorProto,
parent_path: Option<&Path>,
) -> anyhow::Result<Vec<FileDescriptorProto>> {
let mut descriptors: HashMap<String, FileDescriptorProto> = HashMap::new();
let mut queue = VecDeque::new();
Expand All @@ -56,7 +60,7 @@ impl ProtoReader {
let futures: Vec<_> = file
.dependency
.iter()
.map(|import| self.read_proto(import))
.map(|import| self.read_proto(import, parent_path))
.collect();

let results = join_all(futures).await;
Expand All @@ -79,15 +83,35 @@ impl ProtoReader {

/// Tries to load well-known google proto files and if not found uses normal
/// file and http IO to resolve them
async fn read_proto(&self, path: &str) -> anyhow::Result<FileDescriptorProto> {
let content = if let Ok(file) = GoogleFileResolver::new().open_file(path) {
async fn read_proto<T: AsRef<str>>(
&self,
path: T,
parent_dir: Option<&Path>,
) -> anyhow::Result<FileDescriptorProto> {
let content = if let Ok(file) = GoogleFileResolver::new().open_file(path.as_ref()) {
file.source()
.context("Unable to extract content of google well-known proto file")?
.to_string()
} else {
let path = Self::resolve_path(path.as_ref(), parent_dir);
self.resource_reader.read_file(path).await?.content
};
Ok(protox_parse::parse(path, &content)?)
Ok(protox_parse::parse(path.as_ref(), &content)?)
}
/// Checks if path is absolute else it joins file path with relative dir
/// path
fn resolve_path(src: &str, root_dir: Option<&Path>) -> String {
if src.starts_with("http") {
return src.to_string();
}

if Path::new(&src).is_absolute() {
src.to_string()
} else if let Some(path) = root_dir {
path.join(src).to_string_lossy().to_string()
} else {
src.to_string()
}
}
}

Expand All @@ -108,7 +132,7 @@ mod test_proto_config {
crate::runtime::test::init(None),
));
reader
.read_proto("google/protobuf/empty.proto")
.read_proto("google/protobuf/empty.proto", None)
.await
.unwrap();
}
Expand Down Expand Up @@ -136,8 +160,9 @@ mod test_proto_config {
let file_rt = runtime.file.clone();

let reader = ProtoReader::init(ResourceReader::<Cached>::cached(runtime));
let pathbuf = PathBuf::from(&test_file);
let helper_map = reader
.resolve_descriptors(reader.read_proto(&test_file).await?)
.resolve_descriptors(reader.read_proto(&test_file, None).await?, pathbuf.parent())
.await?;
let files = test_dir.read_dir()?;
for file in files {
Expand Down

1 comment on commit ed7bdc6

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running 30s test @ http://localhost:8000/graphql

4 threads and 100 connections

Thread Stats Avg Stdev Max +/- Stdev
Latency 7.43ms 3.36ms 92.37ms 72.09%
Req/Sec 3.41k 178.05 3.66k 92.50%

406689 requests in 30.01s, 2.04GB read

Requests/sec: 13553.50

Transfer/sec: 69.57MB

Please sign in to comment.