Bug 200774 - Detect cp name collision only within repository
Summary: Detect cp name collision only within repository
Status: CLOSED FIXED
Alias: None
Product: Data Tools
Classification: Tools
Component: Connectivity (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows XP
: P3 major (vote)
Target Milestone: 1.6M5   Edit
Assignee: Brian Fitzpatrick CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed
Depends on:
Blocks:
 
Reported: 2007-08-22 03:42 EDT by rob li CLA
Modified: 2008-07-01 17:40 EDT (History)
2 users (show)

See Also:


Attachments
mylyn/context/zip (951 bytes, application/octet-stream)
2007-08-29 00:24 EDT, rob li CLA
no flags Details
Proposed fix for unique path names (15.20 KB, patch)
2007-10-24 10:56 EDT, Brian Fitzpatrick CLA
no flags Details | Diff
patch (5.39 KB, patch)
2007-10-31 01:28 EDT, rob li CLA
no flags Details | Diff
Patch for path issues #2 (23.35 KB, patch)
2007-11-02 17:23 EDT, Brian Fitzpatrick CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description rob li CLA 2007-08-22 03:42:40 EDT
It's impossible to enforce cp name uniqueness across repositories. A simple scenario would be user1 connects to "repo" and creates "MyCP" in it and later, user2 connects to "repo" but already having a local cp also named "MyCP". Hence name collision detection should really be constrained within repositories.

I also observed inconsistency in Connectivity in dealing with this. For example in ImportProfileViewAction.run() we detect name collision across all repositories for importing to local repository: 
   if (manager.getProfileByName(profiles[i].getName() == null) 
but detect name collision in target repositories only for importing to non-local repository:
   if (repo.getProfileByName(profiles[i].getName()) == null)
Comment 1 Brian Fitzpatrick CLA 2007-08-22 11:06:22 EDT
So Rob, what are you proposing?

We can certainly go through and see if we can find all the places (New CP, Rename CP, Import CP) where we might possibly collide with profiles with the same names in other repositories.

However, that doesn't clear up the issue of what if a particular repository isn't connected? 

I'm not sure that we can go further than verifying that the New, Rename, and Import functionality works as best it can -- and then in Ganymede, we look at revamping how we decide "uniqueness" for profiles. I anticipate that repository location will be a big factor in determining uniqueness and if we add user-created categories as folders, it will be the "path" to a profile that makes it unique, not simply its name. I think we'll need to add "versioning" also or maybe even look into hooking into the team provider so users can check out and check in profiles...

For example... We might do something like...

Local Repository
   Databases
      Development
         CP1
      Production
         CP1
      Testing
         CP1

My Repository
   Databases
      Development
         CP1
      Production
         CP1
      Testing
         CP1
      Backup
         CP1

In all of these cases, the name is the same, but the location makes it unique. 
Comment 2 rob li CLA 2007-08-29 00:24:14 EDT
Created attachment 77203 [details]
mylyn/context/zip

ImportProfileViewAction
Comment 3 Brian Fitzpatrick CLA 2007-08-29 09:13:17 EDT
Rob, I don't exactly understand what the zip you attached does. 
Comment 4 Brian Fitzpatrick CLA 2007-08-29 09:55:17 EDT
We should look into solving the profile uniqueness problem for Ganymede.
Comment 5 rob li CLA 2007-08-30 02:14:18 EDT
Oops! Sorry, I created the attachment when playing with Mylyn. Please ignore it.
(In reply to comment #3)
> Rob, I don't exactly understand what the zip you attached does. 

Comment 6 Brian Fitzpatrick CLA 2007-10-24 10:56:51 EDT
Created attachment 81066 [details]
Proposed fix for unique path names

Could you review this patch? It fixes the uniqueness issue for new, import, and rename actions, as well as the property page when you change the profile name. It is repository aware. And the beginnings of category awareness are there, but they're not used at the moment since the Profile Manager isn't using custom categories. 

Let me know if this solves the case you were looking at for the LDAP servers, etc.

Thanks.
--Fitz
Comment 7 Brian Fitzpatrick CLA 2007-10-24 10:58:34 EDT
Rob Li, can you review this?
Comment 8 rob li CLA 2007-10-29 01:37:40 EDT
(In reply to comment #7)
> Rob Li, can you review this?

Brian,
sorry for the late reply. Was quite busy (daughter in hospital)last week...

I tested with your patch, but it seems that something is missing. I cannot create in ldap a new CP if its name collide with CP in local repository. Same thing happens when I do import. Could you double check? thanks.

Rob
Comment 9 Brian Fitzpatrick CLA 2007-10-29 09:21:39 EDT
Sorry to hear about your daughter. I hope everything works out ok.

Can you point me in a direction for the LDAP code and I'll take a look?

--Fitz
Comment 10 rob li CLA 2007-10-31 01:27:40 EDT
(In reply to comment #9)
> Sorry to hear about your daughter. I hope everything works out ok.
> Can you point me in a direction for the LDAP code and I'll take a look?
> --Fitz

Brian,

I think it has nothing to do with the LDAP code. What I found is in InternalProfileManaqger.getProfileByFullPath(), 

if (testRepo != null && mRepositories.contains(testRepo)) {
  hasRepository = true;
}

should really be:

if (testRepo != null && mRepositories.contains(repository represented by testRepo)) {
  hasRepository = true;
}

because mRepositories contains IConnectionProfileRepository. 

Also, ConnectionProfileCreateChange.isValid still compares the new cp name with cps in all repositories. 

After fixing this two I was able to create a cp in my ldap while a same named cp exists in local. But there is still problem...when I delete the cp from ldap, the local one also gets deleted...seems there is something need done in the delete code. I attached my patch for your consideration.


Comment 11 rob li CLA 2007-10-31 01:28:35 EDT
Created attachment 81687 [details]
patch
Comment 12 Brian Fitzpatrick CLA 2007-11-02 17:23:49 EDT
Created attachment 82006 [details]
Patch for path issues #2

Hey Rob...

I think I have your issues resolved... It works in the sample File repository anyway. Let me know if you have any further issues in the LDAP repository. I don't have that code, so it's hard to test against it. 

--Fitz
Comment 13 Brian Fitzpatrick CLA 2007-11-13 12:30:21 EST
Still waiting for feedback 
Comment 14 rob li CLA 2007-11-14 21:15:15 EST
(In reply to comment #13)
> Still waiting for feedback 

Hey Brian,

I tested with 'Patch for path issues #2', yes it works, except in one scenario. Please try this: with a profile named "MyCP" in local, add a same named cp to file repository. You'll find the cp cannot be added. Other than this, the changes look good to me. Thanks!
Comment 15 Brian Fitzpatrick CLA 2008-01-08 12:23:57 EST
Will try and get this in for Ganymede.
Comment 16 Brian Fitzpatrick CLA 2008-01-25 16:53:31 EST
Delivered fixes to the 1.6 head stream. Works for creating, renaming, and deleting profiles from multiple repositories. Tested extensively using the File repository.
Comment 17 Brian Fitzpatrick CLA 2008-07-01 17:40:09 EDT
Closing bugs