[db-macros] support for children without names in lookup_resource! (#7508)

Presently, the `lookup_resource!` macro generates code that will not
compile when a child resource has `lookup_by_name = false` in its
`lookup_resource!` macro invocation.

This is because the code for selecting a child resource by name given
the parent is generated in the `lookup_resource!` invocation for the
*parent* resource, which doesn't *know* whether the child has
`lookup_by_name = true` or `lookup_by_name = false`. This means that
this code is unconditionally generated, even when there is no name
column for the child (i.e., it is an `Asset` rather than a `Resource`).

While we currently have plenty of `lookup_resource!` invocations for
`Asset`s, none of them are currently the child of another resource.
While working on the webhook implementation, I wanted the
`WebhookSecret` resource to be a child of the `WebhookReceiver` resource
for authz purposes, but secrets do not have names. This resulted in the
`lookup_resource!` macro generating code that wouldn't compile. I've
cherry-picked this commit from that branch to try and land it
separately.

This commit fixes that issue by changing the child selector functions
from being generated by the parent's `lookup_resource!` invocation to
instead being generated by the child's `lookup_resource!` invocation.
This way, we can decide whether or not to generate the child-by-name
selectors based on whether or not the child has `lookup_by_name = true`
or not. Since all the `lookup_resource!` invocations are in the same
file, it's alright for the child's `lookup_resource!` invocation to
generate `impl` blocks for the parent as well.

Furthermore, as the list of `children` in the parent resource's
`lookup_resource!` invocation is no longer necessary, I've removed it.
Now, `lookup_resource!` invocations need only provide a list of
ancestors, and need not define their children. This is a bit nicer, as
the relationships are only defined in one place.

Author: hawkw

nexus/db-macros/outputs/project.txt

@@ -15,36 +15,6 @@ pub enum Project<'a> {
     PrimaryKey(Root<'a>, Uuid),
 }
 impl<'a> Project<'a> {
-    ///Select a resource of type Disk within this Project, identified by its name
-    pub fn disk_name<'b, 'c>(self, name: &'b Name) -> Disk<'c>
-    where
-        'a: 'c,
-        'b: 'c,
-    {
-        Disk::Name(self, name)
-    }
-    ///Select a resource of type Disk within this Project, identified by its name
-    pub fn disk_name_owned<'c>(self, name: Name) -> Disk<'c>
-    where
-        'a: 'c,
-    {
-        Disk::OwnedName(self, name)
-    }
-    ///Select a resource of type Instance within this Project, identified by its name
-    pub fn instance_name<'b, 'c>(self, name: &'b Name) -> Instance<'c>
-    where
-        'a: 'c,
-        'b: 'c,
-    {
-        Instance::Name(self, name)
-    }
-    ///Select a resource of type Instance within this Project, identified by its name
-    pub fn instance_name_owned<'c>(self, name: Name) -> Instance<'c>
-    where
-        'a: 'c,
-    {
-        Instance::OwnedName(self, name)
-    }
     /// Fetch the record corresponding to the selected resource
     ///
     /// This is equivalent to `fetch_for(authz::Action::Read)`.
@@ -393,3 +363,20 @@ impl<'a> Project<'a> {
         Ok((authz_silo, authz_project, db_row))
     }
 }
+impl<'a> Silo<'a> {
+    ///Select a resource of type Project within this Silo, identified by its name
+    pub fn project_name<'b, 'c>(self, name: &'b Name) -> Project<'c>
+    where
+        'a: 'c,
+        'b: 'c,
+    {
+        Project::Name(self, name)
+    }
+    ///Select a resource of type Project within this Silo, identified by its name
+    pub fn project_name_owned<'c>(self, name: Name) -> Project<'c>
+    where
+        'a: 'c,
+    {
+        Project::OwnedName(self, name)
+    }
+}

nexus/db-macros/src/lib.rs

@@ -32,7 +32,6 @@ mod test_helpers;
 /// lookup_resource! {
 ///     name = "Organization",
 ///     ancestors = [],
-///     children = [ "Project" ],
 ///     lookup_by_name = true,
 ///     soft_deletes = true,
 ///     primary_key_columns = [ { column_name = "id", rust_type = Uuid } ]
@@ -52,7 +51,6 @@ mod test_helpers;
 /// lookup_resource! {
 ///     name = "Organization",
 ///     ancestors = [],
-///     children = [ "Project" ],
 ///     lookup_by_name = true,
 ///     soft_deletes = true,
 ///     primary_key_columns = [ { column_name = "id", rust_type = Uuid } ]
@@ -61,7 +59,6 @@ mod test_helpers;
 /// lookup_resource! {
 ///     name = "Instance",
 ///     ancestors = [ "Organization", "Project" ],
-///     children = [],
 ///     lookup_by_name = true,
 ///     soft_deletes = true,
 ///     primary_key_columns = [ { column_name = "id", rust_type = Uuid } ]
@@ -77,7 +74,6 @@ mod test_helpers;
 /// lookup_resource! {
 ///     name = "Sled",
 ///     ancestors = [ "Organization", "Project" ],
-///     children = [],
 ///     lookup_by_name = true,
 ///     soft_deletes = true,
 ///     primary_key_columns = [ { column_name = "id", uuid_kind = SledType } ]

nexus/db-macros/src/lookup.rs

@@ -34,9 +34,6 @@ pub struct Input {
     /// with the top of the hierarchy
     /// (e.g., for an Instance, this would be `[ "Silo", "Project" ]`
     ancestors: Vec<String>,
-    /// unordered list of resources that are direct children of this resource
-    /// (e.g., for a Project, these would include "Instance" and "Disk")
-    children: Vec<String>,
     /// whether lookup by name is supported (usually within the parent collection)
     lookup_by_name: bool,
     /// Description of the primary key columns
@@ -127,11 +124,6 @@ pub struct Config {
     /// [`authz_silo`, `authz_project`])
     path_authz_names: Vec<syn::Ident>,
 
-    // Child resources
-    /// list of names of child resources, in the same form and with the same
-    /// assumptions as [`Input::name`] (i.e., typically PascalCase)
-    child_resources: Vec<String>,
-
     // Parent resource, if any
     /// Information about the parent resource, if any
     parent: Option<Resource>,
@@ -160,7 +152,6 @@ impl Config {
             .collect();
         path_authz_names.push(resource.authz_name.clone());
 
-        let child_resources = input.children;
         let parent = input.ancestors.last().map(|s| Resource::for_name(s));
         let silo_restricted = !input.visible_outside_silo
             && input.ancestors.iter().any(|s| s == "Silo");
@@ -177,7 +168,6 @@ impl Config {
             path_types,
             path_authz_names,
             parent,
-            child_resources,
             lookup_by_name: input.lookup_by_name,
             primary_key_columns,
             soft_deletes: input.soft_deletes,
@@ -221,22 +211,22 @@ pub fn lookup_resource(
     let resource_name = &config.resource.name;
     let the_basics = generate_struct(&config);
     let misc_helpers = generate_misc_helpers(&config);
-    let child_selectors = generate_child_selectors(&config);
+    let child_selector = generate_child_selector(&config);
     let lookup_methods = generate_lookup_methods(&config);
     let database_functions = generate_database_functions(&config);
 
     Ok(quote! {
         #the_basics
 
         impl<'a> #resource_name<'a> {
-            #child_selectors
-
             #lookup_methods
 
             #misc_helpers
 
             #database_functions
         }
+
+        #child_selector
     })
 }
 
@@ -287,63 +277,70 @@ fn generate_struct(config: &Config) -> TokenStream {
     }
 }
 
-/// Generates the child selectors for this resource
+/// Generates the child selector for this resource's parent
 ///
-/// For example, for the "Project" resource with child resources "Instance" and
-/// "Disk", this will generate the `Project::instance_name()` and
-/// `Project::disk_name()` functions.
-fn generate_child_selectors(config: &Config) -> TokenStream {
-    let child_resource_types: Vec<_> =
-        config.child_resources.iter().map(|c| format_ident!("{}", c)).collect();
-    let child_selector_fn_names: Vec<_> = config
-        .child_resources
-        .iter()
-        .map(|c| format_ident!("{}_name", heck::AsSnakeCase(c).to_string()))
-        .collect();
-    let child_selector_fn_names_owned: Vec<_> = config
-        .child_resources
-        .iter()
-        .map(|c| {
-            format_ident!("{}_name_owned", heck::AsSnakeCase(c).to_string())
-        })
-        .collect();
-    let child_selector_fn_docs: Vec<_> = config
-        .child_resources
-        .iter()
-        .map(|child| {
-            format!(
-                "Select a resource of type {} within this {}, \
-                identified by its name",
-                child, config.resource.name,
-            )
-        })
-        .collect();
+/// For example, for the "Instance" resource with parent resource "Project",
+/// this will generate the `Project::instance_name()` and
+/// `Project::instance_name_owned()` functions.
+///
+/// This is generated by the child resource codegen, rather than by the parent
+/// resource codegen, since such functions are only generated for resources with
+/// `lookup_by_name = true`.  Whether or not this is enabled for the child
+/// resource is not known when generating the parent resource code, so it's
+/// performed by the child instead.
+fn generate_child_selector(config: &Config) -> TokenStream {
+    // If this resource cannot be looked up by name, we don't need to generate
+    // child selectors on the parent resource.
+    if !config.lookup_by_name {
+        return quote! {};
+    }
+
+    // The child selector is generated for the parent resource type. If there
+    // isn't one, nothing to do here.
+    let Some(ref parent) = config.parent else { return quote! {} };
+
+    let parent_resource_type = &parent.name;
+    let child_selector_fn_name = format_ident!(
+        "{}_name",
+        heck::AsSnakeCase(config.resource.name.to_string()).to_string()
+    );
+
+    let child_selector_fn_name_owned = format_ident!(
+        "{}_name_owned",
+        heck::AsSnakeCase(config.resource.name.to_string()).to_string()
+    );
+    let child_resource_type = &config.resource.name;
+
+    let child_selector_fn_docs = format!(
+        "Select a resource of type {child_resource_type} within this \
+        {parent_resource_type}, identified by its name",
+    );
 
     quote! {
-        #(
+        impl<'a> #parent_resource_type<'a> {
             #[doc = #child_selector_fn_docs]
-            pub fn #child_selector_fn_names<'b, 'c>(
+            pub fn #child_selector_fn_name<'b, 'c>(
                 self,
                 name: &'b Name
-            ) -> #child_resource_types<'c>
+            ) -> #child_resource_type<'c>
             where
                 'a: 'c,
                 'b: 'c,
             {
-                #child_resource_types::Name(self, name)
+                #child_resource_type::Name(self, name)
             }
 
             #[doc = #child_selector_fn_docs]
-            pub fn #child_selector_fn_names_owned<'c>(
+            pub fn #child_selector_fn_name_owned<'c>(
                 self,
                 name: Name,
-            ) -> #child_resource_types<'c>
+            ) -> #child_resource_type<'c>
             where
                 'a: 'c,
             {
-                #child_resource_types::OwnedName(self, name)
+                #child_resource_type::OwnedName(self, name)
             }
-        )*
+        }
     }
 }
 
@@ -991,7 +988,6 @@ mod test {
         let output = lookup_resource(quote! {
             name = "Project",
             ancestors = ["Silo"],
-            children = [ "Disk", "Instance" ],
             lookup_by_name = true,
             soft_deletes = true,
             primary_key_columns = [ { column_name = "id", rust_type = Uuid } ]
@@ -1002,7 +998,6 @@ mod test {
         let output = lookup_resource(quote! {
             name = "SiloUser",
             ancestors = [],
-            children = [],
             lookup_by_name = false,
             soft_deletes = true,
             primary_key_columns = [ { column_name = "id", rust_type = Uuid } ]
@@ -1013,7 +1008,6 @@ mod test {
         let output = lookup_resource(quote! {
             name = "Sled",
             ancestors = [],
-            children = [],
             lookup_by_name = false,
             soft_deletes = true,
             primary_key_columns = [ { column_name = "id", uuid_kind = SledKind } ]
@@ -1024,7 +1018,6 @@ mod test {
         let output = lookup_resource(quote! {
             name = "UpdateArtifact",
             ancestors = [],
-            children = [],
             lookup_by_name = false,
             soft_deletes = false,
             primary_key_columns = [