Skip to content

Commit

Permalink
fix: bad name on shared types (#955)
Browse files Browse the repository at this point in the history
Since types can be shared, prefixing the name with the first
function/path encountered would be confusing for the other types that
also refers to it.

This patch disables prefixing for types with more than one referrer.

#### Migration notes

None

- [ ] The change comes with new or modified tests
- [ ] Hard-to-understand functions have explanatory comments
- [ ] End-user documentation is updated to reflect the change


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

Based on the comprehensive summary of changes across multiple files,
here are the updated release notes:

- **Type System Refinement**
	- Standardized type naming conventions across multiple languages.
	- Replaced specific type aliases with more generic scalar types.
	- Updated function signatures to use simplified type definitions.

- **Version Updates**
	- Incremented metatype version from "0.5.0-rc.8" to "0.5.0-rc.9".

- **Upload Functionality**
- Modified file upload method return types to return boolean instead of
complex output.
	- Updated argument types for upload-related functions.

- **Performance and Naming Improvements**
	- Enhanced reference counting for type naming processor.
	- Refined type generation logic with more consistent naming strategies.

These changes primarily focus on improving type system consistency and
simplifying type definitions across different runtime environments.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
  • Loading branch information
michael-0acf4 authored Jan 13, 2025
1 parent 445bb5d commit 324dffa
Show file tree
Hide file tree
Showing 20 changed files with 471 additions and 381 deletions.
2 changes: 1 addition & 1 deletion examples/typegraphs/metagen/rs/fdk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ impl Router {
}

pub fn init(&self, args: InitArgs) -> Result<InitResponse, InitError> {
static MT_VERSION: &str = "0.5.0-rc.8";
static MT_VERSION: &str = "0.5.0-rc.9";
if args.metatype_version != MT_VERSION {
return Err(InitError::VersionMismatch(MT_VERSION.into()));
}
Expand Down
26 changes: 13 additions & 13 deletions src/typegate/src/typegraphs/typegate.json
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@
},
{
"type": "object",
"title": "root_typegraphs_fn_input",
"title": "struct_542d7",
"properties": {},
"id": [],
"required": []
Expand All @@ -109,7 +109,7 @@
},
{
"type": "string",
"title": "Typegraph_name_string"
"title": "string_5d176"
},
{
"type": "string",
Expand Down Expand Up @@ -198,7 +198,7 @@
},
{
"type": "string",
"title": "root_addTypegraph_fn_input_fromString_string_json",
"title": "string_json_df54e",
"format": "json"
},
{
Expand Down Expand Up @@ -268,7 +268,7 @@
},
{
"type": "optional",
"title": "root_addTypegraph_fn_output_failure_root_addTypegraph_fn_input_fromString_string_json_optional",
"title": "root_addTypegraph_fn_output_failure_string_json_df54e_optional",
"item": 14,
"default_value": null
},
Expand Down Expand Up @@ -296,12 +296,12 @@
},
{
"type": "list",
"title": "root_removeTypegraphs_fn_input_names_Typegraph_name_string_list",
"title": "string_5d176_list_691ee",
"items": 5
},
{
"type": "boolean",
"title": "root_removeTypegraphs_fn_output"
"title": "boolean_a56cd"
},
{
"type": "function",
Expand Down Expand Up @@ -333,7 +333,7 @@
},
{
"type": "list",
"title": "root_argInfoByPath_fn_input_argPaths_root_removeTypegraphs_fn_input_names_Typegraph_name_string_list_list",
"title": "root_argInfoByPath_fn_input_argPaths_string_5d176_list_691ee_list",
"items": 24
},
{
Expand Down Expand Up @@ -369,18 +369,18 @@
},
{
"type": "optional",
"title": "TypeInfo_enum_TypeInfo_enum_root_addTypegraph_fn_input_fromString_string_json_list_optional",
"title": "TypeInfo_enum_TypeInfo_enum_string_json_df54e_list_optional",
"item": 32,
"default_value": null
},
{
"type": "list",
"title": "TypeInfo_enum_root_addTypegraph_fn_input_fromString_string_json_list",
"title": "TypeInfo_enum_string_json_df54e_list",
"items": 14
},
{
"type": "optional",
"title": "TypeInfo_format_Typegraph_name_string_optional",
"title": "TypeInfo_format_string_5d176_optional",
"item": 5,
"default_value": null
},
Expand Down Expand Up @@ -440,7 +440,7 @@
},
{
"type": "object",
"title": "root_findAvailableOperations_fn_input",
"title": "struct_8f8a3",
"properties": {
"typegraph": 5
},
Expand Down Expand Up @@ -596,7 +596,7 @@
},
{
"type": "object",
"title": "root_execRawPrismaRead_fn_input",
"title": "struct_5fa3d",
"properties": {
"typegraph": 5,
"runtime": 5,
Expand Down Expand Up @@ -777,7 +777,7 @@
},
{
"type": "integer",
"title": "root_queryPrismaModel_fn_input_offset_integer"
"title": "integer_e116e"
},
{
"type": "object",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ expression: typegraph.0
},
{
"type": "integer",
"title": "root_one_fn_input_two_integer",
"title": "integer_f0e37",
"minimum": 12,
"maximum": 44
},
Expand Down
142 changes: 133 additions & 9 deletions src/typegraph/core/src/utils/postprocess/naming.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ use common::typegraph::{
visitor::{Edge, PathSegment},
StringFormat, TypeNode, Typegraph,
};
use indexmap::IndexSet;
use sha2::{Digest, Sha256};

use crate::errors::TgError;

Expand All @@ -25,17 +27,27 @@ impl super::PostProcessor for NamingProcessor {
tg,
user_named: self.user_named,
};
let mut acc = VisitCollector {
named_types: Default::default(),
path: vec![],
};

let TypeNode::Object {
data: root_data, ..
} = &tg.types[0]
else {
panic!("first item must be root object")
};

let mut ref_counters = TypeRefCount {
counts: Default::default(),
};
for (_, &ty_id) in &root_data.properties {
collect_ref_info(&cx, &mut ref_counters, ty_id, &mut HashSet::new())?;
}

let mut acc = VisitCollector {
named_types: Default::default(),
path: vec![],
// ref related
counts: ref_counters.counts,
};
for (key, &ty_id) in &root_data.properties {
acc.path.push((
PathSegment {
Expand All @@ -62,6 +74,41 @@ struct VisitContext<'a> {
struct VisitCollector {
named_types: HashMap<u32, Rc<str>>,
path: Vec<(PathSegment, Rc<str>)>,
// ref related
counts: HashMap<u32, IndexSet<u32>>,
}

struct TypeRefCount {
pub counts: HashMap<u32, IndexSet<u32>>,
}

impl TypeRefCount {
pub fn new_hit(&mut self, id: u32, referrer: u32) {
self.counts
.entry(id)
.and_modify(|counter| {
counter.insert(referrer);
})
.or_insert(IndexSet::from([referrer]));
}
}

impl VisitCollector {
pub fn has_more_than_one_referrer(&self, id: u32) -> bool {
if let Some(referrers) = self.counts.get(&id) {
return referrers.len() > 1;
}

false
}

pub fn next_name(&mut self, name: String, hash_input: String) -> String {
let mut sha256 = Sha256::new();
sha256.update(hash_input.bytes().collect::<Vec<_>>());
let hash = format!("{:x}", sha256.finalize());

format!("{name}_{}", hash.chars().take(5).collect::<String>())
}
}

fn visit_type(cx: &VisitContext, acc: &mut VisitCollector, id: u32) -> anyhow::Result<Rc<str>> {
Expand Down Expand Up @@ -215,14 +262,89 @@ fn visit_type(cx: &VisitContext, acc: &mut VisitCollector, id: u32) -> anyhow::R
};

acc.named_types.insert(id, name.clone());

Ok(name)
}

fn collect_ref_info(
cx: &VisitContext,
acc: &mut TypeRefCount,
id: u32,
visited: &mut HashSet<u32>,
) -> anyhow::Result<()> {
if cx.user_named.contains(&id) || visited.contains(&id) {
return Ok(());
}

visited.insert(id);

let node = &cx.tg.types[id as usize];
match node {
TypeNode::Boolean { .. }
| TypeNode::Float { .. }
| TypeNode::Integer { .. }
| TypeNode::String { .. }
| TypeNode::File { .. }
| TypeNode::Any { .. } => {
// base case
}
TypeNode::Optional { data, .. } => {
acc.new_hit(data.item, id);
collect_ref_info(cx, acc, data.item, visited)?;
}
TypeNode::Object { data, .. } => {
for (_, key_id) in &data.properties {
acc.new_hit(*key_id, id);
collect_ref_info(cx, acc, *key_id, visited)?;
}
}
TypeNode::Function { data, .. } => {
acc.new_hit(data.input, id);
acc.new_hit(data.output, id);

collect_ref_info(cx, acc, data.input, visited)?;
collect_ref_info(cx, acc, data.output, visited)?;
}
TypeNode::List { data, .. } => {
acc.new_hit(data.items, id);
collect_ref_info(cx, acc, data.items, visited)?;
}
TypeNode::Union { data, .. } => {
for variant in &data.any_of {
acc.new_hit(*variant, id);
collect_ref_info(cx, acc, *variant, visited)?;
}
}
TypeNode::Either { data, .. } => {
for variant in &data.one_of {
acc.new_hit(*variant, id);
collect_ref_info(cx, acc, *variant, visited)?;
}
}
};

visited.remove(&id);

Ok(())
}

fn gen_name(cx: &VisitContext, acc: &mut VisitCollector, id: u32, ty_name: &str) -> Rc<str> {
let node = &cx.tg.types[id as usize];
let name: Rc<str> = if cx.user_named.contains(&id) {
let node = &cx.tg.types[id as usize];
node.base().title.clone().into()
} else {
macro_rules! join_if_ok {
($prefix:expr, $default:expr) => {
if acc.has_more_than_one_referrer(id) {
// Cannot be opinionated on the prefix path if shared (confusing)
let hash_input = $prefix;
acc.next_name(ty_name.to_string(), hash_input)
} else {
format!("{}_{}", $prefix, $default)
}
};
}

let title;
let mut last = acc.path.len();
loop {
Expand All @@ -232,11 +354,13 @@ fn gen_name(cx: &VisitContext, acc: &mut VisitCollector, id: u32, ty_name: &str)
// we don't include optional and list nodes in
// generated names (useless but also, they might be placeholders)
Edge::OptionalItem | Edge::ArrayItem => continue,
Edge::FunctionInput => format!("{last_name}_input"),
Edge::FunctionOutput => format!("{last_name}_output"),
Edge::ObjectProp(key) => format!("{last_name}_{key}_{ty_name}"),
Edge::FunctionInput => join_if_ok!(last_name.to_string(), "input"),
Edge::FunctionOutput => join_if_ok!(last_name.to_string(), "output"),
Edge::ObjectProp(key) => {
join_if_ok!(format!("{last_name}_{key}"), ty_name)
}
Edge::EitherVariant(idx) | Edge::UnionVariant(idx) => {
format!("{last_name}_t{idx}_{ty_name}")
join_if_ok!(format!("{last_name}_t{idx}"), ty_name)
}
};
break;
Expand Down
Loading

0 comments on commit 324dffa

Please sign in to comment.