Call refreshScope() once when saving all contexts

[double16]

When updating context information, ZAP calls saveAllContexts(). Each context save results in a call to refreshScope(). This is unnecessary. This PR calls refreshScope() after all contexts are saved.

In addition, a performance hit compiling a regex repeatedly was found and fixed.

[thc202]

Be good to add tests.

[thc202]

In addition, a performance hit compiling a regex repeatedly was found and fixed.

Do you have numbers? (Just curiosity.)

[double16]

In addition, a performance hit compiling a regex repeatedly was found and fixed.

Do you have numbers? (Just curiosity.)

Nothing big. I think I read the wrong number, but still good practice to pre-compile.

[double16]

Be good to add tests.

refreshScope() is only called when the view is initialized. I didn't see anything that would mock that in SessionUnitTest. Ideas?

[thc202]

Nothing big. I think I read the wrong number, but still good practice to pre-compile.

Thank you. Agreed.

refreshScope() is only called when the view is initialized. I didn't see anything that would mock that in SessionUnitTest. Ideas?

You can mock the View, though given the EDT is in use probably something to go under src/testGui/.

[double16]

It looks like the only way to verify refreshScope() is called is to check for side effects in the site tree unless the method is made package-private. How do you feel about that?

[double16]

It looks like the only way to verify refreshScope() is called is to check for side effects in the site tree unless the method is made package-private. How do you feel about that?

Making refreshScope() package private would make adding the tests easier. Adding tests into testGui/ looks like a significant copy and paste of setup code from test/ to testGui/ . I don't want to do that work if it's not desired. Can someone comment on direction?

[double16]

recheck

[kingthorin]

Is this old enough that it needs rebase to properly pickup CLA?

[thc202]

No.

[thc202]

#8205 (comment)

Exposing the method would not make much difference as we can't still tell if it's called or not, we should check that through sessionScopeChanged instead.

Regarding the duplication, that should not be needed as testGui should have visibility into test (if not that should be changed rather than duplicate).

[thc202]

Should have a ZAP comment after the license header (same for Session class).

[thc202]

PATTERN_NEWLINES

[kingthorin]

As well as or instead of multi?

[thc202]

That's for the name of the constant.

[double16]

Regarding the duplication, that should not be needed as testGui should have visibility into test (if not that should be changed rather than duplicate).

As far as I can tell testGui does not have visibility into test. IDEA doesn't recognize testGui as a source folder either, but the testGui task is recognized. I can't figure out how to get testGui to depend on the classpath of test. I've tried:

configurations {
    testGuiImplementation {
        extendsFrom(testImplementation.get())
    }
    testGuiRuntimeOnly {
        extendsFrom(testRuntimeOnly.get())
    }
}

but it doesn't help. If you know, could you post the gradle code?