findModuleMiddlewareAndRoutes in modules/@apostrophecsm/express/index.js incorrectly handles the "before" property

[chmdebeer]

The method findModuleMiddlewareAndRoutes in modules/@apostrophecsm/express/index.js incorrectly handles the "before" property of middleware, where the "before middleware" also has a "before" property.

To reproduce, create a middleware with a "before" set to "@apostrophecms/asset" (since asset also has a "before" property)
You will notice the final order is not expected.

A recursive sort is needed instead of the current array flattening.
My code to fix this is way too ugly for a pull request, but I will post it here inspiration.

async findModuleMiddlewareAndRoutes() {
  await self.emit('compileRoutes');
  const labeledList = [];
  const moduleNames = Array.from(new Set([ self.__meta.name, ...Object.keys(self.apos.modules) ]));
  for (const name of moduleNames) {
    const middleware = self.apos.modules[name].middleware || {};
    if (process.env.APOS_LOG_ALL_ROUTES) {
      for (const [ name, item ] of Object.entries(middleware)) {
        item.name = name;
      }
    }

    labeledList.push({
      name: `middleware:${name}`,
      middleware: Object.values(middleware).filter(middleware => !middleware.before)
    });
  }

  for (const name of Object.keys(self.apos.modules)) {
    const _routes = self.apos.modules[name]._routes;
    if (process.env.APOS_LOG_ALL_ROUTES) {
      for (const [ name, item ] of Object.entries(_routes)) {
        item.name = name;
      }
    }

    labeledList.push({
      name: `routes:${name}`,
      routes: _routes.filter(route => !route.before)
    });
  }

  for (const name of Object.keys(self.apos.modules)) {
    const middleware = self.apos.modules[name].middleware || {};
    const _routes = self.apos.modules[name]._routes;
    for (const item of [ ...Object.values(middleware), ..._routes ]) {
      if (process.env.APOS_LOG_ALL_ROUTES) {
        item.moduleName = name;
      }
      if (item.before) {
        let sortName = name;
        if ((!sortName.startsWith('routes:')) && (!sortName.startsWith('middleware:'))) {
          if (item.routes) {
            sortName = `routes:${sortName}`;
          } else {
            sortName = `middleware:${sortName}`;
          }
        }

        let fullBeforeName = item.before;
        if ((!item.before.startsWith('routes:')) && (!item.before.startsWith('middleware:'))) {
          if (item.routes) {
            fullBeforeName = `routes:${item.before}`;
          } else {
            fullBeforeName = `middleware:${item.before}`;
          }
        }
        labeledList.push({
          name: sortName,
          before: fullBeforeName,
          item: item
        });

      }
    }
  }

  let currentIndex = 0;
  let indexOfBefore = -1;
  while (currentIndex < labeledList.length) {
    if (labeledList[currentIndex].before) {
      indexOfBefore = labeledList.findIndex((element) => element.name == labeledList[currentIndex].before);
      if (indexOfBefore == -1) {
        throw new Error(`The module ${name} attempted to add middleware or routes "before" the module ${fullBeforeName.split(':')[1]}, which does not exist`);
      }
      if (indexOfBefore < currentIndex) {
        // moving currentIndex to indexOfBefore);
        labeledList.splice(indexOfBefore, 0, labeledList[currentIndex]);
        labeledList.splice(currentIndex + 1, 1);
        currentIndex = indexOfBefore;
      }
    }
    currentIndex++;

  }
  let sortList = [];
  for (const item of labeledList) {
    if (item.item) {
      sortList.push([item.item]);
    } else {
      if (item.middleware) { sortList.push(item.middleware); }
      if (item.routes) { sortList.push(item.routes); }
    }
  }

  self.finalModuleMiddlewareAndRoutes = sortList.flat();
}

[boutell]

Thanks for opening this ticket.