Key cache lookup by project id and unique id #240
Conversation
To avoid duplicates when projects are copied.
| /** | ||
| * A unique key, within this project. | ||
| * If a project is copied, then this key may be duplicated. | ||
| * Use <code>getCacheKey</code> as a globally unique id for this webhook |
There was a problem hiding this comment.
The fact that I wrote this comment made me wonder if we're too fixated on the current use case.
Should this be getGloballyUniqueKey instead? (or something similar)
There was a problem hiding this comment.
I think if it's just contained within that class and used only as a key for the cache then the name is ok.
I am working through getting the tests running in the API, so haven't had a chance to build it yet.
I found another few places where we need to update references to the key. I'll push my branch once I get it working.
|
I thought there might be some other bit that needed work. I'll take a look later in the week.
|
|
BTW, this should work (I am using Java 8). |
|
From what I can tell - these are to do with test setup. I didn't get very far with debugging though. |
ah, ok. I'll pull the branch and run it up locally. Not sure when yet. |
|
Just pushed a change - I think it should pass the tests. (When I run it locally, I get a bunch of test failures similar to |
|
Yep, that's fixed all the core tests. Just the REST api ones failing now. But don't spend time on that, as I am in the middle of migrating all that code to rest-api-legacy project. |
|
I'm unblocked now - I manually modified the id's in the config, and after restarting teamcity, all the webhooks are showing up in the right place. Now, it's only future project copies that I'm thinking about, but not expecting that to happen too soon, so no rush. Good luck with the legacy fun :) |
|
I'm really sorry. I didn't realise you were blocked by this. D'oh! |
|
No stress - I was only "blocked" until I found how the code worked, and even then, it wasn't a biggie - if push came to shove, I could've edited the config directly. |
|
I thought I had implemented a partial work around where a "save" of the webhook forces a reload on that project's webhooks, but I still have the code here and it never made it into a release. This would have allowed you to edit plugin-settings and then create a new webhook would would've fixed the ids in the cache map. But now we know the actual reason for that issue, so that's a win :-) |
| @@ -53,6 +53,10 @@ public void setup() throws JDOMException, IOException { | |||
|
|
|||
| projectSettings = new WebHookProjectSettings(); | |||
| projectSettings.readFrom(ConfigLoaderUtil.getFullConfigElement(new File("src/test/resources/project-settings-test-elastic.xml")).getChild("webhooks")); | |||
| projectSettings.getWebHooksConfigs().forEach(c -> { | |||
There was a problem hiding this comment.
There is a murky issue with old versions of ASM (which is pulled in by old versions of Jersey) when it comes to Java8 features.
I needed to convert this code to a Java 7 for loop, otherwise the code compiles with a byte code signature of Java8 and ASM can't load it.
I really need to convert to newer ASM lib since tcWebHooks 2.0.0 is now released and I no longer support TeamCity 9 (which ran on Java7).
tcWebHooks 2.0.0 supports 2019.1 and newer only.
|
I have it compiled and running. Copying a project works, but does not refresh the cache. I'll look further into the copy event that Pavel mentioned, and look to do a regenerate of the UniqueId on Project copy. |
To avoid duplicates when projects are copied.
Fixes #238.
Note: I haven't tested this, as I'm struggling to compile locally. (I'm also not a java developer, so there's a lot of things that I dont know that I dont know 馃槃).