Move EventSetup module PSet validation to be called from ComponentMaker

FWCore/Framework/interface/ComponentFactory.h

@@ -54,7 +54,7 @@ namespace edm {
       // ---------- const member functions ---------------------
       std::shared_ptr<base_type> addTo(EventSetupsController& esController,
                                        EventSetupProvider& iProvider,
-                                       edm::ParameterSet const& iConfiguration,
+                                       edm::ParameterSet& iConfiguration,
                                        ModuleTypeResolverBase const* resolver,
                                        bool replaceExisting = false) const {
         std::string modtype = iConfiguration.template getParameter<std::string>("@module_type");

FWCore/Framework/interface/ComponentMaker.h

@@ -22,10 +22,14 @@
 #include <memory>
 #include <string>
 
+#include <fmt/format.h>
+
 // user include files
 #include "FWCore/Framework/interface/ComponentDescription.h"
 #include "FWCore/Framework/interface/DataProxyProvider.h"
 #include "FWCore/Framework/interface/EventSetupRecordIntervalFinder.h"
+#include "FWCore/ParameterSet/interface/ParameterSetDescriptionFiller.h"
+#include "FWCore/Utilities/interface/ConvertException.h"
 
 // forward declarations
 
@@ -49,7 +53,7 @@ namespace edm {
       typedef typename T::base_type base_type;
       virtual std::shared_ptr<base_type> addTo(EventSetupsController& esController,
                                                EventSetupProvider& iProvider,
-                                               ParameterSet const& iConfiguration,
+                                               ParameterSet& iConfiguration,
                                                bool replaceExisting) const = 0;
     };
 
@@ -64,7 +68,7 @@ namespace edm {
       // ---------- const member functions ---------------------
       std::shared_ptr<base_type> addTo(EventSetupsController& esController,
                                        EventSetupProvider& iProvider,
-                                       ParameterSet const& iConfiguration,
+                                       ParameterSet& iConfiguration,
                                        bool replaceExisting) const override;
 
       // ---------- static member functions --------------------
@@ -92,12 +96,30 @@ namespace edm {
     std::shared_ptr<typename ComponentMaker<T, TComponent>::base_type> ComponentMaker<T, TComponent>::addTo(
         EventSetupsController& esController,
         EventSetupProvider& iProvider,
-        ParameterSet const& iConfiguration,
+        ParameterSet& iConfiguration,
         bool replaceExisting) const {
       // This adds components to the EventSetupProvider for the process. It might
       // make a new component then add it or reuse a component from an earlier
       // SubProcess or the top level process and add that.
 
+      {
+        auto modtype = iConfiguration.getParameter<std::string>("@module_type");
+        auto moduleLabel = iConfiguration.getParameter<std::string>("@module_label");
+        try {
+          edm::convertException::wrap([&]() {
+            ConfigurationDescriptions descriptions(T::baseType(), modtype);
+            fillDetails::fillIfExists<TComponent>(descriptions);
+            fillDetails::prevalidateIfExists<TComponent>(descriptions);
+            descriptions.validate(iConfiguration, moduleLabel);
+            iConfiguration.registerIt();
+          });
+        } catch (cms::Exception& iException) {
+          iException.addContext(fmt::format(
+              "Validating configuration of {} of type {} with label: '{}'", T::baseType(), modtype, moduleLabel));
+          throw;
+        }
+      }
+
       if (!replaceExisting) {
         std::shared_ptr<typename T::base_type> alreadyMadeComponent =
             T::getComponentAndRegisterProcess(esController, iConfiguration);

FWCore/Framework/interface/EventSetupsController.h

@@ -45,17 +45,18 @@ namespace edm {
 
     class ESProducerInfo {
     public:
-      ESProducerInfo(ParameterSet const* ps, std::shared_ptr<DataProxyProvider> const& pr)
+      ESProducerInfo(ParameterSet* ps, std::shared_ptr<DataProxyProvider> const& pr)
           : pset_(ps), provider_(pr), subProcessIndexes_() {}
 
-      ParameterSet const* pset() const { return pset_; }
+      ParameterSet const* pset() const { return pset_.get(); }
+      ParameterSet* pset() { return pset_.get(); }
       std::shared_ptr<DataProxyProvider> const& provider() { return get_underlying(provider_); }
       DataProxyProvider const* providerGet() const { return provider_.get(); }
       std::vector<unsigned>& subProcessIndexes() { return subProcessIndexes_; }
       std::vector<unsigned> const& subProcessIndexes() const { return subProcessIndexes_; }
 
     private:
-      ParameterSet const* pset_;
+      edm::propagate_const<ParameterSet*> pset_;
       propagate_const<std::shared_ptr<DataProxyProvider>> provider_;
       std::vector<unsigned> subProcessIndexes_;
     };
@@ -116,7 +117,7 @@ namespace edm {
 
       std::shared_ptr<DataProxyProvider> getESProducerAndRegisterProcess(ParameterSet const& pset,
                                                                          unsigned subProcessIndex);
-      void putESProducer(ParameterSet const& pset,
+      void putESProducer(ParameterSet& pset,
                          std::shared_ptr<DataProxyProvider> const& component,
                          unsigned subProcessIndex);
 
@@ -149,7 +150,7 @@ namespace edm {
                                 unsigned subProcessIndex,
                                 unsigned precedingProcessIndex) const;
 
-      ParameterSet const* getESProducerPSet(ParameterSetID const& psetID, unsigned subProcessIndex) const;
+      ParameterSet& getESProducerPSet(ParameterSetID const& psetID, unsigned subProcessIndex);
 
       std::vector<propagate_const<std::shared_ptr<EventSetupProvider>>> const& providers() const { return providers_; }
 

FWCore/Framework/interface/LooperFactory.h

@@ -82,6 +82,7 @@ namespace edm {
     struct LooperMakerTraits {
       typedef EDLooperBase base_type;
       static std::string name();
+      static std::string const& baseType();
       template <class T>
       static void addTo(EventSetupProvider& iProvider, std::shared_ptr<T> iComponent, ParameterSet const&, bool) {
         //a looper does not always have to be a provider or a finder

FWCore/Framework/interface/ModuleFactory.h

@@ -38,6 +38,7 @@ namespace edm {
       typedef DataProxyProvider base_type;
 
       static std::string name();
+      static std::string const& baseType();
       static void addTo(EventSetupProvider& iProvider,
                         std::shared_ptr<DataProxyProvider> iComponent,
                         ParameterSet const&,
@@ -46,7 +47,7 @@ namespace edm {
       static std::shared_ptr<base_type> getComponentAndRegisterProcess(EventSetupsController& esController,
                                                                        ParameterSet const& iConfiguration);
       static void putComponent(EventSetupsController& esController,
-                               ParameterSet const& iConfiguration,
+                               ParameterSet& iConfiguration,
                                std::shared_ptr<base_type> const& component);
     };
     template <class TType>

FWCore/Framework/interface/SourceFactory.h

@@ -53,6 +53,7 @@ namespace edm {
     struct SourceMakerTraits {
       typedef EventSetupRecordIntervalFinder base_type;
       static std::string name();
+      static std::string const& baseType();
       template <class T>
       static void addTo(EventSetupProvider& iProvider,
                         std::shared_ptr<T> iComponent,

FWCore/Framework/src/EventProcessor.cc

@@ -200,21 +200,6 @@ namespace edm {
   }
 
   // ---------------------------------------------------------------
-  void validateLooper(ParameterSet& pset) {
-    auto const modtype = pset.getParameter<std::string>("@module_type");
-    auto const moduleLabel = pset.getParameter<std::string>("@module_label");
-    auto filler = ParameterSetDescriptionFillerPluginFactory::get()->create(modtype);
-    ConfigurationDescriptions descriptions(filler->baseType(), modtype);
-    filler->fill(descriptions);
-    try {
-      edm::convertException::wrap([&]() { descriptions.validate(pset, moduleLabel); });
-    } catch (cms::Exception& iException) {
-      iException.addContext(
-          fmt::format("Validating configuration of EDLooper of type {} with label: '{}'", modtype, moduleLabel));
-      throw;
-    }
-  }
-
   std::shared_ptr<EDLooperBase> fillLooper(eventsetup::EventSetupsController& esController,
                                            eventsetup::EventSetupProvider& cp,
                                            ParameterSet& params,
@@ -225,8 +210,6 @@ namespace edm {
 
     for (auto const& looperName : loopers) {
       ParameterSet* providerPSet = params.getPSetForUpdate(looperName);
-      validateLooper(*providerPSet);
-      providerPSet->registerIt();
       // Unlikely we would ever need the ModuleTypeResolver in Looper
       vLooper = eventsetup::LooperFactory::get()->addTo(esController, cp, *providerPSet, nullptr);
     }

FWCore/Framework/src/EventSetupProvider.cc

@@ -505,7 +505,7 @@ namespace edm {
         ParameterSetID const& psetID = candidate.first;
         bool canBeShared = candidate.second;
         if (canBeShared) {
-          ParameterSet const& pset = *esController.getESProducerPSet(psetID, subProcessIndex_);
+          ParameterSet const& pset = esController.getESProducerPSet(psetID, subProcessIndex_);
           logInfoWhenSharing(pset);
           ParameterSetIDHolder psetIDHolder(psetID);
           sharingCheckDone.insert(psetIDHolder);
@@ -530,7 +530,7 @@ namespace edm {
           }
         } else {
           if (esController.isLastMatch(psetID, subProcessIndex_, precedingESProvider.subProcessIndex_)) {
-            ParameterSet const& pset = *esController.getESProducerPSet(psetID, subProcessIndex_);
+            ParameterSet& pset = esController.getESProducerPSet(psetID, subProcessIndex_);
             ModuleFactory::get()->addTo(esController, *this, pset, resolver, true);
           }
         }

FWCore/Framework/src/EventSetupProviderMaker.cc

@@ -12,7 +12,6 @@
 #include "FWCore/ParameterSet/interface/ParameterSet.h"
 #include "FWCore/ParameterSet/interface/ParameterSetDescriptionFillerBase.h"
 #include "FWCore/ParameterSet/interface/ParameterSetDescriptionFillerPluginFactory.h"
-#include "FWCore/Utilities/interface/ConvertException.h"
 #include "FWCore/Utilities/interface/EDMException.h"
 #include "FWCore/Utilities/interface/Exception.h"
 
@@ -101,8 +100,6 @@ namespace edm {
            itName != itNameEnd;
            ++itName) {
         ParameterSet* providerPSet = params.getPSetForUpdate(*itName);
-        validateEventSetupParameters(*providerPSet);
-        providerPSet->registerIt();
         ModuleFactory::get()->addTo(esController, cp, *providerPSet, resolver);
       }
 
@@ -111,40 +108,8 @@ namespace edm {
       for (std::vector<std::string>::iterator itName = sources.begin(), itNameEnd = sources.end(); itName != itNameEnd;
            ++itName) {
         ParameterSet* providerPSet = params.getPSetForUpdate(*itName);
-        validateEventSetupParameters(*providerPSet);
-        providerPSet->registerIt();
         SourceFactory::get()->addTo(esController, cp, *providerPSet, resolver);
       }
     }
-
-    // ---------------------------------------------------------------
-    void validateEventSetupParameters(ParameterSet& pset) {
-      std::string modtype;
-      std::string moduleLabel;
-      modtype = pset.getParameter<std::string>("@module_type");
-      moduleLabel = pset.getParameter<std::string>("@module_label");
-      // Check for the "unlabeled" case
-      // This is an artifact left over from the old configuration language
-      // we were using before switching to the python configuration
-      // This is handled in the validation code and python configuration
-      // files by using a label equal to the module typename.
-      if (moduleLabel == std::string("")) {
-        moduleLabel = modtype;
-      }
-
-      std::unique_ptr<ParameterSetDescriptionFillerBase> filler(
-          ParameterSetDescriptionFillerPluginFactory::get()->create(modtype));
-      ConfigurationDescriptions descriptions(filler->baseType(), modtype);
-      filler->fill(descriptions);
-      try {
-        edm::convertException::wrap([&]() { descriptions.validate(pset, moduleLabel); });
-      } catch (cms::Exception& iException) {
-        std::ostringstream ost;
-        ost << "Validating configuration of ESProducer or ESSource of type " << modtype << " with label: '"
-            << moduleLabel << "'";
-        iException.addContext(ost.str());
-        throw;
-      }
-    }
   }  // namespace eventsetup
 }  // namespace edm

FWCore/Framework/src/EventSetupProviderMaker.h

@@ -21,8 +21,6 @@ namespace edm {
                                 EventSetupsController& esController,
                                 EventSetupProvider& cp,
                                 ParameterSet& params);
-
-    void validateEventSetupParameters(ParameterSet& pset);
   }  // namespace eventsetup
 }  // namespace edm
 #endif

FWCore/Framework/src/EventSetupsController.cc

@@ -162,7 +162,7 @@ namespace edm {
       return std::shared_ptr<DataProxyProvider>();
     }
 
-    void EventSetupsController::putESProducer(ParameterSet const& pset,
+    void EventSetupsController::putESProducer(ParameterSet& pset,
                                               std::shared_ptr<DataProxyProvider> const& component,
                                               unsigned subProcessIndex) {
       auto newElement =
@@ -336,8 +336,7 @@ namespace edm {
       return false;
     }
 
-    ParameterSet const* EventSetupsController::getESProducerPSet(ParameterSetID const& psetID,
-                                                                 unsigned subProcessIndex) const {
+    ParameterSet& EventSetupsController::getESProducerPSet(ParameterSetID const& psetID, unsigned subProcessIndex) {
       auto elements = esproducers_.equal_range(psetID);
       for (auto it = elements.first; it != elements.second; ++it) {
         std::vector<unsigned> const& subProcessIndexes = it->second.subProcessIndexes();
@@ -346,12 +345,11 @@ namespace edm {
         if (iFound == subProcessIndexes.end()) {
           continue;
         }
-        return it->second.pset();
+        return *it->second.pset();
       }
       throw edm::Exception(edm::errors::LogicError) << "EventSetupsController::getESProducerPSet\n"
                                                     << "Subprocess index not found. This should never happen\n"
                                                     << "Please report this to a Framework Developer\n";
-      return nullptr;
     }
 
     void EventSetupsController::checkESProducerSharing() {

FWCore/Framework/src/LooperFactory.cc

@@ -14,13 +14,15 @@
 
 // user include files
 #include "FWCore/Framework/interface/LooperFactory.h"
+#include "FWCore/ParameterSet/interface/ParameterSetDescriptionFillerBase.h"
 
 //
 // static member functions
 //
 namespace edm {
   namespace eventsetup {
     std::string LooperMakerTraits::name() { return "CMS EDM Framework EDLooper"; }
+    std::string const& LooperMakerTraits::baseType() { return ParameterSetDescriptionFillerBase::kBaseForEDLooper; }
 
     void LooperMakerTraits::replaceExisting(EventSetupProvider&, std::shared_ptr<EDLooperBase>) {
       throw edm::Exception(edm::errors::LogicError) << "LooperMakerTraits::replaceExisting\n"

FWCore/Framework/src/ModuleFactory.cc

@@ -16,6 +16,7 @@
 #include "FWCore/Framework/interface/ModuleFactory.h"
 #include "FWCore/Framework/interface/EventSetupProvider.h"
 #include "FWCore/Framework/interface/EventSetupsController.h"
+#include "FWCore/ParameterSet/interface/ParameterSetDescriptionFillerBase.h"
 
 //
 // constants, enums and typedefs
@@ -27,6 +28,7 @@ namespace edm {
     // static member functions
     //
     std::string ModuleMakerTraits::name() { return "CMS EDM Framework ESModule"; }
+    std::string const& ModuleMakerTraits::baseType() { return ParameterSetDescriptionFillerBase::kBaseForESProducer; }
     void ModuleMakerTraits::addTo(EventSetupProvider& iProvider,
                                   std::shared_ptr<DataProxyProvider> iComponent,
                                   ParameterSet const&,
@@ -45,7 +47,7 @@ namespace edm {
     }
 
     void ModuleMakerTraits::putComponent(EventSetupsController& esController,
-                                         ParameterSet const& iConfiguration,
+                                         ParameterSet& iConfiguration,
                                          std::shared_ptr<base_type> const& component) {
       esController.putESProducer(iConfiguration, component, esController.indexOfNextProcess());
     }

FWCore/Framework/src/SourceFactory.cc

@@ -18,6 +18,7 @@
 #include "FWCore/Utilities/interface/EDMException.h"
 #include "FWCore/MessageLogger/interface/MessageLogger.h"
 #include "FWCore/ParameterSet/interface/ParameterSet.h"
+#include "FWCore/ParameterSet/interface/ParameterSetDescriptionFillerBase.h"
 
 //
 // static member functions
@@ -26,6 +27,7 @@ namespace edm {
   namespace eventsetup {
 
     std::string SourceMakerTraits::name() { return "CMS EDM Framework ESSource"; }
+    std::string const& SourceMakerTraits::baseType() { return ParameterSetDescriptionFillerBase::kBaseForESSource; }
 
     void SourceMakerTraits::replaceExisting(EventSetupProvider&, std::shared_ptr<EventSetupRecordIntervalFinder>) {
       throw edm::Exception(edm::errors::LogicError) << "SourceMakerTraits::replaceExisting\n"

FWCore/Framework/test/eventsetupscontroller_t.cppunit.cc

@@ -226,10 +226,10 @@ void TestEventSetupsController::esProducerGetAndPutTest() {
   CPPUNIT_ASSERT(!esController.isMatchingESProducer(pset4.id(), 5, 2));
   CPPUNIT_ASSERT_THROW(esController.isMatchingESProducer(pset4.id(), 6, 4), cms::Exception);
 
-  CPPUNIT_ASSERT(esController.getESProducerPSet(pset1.id(), 0) == &pset1);
-  CPPUNIT_ASSERT(esController.getESProducerPSet(pset2.id(), 2) == &pset2);
-  CPPUNIT_ASSERT(esController.getESProducerPSet(pset3.id(), 3) == &pset3);
-  CPPUNIT_ASSERT(esController.getESProducerPSet(pset4.id(), 5) == &pset4);
+  CPPUNIT_ASSERT(&esController.getESProducerPSet(pset1.id(), 0) == &pset1);
+  CPPUNIT_ASSERT(&esController.getESProducerPSet(pset2.id(), 2) == &pset2);
+  CPPUNIT_ASSERT(&esController.getESProducerPSet(pset3.id(), 3) == &pset3);
+  CPPUNIT_ASSERT(&esController.getESProducerPSet(pset4.id(), 5) == &pset4);
   CPPUNIT_ASSERT_THROW(esController.getESProducerPSet(pset4.id(), 6), cms::Exception);
 
   esController.clearComponents();