[JBIDE-21590] Wait for server to reconcile project delete

[jcantrill]

Provides a wait for the server to reconcile the project deletion.

cc @mlabuda @fbricon @adietish

[adietish]

the Runnable "callIfNotOK" looks abit redundant/duplicate to what IJobChangeListener offers: The one that creates the delete job could attach a job change listener to it and implement #done, inspecting the IStatus that's in the IJobChangeEvent he'll get.

[adietish]

other than my comments, code seems legit. It even handles multiple invocation from context menu (adapter is set to 'deleting' while it's deleting) and adds tests. Nice!

[jcantrill]

@adietish made an update but I see THIS event sometimes doesnt work based on the new logic. I'm thinking it would actually be better to have a blocking progress modal that times out after a max time. I noticed another situation the disabled delete doesnt address where i was able to 'expand' a deleted project which threw a different exception. WDYT about making delete project 'blocking' from the context menu?

[adietish]

@jcantrill Do you know why exactly it doesnt work? Frequently this happens if the listeners gets attached after the job was launched and teminates. But I dont see this being true for your code. Maybe you should add brk-points to the other methods (#sleeping, #scheduled, #awake etc.) to further understand when this wont get into #done.
I believe that the job should terminate itself for a given timeout, at least that seems legit to me in terms of separations of concerns (the thread terminates itself vs gets terminated) - this is imho also the logic in jdk with the deprecation of Thread#stop.
For the case where throwing a different exception was not preventing a concurrent deleting thread I'd think that we then should maybe catch all exceptions?
Blocking seems like a bad idea - but maybe I get you wrong - since this would also block the UI thread, something we want to avoid.
Maybe we should discuss this in more details in bj?

[jcantrill]

Updated to reduce contention points of using sync blocks.

[adietish]

i see a possible race-condition where 2 threads concurrently query the cache and create entries here since only #getCacheFor is locking the cache but the latter #createEntry isnt.

[adietish]

@jcantrill looks good now since querying cache and creating a new entry is atomic. Will test tomorrow

[adietish]

testing this I see the DeleteResourceJob enlessly looping.
When breaking in I see ex.getStatus() being null it thus wont stop waiting based on the 404 or 403. Furthermore the sleep counter would be increased after Thread.sleep(), but the exception occurrs right after conn.getResource(project), skipping the sleep and sleepcounter increase. The thread will thus loop endlessly given it will never reach the MAX_PROJECT_DELETE_DELAY value.

"ex" is com.openshift.restclient.authorization.ResourceForbiddenException, the message is 'User "adietish@redhat.com" cannot get projects in project "testproj"', status is null.

[adietish]

The actual reason for the missing status is an issue in the client lib:

DefaultClient#createOpenShiftException()

            if(status.getCode() == STATUS_FORBIDDEN) {
                if(StringUtils.isNotBlank(token)) { //truly forbidden
                    return new ResourceForbiddenException(status.getMessage(), e);

The resourceForbiddenException that is being constructed simply wont add the status, it only adds the message.

[jcantrill]

This openshift/openshift-restclient-java@62cadab fixed that problem. I will update to add a check against null status to be safe.

[jcantrill]

There is a different issue in the total sleep never is advanced because of the exception so you never reach the max value. I will fix here.

[adietish]

Shouldnt the condition to stop retrying be:

!deleted && sleep < MAX_PROJECT_DELETE_DELAY

?

[jcantrill]

merged.

[adietish]

tested it, it works for me. +1