-
Notifications
You must be signed in to change notification settings - Fork 74
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Elasticsearch TicketCreate event handler doesn't check if Ticket still exists #1192
Comments
Okay, a workaround for my issue is calling |
Hi Sterni, sorry for the late reply. We still have some people in vacation, so not totally necessary things are postponed a bit. I just skimmed through things quickly, two things, though - first, we do not have Elasticsearch incorporated into unit tests and just deactivate it for all old and existing tests normally, so some things won't work here. This is definitely work to be done, though. The assumption with tickets existing is usually valid, also, and it directly being deleted is more of a unit test problem - the check should be done, nonetheless, I agree. Furthermore even if a ticket is existing in the database it is not guaranteed, that it exists in the Elasticsearch index. To really solve this, I would want to develop error handling for the invokers, to have the possibility to appropriately react to the elastic search responses. Sven |
This bad effect wasn't recognised before, because the unit tests run without ElasticSearch support per default. It should be made easy to set up an environment with ES. Therefore I propose to add a flag |
That is probably true. in my case, I created and deleted the ticket in the same unit test which caused the issue — something probably unlikely to happen in normal operation. Still an avoidable crash. Also as a correction: The database rollback doesn't cause such a problem (most likely because the events also get rolled back?). In my case the key was that the ticket is created and deleted, but the events for both processed only after all of that happened, causing the crash. The fix for that is to call |
The target queue was already checked above.
Because it can be considered as its own special case.
Use %QueueIsExcluded for the actual lookup.
This is the fallback when the article in no longer found.
Added checks whether the required ticket, article, or attachments still exist when the events are handled. Running the complete test suite had some performance problems when it was running with ES enabled. But the last run, with all the checks enabled, looked fine. It could be that memory was exhausted on my devel machine. @svenoe : Could you take a look at the PR? |
…ket_still_exist Issue #1192 check whether ticket still exist
Run the unit tests again, and the result looks fine. Merged the PR. Closing later, when no problems crop up. |
Did another test run, this time with Docker images taken from hub.docker.com. Still looking good. Closing this issue. |
I have an unit test where I am among other things are doing something like this:
AdminUser
andCustomerUser
where created usingTestUserCreate
andTestCustomerUserCreate
, respectively.$TicketID
is notundef
and creation seems to work fine. However, when we reach the call to$Self->DoneTesting();
at the end, the following happens:I'm using database rollbacks in the test case, so it may be that the events are processed after the rollback (for some reason?!) and failing of course then.
The text was updated successfully, but these errors were encountered: