Fixup generic func comments

coder · johnstcn · Feb 14, 2023 · Jan 27, 2023 · Jan 30, 2023 · Jan 30, 2023
commit ba172ea75e2bd28dbff026df578f59aa9fbb6dd7
diff --git a/coderd/authzquery/authz.go b/coderd/authzquery/authz.go
@@ -54,6 +54,7 @@ func LogNotAuthorizedError(ctx context.Context, logger slog.Logger, err error) e
 	}
 }
 
+// insert is the same as insertWithReturn, but does not return the inserted object.
 func insert[ArgumentType any,
 	Insert func(ctx context.Context, arg ArgumentType) error](
 	// Arguments
@@ -69,6 +70,9 @@ func insert[ArgumentType any,
 	}
 }
 
+// insertWithReturn runs an rbac.ActionCreate on the rbac object argument before
+// running the insertFunc. The insertFunc is expected to return the object that
+// was inserted.
 func insertWithReturn[ObjectType any, ArgumentType any,
 	Insert func(ctx context.Context, arg ArgumentType) (ObjectType, error)](
 	// Arguments
@@ -130,9 +134,45 @@ func update[ObjectType rbac.Objecter,
 	return fetchAndExec(logger, authorizer, rbac.ActionUpdate, fetchFunc, updateExec)
 }
 
-// authorizedFetchAndExecWithConverter uses authorizedFetchAndQueryWithConverter but
-// only cares about the error return type. SQL execs only return an error.
-// See authorizedFetchAndQueryWithConverter for more details.
+// fetch is a generic function that wraps a database
+// query function (returns an object and an error) with authorization. The
+// returned function has the same arguments as the database function.
+//
+// The database query function will **ALWAYS** hit the database, even if the
+// user cannot read the resource. This is because the resource details are
+// required to run a proper authorization check.
+func fetch[ArgumentType any, ObjectType rbac.Objecter,
+	DatabaseFunc func(ctx context.Context, arg ArgumentType) (ObjectType, error)](
+	// Arguments
+	logger slog.Logger,
+	authorizer rbac.Authorizer,
+	f DatabaseFunc) DatabaseFunc {
+	return func(ctx context.Context, arg ArgumentType) (empty ObjectType, err error) {
+		// Fetch the rbac subject
+		act, ok := ActorFromContext(ctx)
+		if !ok {
+			return empty, NoActorError
+		}
+
+		// Fetch the database object
+		object, err := f(ctx, arg)
+		if err != nil {
+			return empty, xerrors.Errorf("fetch object: %w", err)
+		}
+
+		// Authorize the action
+		err = authorizer.Authorize(ctx, act, rbac.ActionRead, object.RBACObject())
+		if err != nil {
+			return empty, LogNotAuthorizedError(ctx, logger, err)
+		}
+
+		return object, nil
+	}
+}
+
+// fetchAndExec uses fetchAndQuery but only returns the error. The naming comes
+// from SQL 'exec' functions which only return an error.
+// See fetchAndQuery for more information.
 func fetchAndExec[ObjectType rbac.Objecter,
 	ArgumentType any,
 	Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error),
@@ -152,6 +192,10 @@ func fetchAndExec[ObjectType rbac.Objecter,
 	}
 }
 
+// fetchAndQuery is a generic function that wraps a database fetch and query.
+// The fetch is used to know which rbac object the action should be asserted on
+// **before** the query runs. The returns from the fetch are only used to
+// assert rbac. The final return of this function comes from the Query function.
 func fetchAndQuery[ObjectType rbac.Objecter, ArgumentType any,
 	Fetch func(ctx context.Context, arg ArgumentType) (ObjectType, error),
 	Query func(ctx context.Context, arg ArgumentType) (ObjectType, error)](
@@ -184,42 +228,6 @@ func fetchAndQuery[ObjectType rbac.Objecter, ArgumentType any,
 	}
 }
 
-// fetch is a generic function that wraps a database
-// query function (returns an object and an error) with authorization. The
-// returned function has the same arguments as the database function.
-//
-// The database query function will **ALWAYS** hit the database, even if the
-// user cannot read the resource. This is because the resource details are
-// required to run a proper authorization check.
-func fetch[ArgumentType any, ObjectType rbac.Objecter,
-	DatabaseFunc func(ctx context.Context, arg ArgumentType) (ObjectType, error)](
-	// Arguments
-	logger slog.Logger,
-	authorizer rbac.Authorizer,
-	f DatabaseFunc) DatabaseFunc {
-	return func(ctx context.Context, arg ArgumentType) (empty ObjectType, err error) {
-		// Fetch the rbac subject
-		act, ok := ActorFromContext(ctx)
-		if !ok {
-			return empty, NoActorError
-		}
-
-		// Fetch the database object
-		object, err := f(ctx, arg)
-		if err != nil {
-			return empty, xerrors.Errorf("fetch object: %w", err)
-		}
-
-		// Authorize the action
-		err = authorizer.Authorize(ctx, act, rbac.ActionRead, object.RBACObject())
-		if err != nil {
-			return empty, LogNotAuthorizedError(ctx, logger, err)
-		}
-
-		return object, nil
-	}
-}
-
 // fetchWithPostFilter is like fetch, but works with lists of objects.
 // SQL filters are much more optimal.
 func fetchWithPostFilter[ArgumentType any, ObjectType rbac.Objecter,