Route-Exclusive Custom Constraint Strategy

[joelnet]

First of all, cheers 🍻 to all the maintainers of this amazing project. Thank you for all your hard work!

The documentation states:

Constraint strategies should be careful to make the deriveConstraint function performant as it is run for every request matched by the router.

I am seeking a way to limit the impact of the deriveConstraint function to only the routes in need of the constraint. This has been previously discussed #165.

The goals:

1. Constraints should not be run on non-constrained routes.
2. If a constraint fails, router should fallback to next matching route.

I did create a hackish solution that involves keeping a second router as an allow list. It works, but I am not completely in love with this.

The solution relies on some Fastify methods, so example isn't limited to find-my-way.

// @ts-check
import Fastify from "fastify";
import fp from "fastify-plugin";
import FindMyWay from "find-my-way";

const port = 8888;
const host = "0.0.0.0";

/**
 * @param {import('fastify').FastifyInstance} fastify
 */
const featurePlugin = fp(async (fastify) => {
  async function expensiveAsyncCall() {
    console.log("!!! expensiveAsyncCall !!!");
    return true;
  }

  const routesAllow = FindMyWay();

  /** @type {any} */
  const featureConstraint = {
    // strategy name for referencing in the route handler `constraints` options
    name: "feature",
    // storage factory for storing routes in the find-my-way route tree
    storage() {
      let state = {};
      return {
        get: (key) => {
          return state[key] || null;
        },
        set: (key, store) => {
          state[key] = store;
        },
      };
    },
    // function to get the value of the constraint from each incoming request
    deriveConstraint(req, ctx, done) {
      const route = routesAllow.find(req.method, req.url);
      if (!route) return done(null, null);

      expensiveAsyncCall().then(
        (data) => done(null, data),
        (err) => done(err, null)
      );
    },
  };

  fastify.addHook("onRoute", (route) => {
    if (route.constraints && featureConstraint.name in route.constraints) {
      // Create Mock Route for "find" in deriveConstraint.
      routesAllow.on(route.method, route.path, () => {});
    }
  });

  fastify.addConstraintStrategy(featureConstraint);
});

async function main() {
  const fastify = Fastify();
  await fastify.register(featurePlugin);

  // Constraint should NOT be run here
  fastify.get("/always", () => {
    return { route: "/always" };
  });

  fastify.get("/sometimes", { constraints: { feature: true } }, () => {
    return { route: "/sometimes" };
  });

  // Constraint should NOT be run here
  fastify.get("*", () => {
    return { route: "*" };
  });

  await fastify.listen({ port, host });

  console.log(`Listening on http://${host}:${port}/`);
  console.log(`             http://${host}:${port}/always`);
  console.log(`             http://${host}:${port}/sometimes`);
  console.log(`             http://${host}:${port}/catchall`);
}

main().catch((err) => {
  console.error(err);
  process.exit(1);
});

The above code has a function expensiveAsyncCall that is only run for the /sometimes route.

if feature is set to false or expensiveAsyncCall returns false, the route will fallback to the "*" route.

I am not sure exactly what I am seeking here. Feedback? Direction? Help?

[ivan-tymoshenko]

Hi, thank you for your proposal. It makes much more sense since we added async constraints, and I think we should add it. Although it wouldn't be an easy PR for a couple of reasons:

1. Sync constraints should be as fast as they are.
2. We shouldn't call the same strategy a second time if the first route fails. We should cache these values.
3. find, and lookup methods should have the same interfaces and expected behavior unless we decide to release a new major for this feature.

It might take some time to add this.

[joelnet]

I briefly looked at the source and it does indeed look like a complicated change. The complexity is mostly due to the constraints being fully derived prior to call to find.

  /** code has been simplified **/
  this.constrainer.deriveConstraints(req, ctx, (err, constraints) => {
      const handle = this.find(req.method, req.url, constraints)
  })

It looks like find (and thus HandlerStorage) would have to be modified to support an asynchronous, on-the-fly lookup of constraints.

[SgtPooki]

From reading all the docs, it seemed like a constraint that failed validate would not apply to a route. Unfortunately, this is not the case and the whole server startup fails.

[ivan-tymoshenko]

From reading all the docs, it seemed like a constraint that failed validate would not apply to a route. Unfortunately, this is not the case and the whole server startup fails.

1. Can you point to the place in the docs where it's said.
2. It sounds like a completely different problem, so open a new issue.