Uploaded image for project: 'folio-spring-base'
  1. folio-spring-base
  2. FOLSPRINGB-95

non-public beginFolioExecutionContext avoids wrong tenant/user

    XMLWordPrintable

Details

    • Spring Force
    • Poppy (R2 2023)
    • Implementation coding issue
    • Orchid (R1 2023), Nolana (R3 2022)

    Description

      A thread runs with a wrong tenant and wrong user when

      • a previous use of the thread sets the tenant and user
      • the previous use of the thread doesn't clear tenant and user
      • the current use of the thread doesn't set tenant and user.

      beginFolioExecutionContext sets tenant and user.
      endFolioExecutionContext clears tenant and user.

      See the code in FolioExecutionScopeExecutionContextManager:
      https://github.com/folio-org/folio-spring-base/blob/v6.0.1/src/main/java/org/folio/spring/scope/FolioExecutionScopeExecutionContextManager.java#L47-L69

      Two options are available that enforce that beginFolioExecutionContext is called at the begin of some task and endFolioExecutionContext is always called afterwards:

      https://github.com/folio-org/folio-spring-base/blob/v6.0.1/src/main/java/org/folio/spring/scope/FolioExecutionScopeExecutionContextManager.java#L71-L93

      import static org.folio.spring.scope.FolioExecutionScopeExecutionContextManager.getRunnableWithCurrentFolioContext;
      
      executor.execute(getRunnableWithCurrentFolioContext(() -> /* some stuff */));
      

      https://github.com/folio-org/folio-spring-base/blob/v6.0.1/src/main/java/org/folio/spring/scope/FolioExecutionContextSetter.java

      try (var x = new FolioExecutionContextSetter(currentFolioExecutionContext)) {
        // some stuff
      }
      

      Manually calling beginFolioExecutionContext and endFolioExecutionContext is error-prone because one or both of them can be forgotten. If forgotten the unit tests doesn't catch this. See two only recently fixed examples: MODEXPW-375, FOLSPRINGB-86.

      To end the error-prone usage these two error-prone methods should become package-private.

      Task:

      • Make FolioExecutionScopeExecutionContextManager.beginFolioExecutionContext package-private.

      (FolioExecutionScopeExecutionContextManager.endFolioExecutionContext can remain public to be available for cleaning the context in unit tests.)

      This forces a review of all code that use it.
      Most usages should switch to one of the secure methods shown above (runnable decorator, try-with-resources). A few remaining usages may use endFolioExecutionContext without try-with-resources. This triggers a sonar warning that the closable is not closed. And this code should undergo regular code review (for example once per flower release).

      TestRail: Results

        Attachments

          Issue Links

            Activity

              People

                Taras_Spashchenko Taras Spashchenko
                julianladisch Julian Ladisch
                Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                  Created:
                  Updated:
                  Resolved:

                  TestRail: Runs

                    TestRail: Cases