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 <ftwee...@redhat.com> > 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..bce675e7bf993d97a086fb830e34d50000c4f396 > 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 <ftwee...@redhat.com> > 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 > relying on the current behaviour, so add the clear() method to the > interface. > > Part of: https://fedorahosted.org/pki/ticket/2588 > --- > base/common/src/com/netscape/certsrv/base/ISourceConfigStore.java | 5 > +++++ > .../cmscore/src/com/netscape/cmscore/base/PropConfigStore.java | 4 ++++ > 2 files changed, 9 insertions(+) > > diff --git > a/base/common/src/com/netscape/certsrv/base/ISourceConfigStore.java > b/base/common/src/com/netscape/certsrv/base/ISourceConfigStore.java > index > 42637c258258472d8469fd8c691a826e3bcb1b3e..8eb86c2ba5cfd1522f52b98a676d1a509424c270 > 100644 > --- a/base/common/src/com/netscape/certsrv/base/ISourceConfigStore.java > +++ b/base/common/src/com/netscape/certsrv/base/ISourceConfigStore.java > @@ -63,6 +63,11 @@ public interface ISourceConfigStore extends Serializable { > public Enumeration<String> keys(); > > /** > + * Clear the config store. > + */ > + public void clear(); > + > + /** > * Reads a config store from an input stream. > * > * @param in input stream where the properties are located > diff --git > a/base/server/cmscore/src/com/netscape/cmscore/base/PropConfigStore.java > b/base/server/cmscore/src/com/netscape/cmscore/base/PropConfigStore.java > index > cc16e247d01428f958d0d397ff95127fcb8d2f45..acf28441337b76628d47dc58351a0233568397d8 > 100644 > --- a/base/server/cmscore/src/com/netscape/cmscore/base/PropConfigStore.java > +++ b/base/server/cmscore/src/com/netscape/cmscore/base/PropConfigStore.java > @@ -223,6 +223,10 @@ public class PropConfigStore implements IConfigStore, > Cloneable { > } > } > > + public synchronized void clear() { > + mSource.clear(); > + } > + > /** > * Reads a config store from an input stream. > * > -- > 2.9.3 > > From 87ceb6f2e0ad93359f6ca94a08a8cb7f2bbfeaaa Mon Sep 17 00:00:00 2001 > From: Fraser Tweedale <ftwee...@redhat.com> > Date: Tue, 7 Feb 2017 21:12:08 +1000 > Subject: [PATCH 161/161] ProfileService: clear profile attributes when > modifying > > When modifying a profile, attributes are not cleared. Attributes > that were removed in the updated profile configuration are not > actually removed. > > When updating a profile via PUT /ca/rest/profiles/{id}/raw, clear > the config store before loading the new configuration. > > Fixes: https://fedorahosted.org/pki/ticket/2588 > --- > base/ca/src/org/dogtagpki/server/ca/rest/ProfileService.java | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/base/ca/src/org/dogtagpki/server/ca/rest/ProfileService.java > b/base/ca/src/org/dogtagpki/server/ca/rest/ProfileService.java > index > 41d009b9d38003f054f45c2dd8070de8f46065f3..26e86c51d388d83fc7070b355505f011426350c0 > 100644 > --- a/base/ca/src/org/dogtagpki/server/ca/rest/ProfileService.java > +++ b/base/ca/src/org/dogtagpki/server/ca/rest/ProfileService.java > @@ -738,6 +738,7 @@ public class ProfileService extends PKIService implements > ProfileResource { > } > > // no error thrown, so commit updated profile config > + profile.getConfigStore().clear(); > profile.getConfigStore().load(new ByteArrayInputStream(data)); > ps.disableProfile(profileId); > ps.commitProfile(profileId); > -- > 2.9.3 > > _______________________________________________ > Pki-devel mailing list > Pki-devel@redhat.com > https://www.redhat.com/mailman/listinfo/pki-devel _______________________________________________ Pki-devel mailing list Pki-devel@redhat.com https://www.redhat.com/mailman/listinfo/pki-devel