OpenMRS Core
  1. OpenMRS Core
  2. TRUNK-3384

Module upgrade doesn't restart modules in the right order

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Should Should
    • Resolution: Fixed
    • Affects Version/s: OpenMRS 1.9.0
    • Fix Version/s: OpenMRS 1.9.1
    • Component/s: None
    • Labels:
      None
    • Complexity:
      Medium

      Description

      I have the following set of modules:

      • UI Framework
      • UI Library (depends on UI Framework)
      • App Framework (depends UI Framework and UI Library)
      • Kenya EMR (depends on UI Framework, UI Library, App Framework, Metadata Sharing, HTML Form Entry)

      Everything was running fine, and I uploaded a new version of App Framework. This worked fine.

      Then I uploaded a new version of UI Framework, and I got the errors that:

      • Kenya EMR failed to start because: Not all required modules are started: org.openmrs.module.uiframework 1.2, org.openmrs.module.htmlformentry 1.9.1, org.openmrs.module.appframework 1.0, org.openmrs.module.uilibrary 1.1, org.openmrs.module.metadatasharing 1.0.7. Module: Kenya OpenMRS EMR Module
      • App Framework failed to start because: Not all required modules are started: org.openmrs.module.uiframework, org.openmrs.module.uilibrary. Module: App Framework Module

      However UI Framework and UI Library are in fact started.

      Perhaps we have a bug in our code that determines module startup order that only becomes an issue when we have multiple layers of dependency?

        Gliffy Diagrams

          Activity

          Hide
          Darius Jazayeri added a comment -

          This is consistent and reproducible. Every time I upload a new version of UI Framework, I end up with App Framework and Kenya EMR failing to start. (And I have to start them manually.)

          Show
          Darius Jazayeri added a comment - This is consistent and reproducible. Every time I upload a new version of UI Framework, I end up with App Framework and Kenya EMR failing to start. (And I have to start them manually.)
          Hide
          Darius Jazayeri added a comment -

          Debugging notes:

          ModuleListController line 133 does:
          dependentModulesStopped = ModuleFactory.stopModule(existingModule, false, true); // stop the module with these parameters so that mandatory modules can be upgraded

          After this, though, dependentModulesStopped is [appframework, kenyaemr, kenyaemr, uilibrary]. It seems wrong that there's a duplicate. (It's a Vector, and the two kenyaemr items are the same Module object, per the debugger.)

          Actually, the problem is around line 179 of ModuleListController, because it tries to restart the modules in their order in that Vector, without checking for dependencies. In this case it tries to start appframework first (fails, because that also requires uilibrary), then tries to start kenyaemr (fails also), and finally succeeds at starting uilibrary. This is exactly the consistent behavior I've been seeing.

          Show
          Darius Jazayeri added a comment - Debugging notes: ModuleListController line 133 does: dependentModulesStopped = ModuleFactory.stopModule(existingModule, false, true); // stop the module with these parameters so that mandatory modules can be upgraded After this, though, dependentModulesStopped is [appframework, kenyaemr, kenyaemr, uilibrary] . It seems wrong that there's a duplicate. (It's a Vector, and the two kenyaemr items are the same Module object, per the debugger.) Actually, the problem is around line 179 of ModuleListController, because it tries to restart the modules in their order in that Vector, without checking for dependencies. In this case it tries to start appframework first (fails, because that also requires uilibrary), then tries to start kenyaemr (fails also), and finally succeeds at starting uilibrary. This is exactly the consistent behavior I've been seeing.
          Hide
          Darius Jazayeri added a comment -

          Committed to trunk in rev:27679

          Show
          Darius Jazayeri added a comment - Committed to trunk in rev:27679
          Hide
          Darius Jazayeri added a comment -

          @Ben, can I get a quick review so I can backport to 1.9.x and save myself a lot of annoyance while doing development of a distro with lots of module dependencies?

          Show
          Darius Jazayeri added a comment - @Ben, can I get a quick review so I can backport to 1.9.x and save myself a lot of annoyance while doing development of a distro with lots of module dependencies?
          Hide
          Ben Wolfe added a comment -

          Yep, looks fine. Add a license header to your unit test, but code looks fine.

          Show
          Ben Wolfe added a comment - Yep, looks fine. Add a license header to your unit test, but code looks fine.
          Hide
          Darius Jazayeri added a comment -

          Added license header in rev:27682.

          Backported to 1.9.x in rev:27684.

          Show
          Darius Jazayeri added a comment - Added license header in rev:27682. Backported to 1.9.x in rev:27684.
          Hide
          Darius Jazayeri added a comment -

          I have only fixed this in trunk and 1.9.x. If someone needs it backported to an earlier version, they should comment here.

          Show
          Darius Jazayeri added a comment - I have only fixed this in trunk and 1.9.x. If someone needs it backported to an earlier version, they should comment here.

            People

            • Assignee:
              Darius Jazayeri
              Reporter:
              Darius Jazayeri
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development