Merge pull request #38875 from makortel/failedToRegisterConsumes

Throw GetByLabelWithoutRegistration exception also when no module in a job registered consumption of a given branch
cms-sw · Aug 2, 2022 · 79e7839 · 79e7839
2 parents 3266dc5 + 7b2d9a7
commit 79e7839
Show file tree

Hide file tree

Showing 6 changed files with 206 additions and 0 deletions.
diff --git a/FWCore/Framework/src/Principal.cc b/FWCore/Framework/src/Principal.cc
@@ -769,6 +769,24 @@ namespace edm {
                                 inputTag.instance(),
                                 appendCurrentProcessIfAlias(inputTag.process(), processConfiguration_->processName()));
       } else if (index == ProductResolverIndexInvalid) {
+        // can occur because of missing consumes if nothing else in the process consumes the product
+        for (auto const& item : preg_->productList()) {
+          auto const& bd = item.second;
+          if (bd.present() and bd.unwrappedTypeID() == typeID and bd.moduleLabel() == inputTag.label() and
+              bd.productInstanceName() == inputTag.instance()) {
+            bool const inCurrentProcess = bd.processName() == processConfiguration_->processName();
+            if (inputTag.process().empty() or bd.processName() == inputTag.process() or
+                (skipCurrentProcess and not inCurrentProcess) or
+                (inputTag.process() == InputTag::kCurrentProcess and inCurrentProcess)) {
+              failedToRegisterConsumes(
+                  kindOfType,
+                  typeID,
+                  inputTag.label(),
+                  inputTag.instance(),
+                  appendCurrentProcessIfAlias(inputTag.process(), processConfiguration_->processName()));
+            }
+          }
+        }
         return nullptr;
       }
       inputTag.tryToCacheIndex(index, typeID, branchType(), &productRegistry());
