Webhooks not showing when project is copied #238
Comments
|
Hi @matt-richardson. How do you copy the projects? On the file system or via the UI (or REST API) or some other way?
Ah, that makes sense. I wondered how that could happen as it might also be tangentially related to #232
I really like the ID in the XML file as it helps so much with debugging issues. Also, the ID there is the one used in the REST API to identify the webhook config. I'll percolate on it. I'm always happy to hear your suggestions, as they are very well considered. |
We do it via the admin UI
|
|
Thanks. Was just wondering in case I needed to do some de-duplication on changes based on file reloading. |
|
If the ID in that map is the problem, then combining the key in that map with the projectId would solve it. But I just realised I could use the project internal ID as part of the key. class WebHookCacheKey {
private string projectInternalId;
private String webhookId;
}rather than string munging when finding a webhook in the cache by ID. |
|
That'd work pretty well 👍 |
|
I have a working prototype. https://github.com/tcplugins/tcWebHooks/tree/issue_238-webhooks_not_showing_when_project_copied |
|
More updates. Much better handling of delete, archive, copy, de-archive of a project. |
|
I think the only remaining thing is to go through and find duplicate IDs and rename them if they exist in two projects. |
|
Just gave it a spin: I'm running Update Ahh, just realised that that would be a case of "I should be using the Yep, that was it: |
|
Yeah, that check is just for the marketplace it seems. I have already mentioned that to Jetbrains. |
|
Okay, doing some testing, it seems to work fine in the UI, but oddly, the persisted file on disk still refers to the original id. |
Yeah, that's the last piece to fix. |
|
I realised in the middle of that night that keeping the key in the map with just the ID would work fine now. At the time that I write the webhook it into the map, I just need to do a Whereas now I need to iterate over every key and check if the unique id matches and project id doesn't. |
|
Arguably, it's not too much work on most systems. I ran a script and created thousands of webhooks to test the threading issues, so I probably have more webhooks than everyone else. |
|
I've removed that cache key, and added the re-keying logic. I'll kick off a build now. |
|
Sorry, forgot to update the version, so it's still 2.0.0-rc.1 |
|
Gave it a quick spin - seems to work well. I glanced through the code - happened to notice this code comment is no longer valid. I have a niggling feeling that the 10 second delay might cause a bit of havoc - it seems ripe for race conditions. We use versioned settings, so I wonder if that might cause things to take longer than 10 seconds? |
|
At the risk of pushing my luck... I happen to know about a semi-related bug... if a webhook is set to run for specific build configs, and then that build config is deleted, the "project edit" page wont load. Want me to raise a new issue for that? |
|
Is it related to this change, or did it exist previously? |
No, it existed previously |
Pavel tells me that projectRestored will fire when the VCS config is reloaded. |
|
I presume it still exists or you wouldn't have mentioned it. I'll look at it for this release. Is there any more detail I should know about how to reproduce it. If so please raise a new issue |
@Override
public void projectRestored(String projectId) {
Loggers.SERVER.debug("WebHookListener :: Handling projectRestored event for project: " + projectId);
this.webHookSettingsEventHandler.handleEvent(WebHookSettingsEventType.PROJECT_CHANGED, projectId);
}I might need to change this, as you say it will introduce 10 second delay. I think I'll add a new state to the ENUM, and only wait a second or so on this, as I have been told that it fires after the change is read back in. |
Just reconfirmed it - I listed out the repro steps in #242 to make it as easy to track down as I could. I suspect the fix could be related to this one - capturing an event when the build config is deleted and modifying the webhook there. |
Expected Behavior
When copying a project, the new project should show the webhooks correctly.
Current Behavior
The webhooks will only show in one of the projects (it's not deterministic which one)
Steps to Reproduce (for bugs)
Project1)Project2)Problem
We copy projects on a semi-regular basis, to support our old "LTS" branches.
The new webhook has the same id as the old webhook, meaning that when it ends up in this map, we drop webhook instances.
This is similar to #10 - we'd need to deal with modifying the IDs during copy, but it still doesn't look like there's a nice way of being notified when a copy happens.
I looked to see what would happen if we dropped the uniqueid altogether, and it seems about half of the usages are around logging, but there's still a lot of spots that require some thinking.
🤔 An option could be to prefix the unique id with the project id when dealing with it in code, so the duplicate id's in the config dont matter.
🤔 Another option could be to drop the id from the config, and generate a hash of the projectid+the entire webhook config for use as the id?
Workaround
We can modify the id's manually in the config so they are unique, but it's not ideal.
Your Environment
The text was updated successfully, but these errors were encountered: