1
\$\begingroup\$

I'm trying to give one Controller one operation. However, some Controllers exists of 2 or more operations. I'm not sure whats the best way to structure this part of the app.

For example, I have 2 Controller Functions, each doing their independent thing
1. attemptGetUserById (checks input, calls database provider, returns user or error)
2. attemptVerifyToken (checks input, tries to verify token, returns valid/invalid)

Then, there is another Controller "attemptCheckUser" which is essentially calling
--> attemptVerifyToken --> if token valid --> attemptGetUserById

This Controller is called via a route.

Now, I have 3 variants which might be acceptable.

Variant1
This variant is the one described above. When the API endpoint is accessed, it will call attemptCheckUser, which is calling the 2 other controllers. No logic in Endpoint.

// routes.ts
{
    path: '/api/core/users/check',
    method: 'post',
    handler: [
      async ({ headers: { authorization } }: Request, res: Response, next: NextFunction) => {
        const { error, user } = await attemptCheckUser(authorization)
        if (error) return next(error);
        return res.status(200).send(user);
      },
    ],
  },
  ...

// auth.controller.ts
export const attemptVerifyToken = (token: string) => {
  if (token) {
    const secret = "xxx";
    // only provider - no logic in verifyToken
    const { error, userId } = verifyToken(token, secret); 
    if(error) return { error };
    return { userId, token };
  }
  return { error: new HTTP400Error() };
};


// user.controller.ts
export const attemptGetUserById = async (_id: string) => {
  if (_id) {
    try {
      // only provider - no logic in getFullUserById only db operation
      const user = await getUserById(_id);
      if (user) return { user };
      return { error: new HTTP404Error() };
    } catch (e) {
      return { error: new HTTP400Error(e) };
    }
  }
  return { error: new HTTP400Error() };
};

export const attemptCheckUser = async (token: string) => {
  const { error, userId} = attemptVerifyToken(token);
  if (error) return { error };

  if (userId) {
    const { error, user } = await attemptGetUserById(userId);
    if (error) return { error };
    return { user };
  }
  return { error: new HTTP400Error() };
}

Variant2
Instead of having attemptCheckUser, move logic into Endpoint/Route so each Controller remains to have only one operation. attemptVerifyToken and attemptGetUserById remain the same as in Variant1

// routes.ts
{
  path: '/api/core/users/check',
  method: 'post',
  handler: [
    async ({ headers: { authorization } }: Request, res: Response, next: NextFunction) => {
      const { error, userId } = attemptVerifyToken(authorization);
      if (error) return next(error);
      if (userId) {
        const { error, user } = await attemptGetUserById(userId);
        if (error) return next(error);
        return res.status(200).send(user);
      }
      return next();
    },
  ],
},
...

// auth.controller.ts
export const attemptVerifyToken = (token: string) => {
  if (token) {
    const secret = "xxx";
    // only provider - no logic in verifyToken
    const { error, userId } = verifyToken(token, secret); 
    if(error) return { error };
    return { userId, token };
  }
  return { error: new HTTP400Error() };
};


// user.controller.ts
export const attemptGetUserById = async (_id: string) => {
  if (_id) {
    try {
      // only provider - no logic in getFullUserById only db operation
      const user = await getUserById(_id);
      if (user) return { user };
      return { error: new HTTP404Error() };
    } catch (e) {
      return { error: new HTTP400Error(e) };
    }
  }
  return { error: new HTTP400Error() };
};

Variant3
This variant is nearly identical to Variant1, however, the route injects the dependencies (the other 2 controllers) into attemptCheckUser. This Controller gets a new Interface.

// routes.ts
{
  path: '/api/core/users/check',
  method: 'post',
  handler: [
    async ({ headers: { authorization } }: Request, res: Response, next: NextFunction) => {
      const { error, user } = await attemptCheckUser(
        {
          attemptVerifyToken,
          attemptGetUserById,
        },
        authorization,
      );
      if (error) return next(error);
      return res.status(200).send(user);
    },
  ],
},
...

// auth.controller.ts
export const attemptVerifyToken = (token: string) => {
  if (token) {
    const secret = "xxx";
    // only provider - no logic in verifyToken
    const { error, userId } = verifyToken(token, secret); 
    if(error) return { error };
    return { userId, token };
  }
  return { error: new HTTP400Error() };
};


// user.controller.ts
export const attemptGetUserById = async (_id: string) => {
  if (_id) {
    try {
      // only provider - no logic in getFullUserById only db operation
      const user = await getUserById(_id);
      if (user) return { user };
      return { error: new HTTP404Error() };
    } catch (e) {
      return { error: new HTTP400Error(e) };
    }
  }
  return { error: new HTTP400Error() };
};



interface OptionsCheckUser {
 attemptVerifyToken: (token: string, skipToken?: boolean) => any;
 attemptGetFullUserById: (userId: string, organisationId: string) => any;
}

export const attemptCheckUser = async (dependencies: OptionsCheckUser, token: string) => {
  const { error, userId} = dependencies.attemptVerifyToken(token);
  if (error) return { error };

  if (userId) {
    const { error, user } = await dependencies.attemptGetUserById(userId);
    if (error) return { error };
    return { user };
  }
  return { error: new HTTP400Error() };

I'm creating this question here on codereview, because I came up with the idea of Variant 3. I wanted to decouple those controllers calling other controllers, however, it just doesn't feel right. The reason I feel unsure about is that the interface needs to describe the injected functions quite precisely, which means it's likely there are no other functions I can inject since those functions would theoretically be doing the same job, so they simply don't exist (as they would be duplicates).

Note: Please consider that this is a simplified example. The app will have multiple services, and one Controller will need to call other Controller Functions of a different service. That's why I found I need some kind of decoupling of those Controllers and how they call other Controller functions inside.

What variant would you choose (if any) and why?

Thanks a lot in advance!

\$\endgroup\$

0

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.