Add configuration to allow deferring the initial Deployment until after Server is started

[joakime]

Introduce a WebAppProvider (and ScanningAppProvider) configuration .setDeferInitialScan(boolean) (defaults to false), which controls the initial deployment of webapps.

• true = to have initial scan deferred until the Server component has reached it's STARTED state.
• false = (default) to have initial scan occur as normal on ScanningAppProvider startup.

Notes:
Having true set will not fail the Server startup.
Having false be default allows for failed deployments to also fail the server start.

Fixes #10669

[joakime]

This needs unit tests still.

[gregw]

I think this approach achieve the goal.... but it might be simpler to make a change to the scanner itself. Specifically reportExisting is not sufficient, I think the doStart of the scanner could be modified to look like:

        if (_scanInStart)
        {
            if (_reportExisting)
            {
                // if files exist at startup, report them
                scan();
                scan(); // scan twice so files reported as stable
            }
            else
            {
                //just register the list of existing files and only report changes
                _prevScan = scanFiles();
            }
        }

That way you could add the scanner normally in the AppProvider and the listener would only need to do a nudge.

[gregw]

Perhaps this approach is best for this release cycle and we can consider improving the implementation by changing the scanner in the next release cycle. @janbartel thoughts?

[joakime]

@gregw doh! I already made the change to the Scanner in this branch / PR.
I'm comfortable with delaying this until 10.0.18 (note: 10.0.17 is the release occuring this next week).
But, i'm only comfortable with that change if we can get a quick release of 10.0.18 after this PR has been vetted more.

[gregw]

LGTM

[janbartel]

Some name changes to better indicate what is going on: we're not deferring the initial scan, we're deferring starting the whole scan cycle. Also need to remove extra call (currently to startup()) in doStart().

[joakime]

Some name changes to better indicate what is going on: we're not deferring the initial scan, we're deferring starting the whole scan cycle. Also need to remove extra call (currently to startup()) in doStart().

We are deferring the initial scan.
The startup() method performs the initial scan AND optionally schedules a scan task (on the scheduler, but only if the scan interval makes sense).

The activateScan() naming is too vague, esp considering we already have a .scan() method.

We use the term "initial scan" in other javadoc currently (like setReportExistingFilesOnStartup), so using it for new methods and techniques doesn't seem off or unusual.

[joakime]

@janbartel also, something to think about, what would you call the ScanningAppProvider.setDeferInitialScan(boolean) method in this new naming?
Or would you leave that one alone?

[joakime]

@janbartel I committed a pass at better naming.
Most of the changes are in Scanner, but the new naming in ScanningAppProvider needs a review too.

[janbartel]

I don't think I really love any of the possible naming changes, even ones I suggested :), so happy to go with the majority decision there. But the call to manually start the scheduler is something that I really want removed.

[joakime]

@gregw @janbartel The manual scheduler start is removed in this version.

[gregw]

Conditional approval. You need to fix a few javadoc comments.

[janbartel]

Couple of javadoc fixes, and it would be good if the test could be more specific that the webapp is not actually deployed at the point in time the DeploymentManager/Scanner starts.

[joakime]

@janbartel @gregw I went ahead and applied all of the javadoc changes requested, along with improvements on testDelayedDeploy() to ensure the order of events is sane. (started components and initial scans)

[chadlwilson]

Oops, wrong missing <scope>test</scope>!

Submitted a PR to correct at #10813