Community
Participate
Working Groups
We need to be able to create a composite repository which isn't local. Currently the constructor code tries to save the repo one it is created. This will fail because the repo is not modifiable. Currently the code also will not allow for the addition of a non-local child. Note that the code is the same for artifact and metadata repos.
Jeff I'll move this bug to me but if you want it, then feel free to grab it.
Thanks for opening this DJ. I was thinking that we could address the first problem by simply checking isModifiable() before calling save().
Yeah I was going to do that and then I was wondering if we just wanted to change #save method so it didn't throw the assertion failed exception... I mean if all the callers are going to do the check then why not change the lower code. Basically: private void save() { if (!isModifiable()) return; ... } I find that I am getting conflicting feelings in the way we should go on this. If you are creating Composite Repositories in scripts or via Ant or as part of a build or mirroring process, then I would argue that you would want to fail if the user tries to save an unmodifiable repository. But on the other hand if I'm just spoofing one up in memory and plan to throw it away later, then I shouldn't be required to persist it. Adding John to CC in case he has some thoughts as he worked with Andrew on this a bit.
I see your points. The quirkiness comes from the case where you have a persistent repo that is NOT modifiable. If someone calls save() they should likely be spanked in the same way as someone calling a write method on a read only file. For in memory repos (a real usecase) what you do in save (or whether save is even called) is likely irrelevant. So I would make the save()s private and put the isModifiable() test in the callers.
Sorry for the delay in this Jeff but I read your comment a bunch of times and am still a little confused. Unless you meant to say: "So I would make the save()s public and put the isModifiable() test in the callers." This would make it so that we can add things to a repo in memory but if someone explicitly tries to save a non-modifiable repo, then it would throw an exception.
Created attachment 123010 [details] patch Here is a patch with a public save method and all the local callers checking first to see if the repository is modifiable... in case we're thinking the same thing?
I really did mean that the save()s should be private. No one calls them externally. save() is not part of the repository API and is only called by internal lifecycle methods.
I think my confusion comes when you say that people calling #save should be spanked in the case where they have a non-local repo. So I have removed the spanking (e.g. call to #assertModifiable) in #save and changed it to only do work if the repo is modifiable. (we've spent way too much time on this bug which I thought was a simple fix :-) I figure if the method is private and all callers are calling #isModifiable then we might as well put the check in #save itself. I released the change but if that's not what you're thinking then please make changes and re-release.
