Re: [Pki-devel] [PATCH] 0159..0161 Fix config param removal in profile modification
I have created a gerrit review for this patchset: https://review.gerrithub.io/#/c/357607/ Thanks, Fraser On Tue, Feb 07, 2017 at 09:39:52PM +1000, Fraser Tweedale wrote: > Please review the attached patches which fix > https://fedorahosted.org/pki/ticket/2588, a bug in profile > modification where config params can only be added or changed, but > not removed. > > Thanks, > Fraser > From 0a86f63cfe2d5391befe401541e9dcc0dae6ce29 Mon Sep 17 00:00:00 2001 > From: Fraser Tweedale > Date: Tue, 7 Feb 2017 17:27:06 +1000 > Subject: [PATCH 159/161] LDAPProfileSubsystem: avoid duplicating logic in > superclass > > Part of: https://fedorahosted.org/pki/ticket/2588 > --- > .../cmscore/profile/AbstractProfileSubsystem.java | 7 +++- > .../cmscore/profile/LDAPProfileSubsystem.java | 43 > -- > 2 files changed, 13 insertions(+), 37 deletions(-) > > diff --git > a/base/server/cmscore/src/com/netscape/cmscore/profile/AbstractProfileSubsystem.java > > b/base/server/cmscore/src/com/netscape/cmscore/profile/AbstractProfileSubsystem.java > index > 116b8e2026e80b012fb87647fd8924b567194fa3..2a209ad5b2656d65db57d36b7ecb2745527ab081 > 100644 > --- > a/base/server/cmscore/src/com/netscape/cmscore/profile/AbstractProfileSubsystem.java > +++ > b/base/server/cmscore/src/com/netscape/cmscore/profile/AbstractProfileSubsystem.java > @@ -121,7 +121,7 @@ public abstract class AbstractProfileSubsystem implements > IProfileSubsystem { > /** > * Commits a profile. > */ > -public void commitProfile(String id) > +public synchronized void commitProfile(String id) > throws EProfileException { > IConfigStore cs = mProfiles.get(id).getConfigStore(); > > @@ -157,6 +157,11 @@ public abstract class AbstractProfileSubsystem > implements IProfileSubsystem { > > // finally commit the configStore > // > +commitConfigStore(id, cs); > +} > + > +protected void commitConfigStore(String id, IConfigStore cs) > +throws EProfileException { > try { > cs.commit(false); > } catch (EBaseException e) { > diff --git > a/base/server/cmscore/src/com/netscape/cmscore/profile/LDAPProfileSubsystem.java > > b/base/server/cmscore/src/com/netscape/cmscore/profile/LDAPProfileSubsystem.java > index > fff8ead3f2088aedaf5856c308dd33be90af7779..bce675e7bf993d97a086fb830e34d5c4f396 > 100644 > --- > a/base/server/cmscore/src/com/netscape/cmscore/profile/LDAPProfileSubsystem.java > +++ > b/base/server/cmscore/src/com/netscape/cmscore/profile/LDAPProfileSubsystem.java > @@ -303,43 +303,14 @@ public class LDAPProfileSubsystem > readProfile(entry); > } > > +/** > + * Commit the configStore and track the resulting > + * entryUSN and (in case of add) the nsUniqueId > + */ > @Override > -public synchronized void commitProfile(String id) throws > EProfileException { > -LDAPConfigStore cs = (LDAPConfigStore) > mProfiles.get(id).getConfigStore(); > - > -// first create a *new* profile object from the configStore > -// and initialise it with the updated configStore > -// > -IPluginRegistry registry = (IPluginRegistry) > -CMS.getSubsystem(CMS.SUBSYSTEM_REGISTRY); > -String classId = mProfileClassIds.get(id); > -IPluginInfo info = registry.getPluginInfo("profile", classId); > -String className = info.getClassName(); > -IProfile newProfile = null; > -try { > -newProfile = (IProfile) Class.forName(className).newInstance(); > -} catch (ClassNotFoundException | InstantiationException | > IllegalAccessException e) { > -throw new EProfileException("Could not instantiate class '" > -+ classId + "' for profile '" + id + "': " + e); > -} > -newProfile.setId(id); > -try { > -newProfile.init(this, cs); > -} catch (EBaseException e) { > -throw new EProfileException( > -"Failed to initialise profile '" + id + "': " + e); > -} > - > -// next replace the existing profile with the new profile; > -// this is to avoid any intermediate state where the profile > -// is not fully initialised with its inputs, outputs and > -// policy objects. > -// > -mProfiles.put(id, newProfile); > - > -// finally commit the configStore and track the resulting > -// entryUSN and (in case of add) the nsUniqueId > -// > +protected void commitConfigStore(String id, IConfigStore configStore) > +throws EProfileException { > +LDAPConfigStore cs = (LDAPConfigStore) configStore; > try { > String[] attrs = {"entryUSN", "nsUniqueId"}; > LDAPEntry entry = cs.commitReturn(false, attrs); > -- > 2.9.3 > > From ca09f58f4a953fb8d40898a1924f236bba42fa29 Mon Sep 17 0
[Pki-devel] [PATCH] 0159..0161 Fix config param removal in profile modification
Please review the attached patches which fix https://fedorahosted.org/pki/ticket/2588, a bug in profile modification where config params can only be added or changed, but not removed. Thanks, Fraser From 0a86f63cfe2d5391befe401541e9dcc0dae6ce29 Mon Sep 17 00:00:00 2001 From: Fraser Tweedale Date: Tue, 7 Feb 2017 17:27:06 +1000 Subject: [PATCH 159/161] LDAPProfileSubsystem: avoid duplicating logic in superclass Part of: https://fedorahosted.org/pki/ticket/2588 --- .../cmscore/profile/AbstractProfileSubsystem.java | 7 +++- .../cmscore/profile/LDAPProfileSubsystem.java | 43 -- 2 files changed, 13 insertions(+), 37 deletions(-) diff --git a/base/server/cmscore/src/com/netscape/cmscore/profile/AbstractProfileSubsystem.java b/base/server/cmscore/src/com/netscape/cmscore/profile/AbstractProfileSubsystem.java index 116b8e2026e80b012fb87647fd8924b567194fa3..2a209ad5b2656d65db57d36b7ecb2745527ab081 100644 --- a/base/server/cmscore/src/com/netscape/cmscore/profile/AbstractProfileSubsystem.java +++ b/base/server/cmscore/src/com/netscape/cmscore/profile/AbstractProfileSubsystem.java @@ -121,7 +121,7 @@ public abstract class AbstractProfileSubsystem implements IProfileSubsystem { /** * Commits a profile. */ -public void commitProfile(String id) +public synchronized void commitProfile(String id) throws EProfileException { IConfigStore cs = mProfiles.get(id).getConfigStore(); @@ -157,6 +157,11 @@ public abstract class AbstractProfileSubsystem implements IProfileSubsystem { // finally commit the configStore // +commitConfigStore(id, cs); +} + +protected void commitConfigStore(String id, IConfigStore cs) +throws EProfileException { try { cs.commit(false); } catch (EBaseException e) { diff --git a/base/server/cmscore/src/com/netscape/cmscore/profile/LDAPProfileSubsystem.java b/base/server/cmscore/src/com/netscape/cmscore/profile/LDAPProfileSubsystem.java index fff8ead3f2088aedaf5856c308dd33be90af7779..bce675e7bf993d97a086fb830e34d5c4f396 100644 --- a/base/server/cmscore/src/com/netscape/cmscore/profile/LDAPProfileSubsystem.java +++ b/base/server/cmscore/src/com/netscape/cmscore/profile/LDAPProfileSubsystem.java @@ -303,43 +303,14 @@ public class LDAPProfileSubsystem readProfile(entry); } +/** + * Commit the configStore and track the resulting + * entryUSN and (in case of add) the nsUniqueId + */ @Override -public synchronized void commitProfile(String id) throws EProfileException { -LDAPConfigStore cs = (LDAPConfigStore) mProfiles.get(id).getConfigStore(); - -// first create a *new* profile object from the configStore -// and initialise it with the updated configStore -// -IPluginRegistry registry = (IPluginRegistry) -CMS.getSubsystem(CMS.SUBSYSTEM_REGISTRY); -String classId = mProfileClassIds.get(id); -IPluginInfo info = registry.getPluginInfo("profile", classId); -String className = info.getClassName(); -IProfile newProfile = null; -try { -newProfile = (IProfile) Class.forName(className).newInstance(); -} catch (ClassNotFoundException | InstantiationException | IllegalAccessException e) { -throw new EProfileException("Could not instantiate class '" -+ classId + "' for profile '" + id + "': " + e); -} -newProfile.setId(id); -try { -newProfile.init(this, cs); -} catch (EBaseException e) { -throw new EProfileException( -"Failed to initialise profile '" + id + "': " + e); -} - -// next replace the existing profile with the new profile; -// this is to avoid any intermediate state where the profile -// is not fully initialised with its inputs, outputs and -// policy objects. -// -mProfiles.put(id, newProfile); - -// finally commit the configStore and track the resulting -// entryUSN and (in case of add) the nsUniqueId -// +protected void commitConfigStore(String id, IConfigStore configStore) +throws EProfileException { +LDAPConfigStore cs = (LDAPConfigStore) configStore; try { String[] attrs = {"entryUSN", "nsUniqueId"}; LDAPEntry entry = cs.commitReturn(false, attrs); -- 2.9.3 From ca09f58f4a953fb8d40898a1924f236bba42fa29 Mon Sep 17 00:00:00 2001 From: Fraser Tweedale Date: Tue, 7 Feb 2017 17:39:33 +1000 Subject: [PATCH 160/161] ISourceConfigStore: add clear() method to interface The SourceConfigStore load() method does not clear the config store, but this might be necessary to avoid stale data if wanting to perform a complete replacement of the data (e.g. reload from file). We should not change the behaviour of load() in case some code is rely