@@ -808,6 +826,16 @@ namespace edm {
     if (index == ProductResolverIndexAmbiguous) {
       throwAmbiguousException("findProductByLabel", typeID, label, instance, process);
     } else if (index == ProductResolverIndexInvalid) {
+      // can occur because of missing consumes if nothing else in the process consumes the product
+      for (auto const& item : preg_->productList()) {
+        auto const& bd = item.second;
+        if (bd.present() and bd.unwrappedTypeID() == typeID and bd.moduleLabel() == label and
+            bd.productInstanceName() == instance) {
+          if (process.empty() or bd.processName() == process) {
+            failedToRegisterConsumes(kindOfType, typeID, label, instance, process);
+          }
+        }
+      }
       return nullptr;
     }
 

diff --git a/FWCore/Integration/test/BuildFile.xml b/FWCore/Integration/test/BuildFile.xml
@@ -545,4 +545,12 @@
   </test>
   <test name="TestIntegrationFinalPath" command="test_finalpath.sh"/>
 
+  <library file="TestGetByLabelAnalyzer.cc" name ="TestGetByLabelAnalyzer">
+    <flags EDM_PLUGIN="1"/>
+    <use name="FWCore/Framework"/>
+    <use name="FWCore/ParameterSet"/>
+    <use name="FWCore/MessageLogger"/>
+  </library>
+  <test name="TestIntegrationGetByLabel" command="run_TestGetByLabel.sh"/>
+
 </environment>
diff --git a/FWCore/Integration/test/TestGetByLabelAnalyzer.cc b/FWCore/Integration/test/TestGetByLabelAnalyzer.cc
@@ -0,0 +1,76 @@
+#include "FWCore/Framework/interface/global/EDAnalyzer.h"
+#include "FWCore/Framework/interface/Event.h"
+#include "FWCore/Framework/interface/MakerMacros.h"
+
+#include "DataFormats/TestObjects/interface/ToyProducts.h"
+#include "DataFormats/TestObjects/interface/ThingCollection.h"
+
+namespace edmtest {
+  template <typename T>
+  class TestGetByLabelAnalyzerT : public edm::global::EDAnalyzer<> {
+  public:
+    TestGetByLabelAnalyzerT(edm::ParameterSet const& iPSet)
+        : src_(iPSet.getUntrackedParameter<edm::InputTag>("src")),
+          getCategory_(iPSet.getUntrackedParameter<std::string>("getExceptionCategory")),
+          accessCategory_(iPSet.getUntrackedParameter<std::string>("accessExceptionCategory")) {
+      if (iPSet.getUntrackedParameter<bool>("consumes")) {
+        consumes<T>(src_);
+      }
+    }
+
+    static void fillDescriptions(edm::ConfigurationDescriptions& descriptions) {
+      edm::ParameterSetDescription desc;
+      desc.addUntracked<edm::InputTag>("src");
+      desc.addUntracked<std::string>("getExceptionCategory", "");
+      desc.addUntracked<std::string>("accessExceptionCategory", "");
+      desc.addUntracked<bool>("consumes", false);
+      descriptions.addDefault(desc);
+    }
+
+    void analyze(edm::StreamID, edm::Event const& event, edm::EventSetup const&) const {
+      edm::Handle<T> handle;
+
+      auto test = [](std::string const& category, std::string const& msg, auto func) {
+        bool caught = false;
+        try {
+          func();
+        } catch (cms::Exception& e) {
+          caught = true;
+          if (category.empty())
+            throw;
+          if (e.category() != category) {
+            throw cms::Exception("Assert")
+                << "Expected cms::Exception from " << msg << " with category " << category << ", got " << e.category();
+          }
+          return false;
+        }
+        if (not category.empty() and not caught) {
+          throw cms::Exception("Assert") << "Expected cms::Exception to be thrown from " << msg << ", but got nothing";
+        }
+        return true;
+      };
+
+      bool noException = test(getCategory_, "getByLabel(InputTag)", [&]() { event.getByLabel(src_, handle); });
+      if (noException) {
+        test(accessCategory_, "*handle from InputTag", [&]() { *handle; });
+      }
+
+      noException =
+          test(getCategory_, "getByLabel(strings)", [&]() { event.getByLabel(src_.label(), src_.instance(), handle); });
+      if (noException) {
+        test(accessCategory_, "*handle from strings", [&]() { *handle; });
+      }
+    }
+
+  private:
+    edm::InputTag const src_;
+    std::string const getCategory_;
+    std::string const accessCategory_;
+  };
+
+  using TestGetByLabelIntAnalyzer = TestGetByLabelAnalyzerT<edmtest::IntProduct>;
+  using TestGetByLabelThingAnalyzer = TestGetByLabelAnalyzerT<edmtest::ThingCollection>;
+}  // namespace edmtest
+
+DEFINE_FWK_MODULE(edmtest::TestGetByLabelIntAnalyzer);
+DEFINE_FWK_MODULE(edmtest::TestGetByLabelThingAnalyzer);
diff --git a/FWCore/Integration/test/run_TestGetByLabel.sh b/FWCore/Integration/test/run_TestGetByLabel.sh
@@ -0,0 +1,24 @@
+#!/bin/bash
+
+function die { echo Failure $1: status $2 ; exit $2 ; }
+
+TEST_NAME=TestGetByLabel
+if [ -d ${TEST_NAME} ]; then
+    rm -fR ${TEST_NAME}
+fi
+mkdir ${TEST_NAME}
+
+pushd ${TEST_NAME}
+
+TEST_PATH=${LOCALTOP}/src/FWCore/Integration/test
+
+cmsRun ${TEST_PATH}/testGetByLabelStep1_cfg.py || die "Failed cmsRun testGetByLabel_step1_cfg.py" $1
+
+cmsRun ${TEST_PATH}/testGetByLabelStep2_cfg.py || die "Failed cmsRun testGetByLabel_step2_cfg.py" $1
+cmsRun ${TEST_PATH}/testGetByLabelStep2_cfg.py -- --noConsumes || die "Failed cmsRun testGetByLabel_step2_cfg.py --noConsumes" $1
+cmsRun ${TEST_PATH}/testGetByLabelStep2_cfg.py -- --thing || die "Failed cmsRun testGetByLabel_step2_cfg.py --thing" $1
+cmsRun ${TEST_PATH}/testGetByLabelStep2_cfg.py -- --thing --noConsumes || die "Failed cmsRun testGetByLabel_step2_cfg.py --thing --noConsumes" $1
+cmsRun ${TEST_PATH}/testGetByLabelStep2_cfg.py -- --otherInt || die "Failed cmsRun testGetByLabel_step2_cfg.py --otherInt" $1
+cmsRun ${TEST_PATH}/testGetByLabelStep2_cfg.py -- --otherInt --noConsumes || die "Failed cmsRun testGetByLabel_step2_cfg.py --otherInt --noConsumes" $1
+
+popd
diff --git a/FWCore/Integration/test/testGetByLabelStep1_cfg.py b/FWCore/Integration/test/testGetByLabelStep1_cfg.py
@@ -0,0 +1,15 @@
+import FWCore.ParameterSet.Config as cms
+
+process = cms.Process("TESTPROD")
+process.maxEvents.input = 3
+
+process.source = cms.Source("EmptySource")
+
+process.intProduct = cms.EDProducer("IntProducer", ivalue = cms.int32(42))
+
+process.output = cms.OutputModule("PoolOutputModule",
+    fileName = cms.untracked.string('getbylabel_step1.root')
+)
+
+process.p = cms.Path(process.intProduct)
+process.ep = cms.EndPath(process.output)
diff --git a/FWCore/Integration/test/testGetByLabelStep2_cfg.py b/FWCore/Integration/test/testGetByLabelStep2_cfg.py
@@ -0,0 +1,55 @@
+import FWCore.ParameterSet.Config as cms
+
+import sys
+import argparse
+
+parser = argparse.ArgumentParser(prog=sys.argv[0], description='Test ProcessAccelerator.')
+
+parser.add_argument("--noConsumes", help="Do not call consumes", action="store_true")
+parser.add_argument("--thing", help="Add producer and consumer for Thing", action="store_true")
+parser.add_argument("--otherInt", help="Add another producer and consumer for int", action="store_true")
+
+argv = sys.argv[:]
+if '--' in argv:
+    argv.remove("--")
+args, unknown = parser.parse_known_args(argv)
+
+process = cms.Process("TESTANA")
+process.maxEvents.input = -1
+
+process.source = cms.Source("PoolSource",
+    fileNames = cms.untracked.vstring("file:getbylabel_step1.root")
+)
+
+process.intAnalyzer = cms.EDAnalyzer("edmtest::TestGetByLabelIntAnalyzer",
+    src = cms.untracked.InputTag("intProduct"),
+    consumes = cms.untracked.bool(True)
+)
+
+process.p = cms.Path(
+    process.intAnalyzer
+)
+
+if args.thing:
+    process.thingProduct = cms.EDProducer("ThingProducer")
+    process.thingAnalyzer = cms.EDAnalyzer("edmtest::TestGetByLabelThingAnalyzer",
+        src = cms.untracked.InputTag("thingProduct"),
+        consumes = cms.untracked.bool(True)
+    )
+    process.p += (process.thingProduct+process.thingAnalyzer)
+
+if args.otherInt:
+    process.otherIntProduct = cms.EDProducer("IntProducer", ivalue = cms.int32(314))
+    process.otherIntAnalyzer = cms.EDAnalyzer("edmtest::TestGetByLabelIntAnalyzer",
+        src = cms.untracked.InputTag("otherIntProduct"),
+        consumes = cms.untracked.bool(True)
+    )
+    process.p += (process.otherIntProduct+process.otherIntAnalyzer)
+
+if args.noConsumes:
+    process.intAnalyzer.consumes = False
+    process.intAnalyzer.getExceptionCategory = cms.untracked.string("GetByLabelWithoutRegistration")
+
+    if args.thing:
+        process.thingAnalyzer.consumes = False
+        process.thingAnalyzer.getExceptionCategory = cms.untracked.string("GetByLabelWithoutRegistration")