Uploaded image for project: 'OpenMRS Core'
  1. OpenMRS Core
  2. TRUNK-4850

Module.getExtensions() does not call Module.expandExtensionNames() appropriately



    • Bug
    • Status: Closed
    • Could
    • Resolution: Fixed
    • None
    • Core 2.1.0
    • Module Engine
    • None


      In the current implementation of the Module class, the variable extensions can be set using setExtensions(List<Extension>) or it can be set by first setting extensionNames with setExtensionNames(IdentityHashMap<String, String>) then calling getExtensions() to fill extensions using expandExtntionNames().

      As of now, getExtensions() will only expand extensionNames if the size of extensions and extensionNames do not match. This will cause unintended behavior if they have the same size, but different contents. extensionNames could list different pointIds and className pairs than those that exist in extensions. In this case, extensionNames should be expanded even if it is the same length as extensions.

      Another issue with the current implementation is that if extensions starts at a non-zero length, every time getExtensions() is called it will expand extensionNames and add to extensions without replacing the current entries. In this case, extensions and extensionNames will never be the same size unless they are reset with setExtensions or setExtensionNames.

      The proposed solution is to replace comparing the sizes of the two variables with a function that will check the contents of the two variables. This way, getExtensions() will only expand extensionNames if extensionNames is not null, not empty, and if its pointId and className pairs do not match the objects in extensions. Additionally, when expandExtensionNames() is called, it will clear extensions before filling it. This way, all the entries in extensions are replaced instead of added to.

      These modifications and accompanying unit tests are in the following pull request: https://github.com/openmrs/openmrs-core/pull/1763

      Gliffy Diagrams




              ern2 Jessica Ern
              ern2 Jessica Ern
              0 Vote for this issue
              2 Start watching this issue