From a4ff9caf69cb8fbbf3e5d40a258ec99a070b0f94 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 15 Jun 2022 19:43:53 -0400 Subject: DRTVWR-564: WIP: add LLEventPumps::registerPumpFactory() and registerTypeFactory(). Untested. This will support registering just-in-time LLEventAPI instances, instantiated on demand. --- indra/llcommon/llevents.cpp | 44 ++++++++++++++++++++++++++++++++++++++------ indra/llcommon/llevents.h | 39 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 76 insertions(+), 7 deletions(-) (limited to 'indra') diff --git a/indra/llcommon/llevents.cpp b/indra/llcommon/llevents.cpp index 0a213bddef..5725dad9cc 100644 --- a/indra/llcommon/llevents.cpp +++ b/indra/llcommon/llevents.cpp @@ -68,19 +68,51 @@ LLEventPumps::LLEventPumps(): mFactories { - { "LLEventStream", [](const std::string& name, bool tweak) + { "LLEventStream", [](const std::string& name, bool tweak, const std::string& /*type*/) { return new LLEventStream(name, tweak); } }, - { "LLEventMailDrop", [](const std::string& name, bool tweak) + { "LLEventMailDrop", [](const std::string& name, bool tweak, const std::string& /*type*/) { return new LLEventMailDrop(name, tweak); } } }, mTypes { - // LLEventStream is the default for obtain(), so even if somebody DOES - // call obtain("placeholder"), this sample entry won't break anything. - { "placeholder", "LLEventStream" } +// { "placeholder", "LLEventStream" } } {} +bool LLEventPumps::registerTypeFactory(const std::string& type, const TypeFactory& factory) +{ + auto found = mFactories.find(type); + // can't re-register a TypeFactory for a type name that's already registered + if (found != mFactories.end()) + return false; + // doesn't already exist, go ahead and register + mFactories[type] = factory; + return true; +} + +bool LLEventPumps::registerPumpFactory(const std::string& name, const PumpFactory& factory) +{ + // Do we already have a pump by this name? + if (mPumpMap.find(name) != mPumpMap.end()) + return false; + // Do we already have an override for this pump name? + if (mTypes.find(name) != mTypes.end()) + return false; + // Leverage the two-level lookup implemented by mTypes (pump name -> type + // name) and mFactories (type name -> factory). We could instead create a + // whole separate (pump name -> factory) map, and look in both; or we + // could change mTypes to (pump name -> factory) and, for typical type- + // based lookups, use a "factory" that looks up the real factory in + // mFactories. But this works, and we don't expect many calls to make() - + // either explicit or implicit via obtain(). + // Create a bogus type name extremely unlikely to collide with an actual type. + static std::string nul(1, '\0'); + std::string type_name{ nul + name }; + mTypes[name] = type_name; + mFactories[type_name] = factory; + return true; +} + LLEventPump& LLEventPumps::obtain(const std::string& name) { PumpMap::iterator found = mPumpMap.find(name); @@ -114,7 +146,7 @@ LLEventPump& LLEventPumps::make(const std::string& name, bool tweak, // Passing an unrecognized type name is a no-no LLTHROW(BadType(type)); } - auto newInstance = (found->second)(name, tweak); + auto newInstance = (found->second)(name, tweak, type); // LLEventPump's constructor implicitly registers each new instance in // mPumpMap. But remember that we instantiated it (in mOurPumps) so we'll // delete it later. diff --git a/indra/llcommon/llevents.h b/indra/llcommon/llevents.h index ae6e5aabc9..38adc31121 100644 --- a/indra/llcommon/llevents.h +++ b/indra/llcommon/llevents.h @@ -268,6 +268,43 @@ public: LLEventPump& make(const std::string& name, bool tweak=false, const std::string& type=std::string()); + /// function passed to registerTypeFactory() + typedef std::function TypeFactory; + + /** + * Register a TypeFactory for use with make(). When make() is called with + * the specified @a type string, call @a factory(name, tweak, type) to + * instantiate it. + * + * Returns true if successfully registered, false if there already exists + * a TypeFactory for the specified @a type name. + */ + bool registerTypeFactory(const std::string& type, const TypeFactory& factory); + + /// function passed to registerPumpFactory() + typedef std::function PumpFactory; + + /** + * Register a PumpFactory for use with obtain(). When obtain() is called + * with the specified @a name string, if an LLEventPump with the specified + * @a name doesn't already exist, call @a factory(name) to instantiate it. + * + * Returns true if successfully registered, false if there already exists + * a factory override for the specified @a name. + * + * PumpFactory does not support @a tweak because it's only called when + * that particular @a name is passed to obtain(). Bear in mind that + * obtain(name) might still bypass the caller's PumpFactory for a + * couple different reasons: + * + * * registerPumpFactory() returns false because there's already a factory + * override for the specified @name + * * between a successful registerPumpFactory(name) call (returns + * true) and a call to obtain(name), someone explicitly + * instantiated an LLEventPump(name), so obtain(name) returned that. + */ + bool registerPumpFactory(const std::string& name, const PumpFactory& factory); + /** * Find the named LLEventPump instance. If it exists post the message to it. * If the pump does not exist, do nothing. @@ -325,7 +362,7 @@ testable: typedef std::set PumpSet; PumpSet mOurPumps; // for make(), map string type name to LLEventPump subclass factory function - typedef std::map> PumpFactories; + typedef std::map PumpFactories; // Data used by make(). // One might think mFactories and mTypes could reasonably be static. So // they could -- if not for the fact that make() or obtain() might be -- cgit v1.2.3 From b26e516d2b93a442d09f5c3b1b4d8d60139c42f5 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 6 Apr 2022 17:34:28 -0400 Subject: DRTVWR-558: Change LLEventDispatcher error action (also LLEventAPI). Originally the LLEventAPI mechanism was primarily used for VITA testing. In that case it was okay for the viewer to crash with LL_ERRS if the test script passed a bad request. With puppetry, hopefully new LEAP scripts will be written to engage LLEventAPIs in all sorts of interesting ways. Change error handling from LL_ERRS to LL_WARNS. Furthermore, if the incoming request contains a "reply" key, send back an error response to the requester. Update lleventdispatcher_test.cpp accordingly. (cherry picked from commit de0539fcbe815ceec2041ecc9981e3adf59f2806) --- indra/llcommon/lleventdispatcher.cpp | 127 +++++++++++++++++------- indra/llcommon/lleventdispatcher.h | 29 ++++-- indra/llcommon/tests/lleventdispatcher_test.cpp | 62 ++++++++---- 3 files changed, 152 insertions(+), 66 deletions(-) (limited to 'indra') diff --git a/indra/llcommon/lleventdispatcher.cpp b/indra/llcommon/lleventdispatcher.cpp index 5b6d4efbe9..742d6cf51f 100644 --- a/indra/llcommon/lleventdispatcher.cpp +++ b/indra/llcommon/lleventdispatcher.cpp @@ -40,15 +40,24 @@ // other Linden headers #include "llevents.h" #include "llerror.h" +#include "llexception.h" #include "llsdutil.h" #include "stringize.h" #include // std::auto_ptr +/***************************************************************************** +* DispatchError +*****************************************************************************/ +struct DispatchError: public LLException +{ + DispatchError(const std::string& what): LLException(what) {} +}; + /***************************************************************************** * LLSDArgsSource *****************************************************************************/ /** - * Store an LLSD array, producing its elements one at a time. Die with LL_ERRS + * Store an LLSD array, producing its elements one at a time. It is an error * if the consumer requests more elements than the array contains. */ class LL_COMMON_API LLSDArgsSource @@ -74,8 +83,7 @@ LLSDArgsSource::LLSDArgsSource(const std::string function, const LLSD& args): { if (! (_args.isUndefined() || _args.isArray())) { - LL_ERRS("LLSDArgsSource") << _function << " needs an args array instead of " - << _args << LL_ENDL; + LLTHROW(DispatchError(stringize(_function, " needs an args array instead of ", _args))); } } @@ -88,8 +96,8 @@ LLSD LLSDArgsSource::next() { if (_index >= _args.size()) { - LL_ERRS("LLSDArgsSource") << _function << " requires more arguments than the " - << _args.size() << " provided: " << _args << LL_ENDL; + LLTHROW(DispatchError(stringize(_function, " requires more arguments than the ", + _args.size(), " provided: ", _args))); } return _args[_index++]; } @@ -163,7 +171,8 @@ public: /// default values LLSDArgsMapper(const std::string& function, const LLSD& names, const LLSD& defaults); - /// Given arguments map, return LLSD::Array of parameter values, or LL_ERRS. + /// Given arguments map, return LLSD::Array of parameter values, or + /// trigger error. LLSD map(const LLSD& argsmap) const; private: @@ -195,7 +204,7 @@ LLSDArgsMapper::LLSDArgsMapper(const std::string& function, { if (! (_names.isUndefined() || _names.isArray())) { - LL_ERRS("LLSDArgsMapper") << function << " names must be an array, not " << names << LL_ENDL; + LLTHROW(DispatchError(stringize(function, " names must be an array, not ", names))); } LLSD::Integer nparams(_names.size()); // From _names generate _indexes. @@ -218,8 +227,8 @@ LLSDArgsMapper::LLSDArgsMapper(const std::string& function, // defaults is a (possibly empty) array. Right-align it with names. if (ndefaults > nparams) { - LL_ERRS("LLSDArgsMapper") << function << " names array " << names - << " shorter than defaults array " << defaults << LL_ENDL; + LLTHROW(DispatchError(stringize(function, " names array ", names, + " shorter than defaults array ", defaults))); } // Offset by which we slide defaults array right to right-align with @@ -256,14 +265,14 @@ LLSDArgsMapper::LLSDArgsMapper(const std::string& function, } if (bogus.size()) { - LL_ERRS("LLSDArgsMapper") << function << " defaults specified for nonexistent params " - << formatlist(bogus) << LL_ENDL; + LLTHROW(DispatchError(stringize(function, " defaults specified for nonexistent params ", + formatlist(bogus)))); } } else { - LL_ERRS("LLSDArgsMapper") << function << " defaults must be a map or an array, not " - << defaults << LL_ENDL; + LLTHROW(DispatchError(stringize(function, " defaults must be a map or an array, not ", + defaults))); } } @@ -271,8 +280,8 @@ LLSD LLSDArgsMapper::map(const LLSD& argsmap) const { if (! (argsmap.isUndefined() || argsmap.isMap() || argsmap.isArray())) { - LL_ERRS("LLSDArgsMapper") << _function << " map() needs a map or array, not " - << argsmap << LL_ENDL; + LLTHROW(DispatchError(stringize(_function, " map() needs a map or array, not ", + argsmap))); } // Initialize the args array. Indexing a non-const LLSD array grows it // to appropriate size, but we don't want to resize this one on each @@ -369,8 +378,8 @@ LLSD LLSDArgsMapper::map(const LLSD& argsmap) const // by argsmap, that's a problem. if (unfilled.size()) { - LL_ERRS("LLSDArgsMapper") << _function << " missing required arguments " - << formatlist(unfilled) << " from " << argsmap << LL_ENDL; + LLTHROW(DispatchError(stringize(_function, " missing required arguments ", + formatlist(unfilled), " from ", argsmap))); } // done @@ -420,7 +429,7 @@ struct LLEventDispatcher::LLSDDispatchEntry: public LLEventDispatcher::DispatchE std::string mismatch(llsd_matches(mRequired, event)); if (! mismatch.empty()) { - LL_ERRS("LLEventDispatcher") << desc << ": bad request: " << mismatch << LL_ENDL; + LLTHROW(DispatchError(stringize(desc, ": bad request: ", mismatch))); } // Event syntax looks good, go for it! mFunc(event); @@ -596,48 +605,90 @@ bool LLEventDispatcher::remove(const std::string& name) return true; } -/// Call a registered callable with an explicitly-specified name. If no -/// such callable exists, die with LL_ERRS. +/// Call a registered callable with an explicitly-specified name. It is an +/// error if no such callable exists. void LLEventDispatcher::operator()(const std::string& name, const LLSD& event) const { - if (! try_call(name, event)) + std::string error{ try_call_log(std::string(), name, event) }; + if (! error.empty()) { - LL_ERRS("LLEventDispatcher") << "LLEventDispatcher(" << mDesc << "): '" << name - << "' not found" << LL_ENDL; + callFail(event, error); } } -/// Extract the @a key value from the incoming @a event, and call the -/// callable whose name is specified by that map @a key. If no such -/// callable exists, die with LL_ERRS. +/// Extract the @a key value from the incoming @a event, and call the callable +/// whose name is specified by that map @a key. It is an error if no such +/// callable exists. void LLEventDispatcher::operator()(const LLSD& event) const { - // This could/should be implemented in terms of the two-arg overload. - // However -- we can produce a more informative error message. - std::string name(event[mKey]); - if (! try_call(name, event)) + std::string error{ try_call_log(mKey, event[mKey], event) }; + if (! error.empty()) + { + callFail(event, error); + } +} + +void LLEventDispatcher::callFail(const LLSD& event, const std::string& msg) const +{ + static LLSD::String key{ "reply" }; + if (event.has(key)) { - LL_ERRS("LLEventDispatcher") << "LLEventDispatcher(" << mDesc << "): bad " << mKey - << " value '" << name << "'" << LL_ENDL; + // Oh good, the incoming event specifies a reply pump -- pass back a + // response that includes an "error" key with the message. + sendReply(llsd::map("error", msg), event, key); } } bool LLEventDispatcher::try_call(const LLSD& event) const { - return try_call(event[mKey], event); + return try_call_log(mKey, event[mKey], event).empty(); } bool LLEventDispatcher::try_call(const std::string& name, const LLSD& event) const +{ + return try_call_log(std::string(), name, event).empty(); +} + +std::string LLEventDispatcher::try_call_log(const std::string& key, const std::string& name, + const LLSD& event) const +{ + std::string error{ try_call(key, name, event) }; + if (! error.empty()) + { + LL_WARNS("LLEventDispatcher") << error << LL_ENDL; + } + return error; +} + +// This internal method returns empty string if the call succeeded, else +// non-empty error message. +std::string LLEventDispatcher::try_call(const std::string& key, const std::string& name, + const LLSD& event) const { DispatchMap::const_iterator found = mDispatch.find(name); if (found == mDispatch.end()) { - return false; + if (key.empty()) + { + return stringize("LLEventDispatcher(", mDesc, "): '", name, "' not found"); + } + else + { + return stringize("LLEventDispatcher(", mDesc, "): bad ", key, " value '", name, "'"); + } + } + + try + { + // Found the name, so it's plausible to even attempt the call. + found->second->call(stringize("LLEventDispatcher(", mDesc, ") calling '", name, "'"), + event); + } + catch (const DispatchError& err) + { + return err.what(); } - // Found the name, so it's plausible to even attempt the call. - found->second->call(STRINGIZE("LLEventDispatcher(" << mDesc << ") calling '" << name << "'"), - event); - return true; // tell caller we were able to call + return {}; // tell caller we were able to call } LLSD LLEventDispatcher::getMetadata(const std::string& name) const diff --git a/indra/llcommon/lleventdispatcher.h b/indra/llcommon/lleventdispatcher.h index 9e1244ef5b..b78c77f8b3 100644 --- a/indra/llcommon/lleventdispatcher.h +++ b/indra/llcommon/lleventdispatcher.h @@ -254,28 +254,28 @@ public: /// Unregister a callable bool remove(const std::string& name); - /// Call a registered callable with an explicitly-specified name. If no - /// such callable exists, die with LL_ERRS. If the @a event fails to match - /// the @a required prototype specified at add() time, die with LL_ERRS. + /// Call a registered callable with an explicitly-specified name. It is an + /// error if no such callable exists. It is an error if the @a event fails + /// to match the @a required prototype specified at add() time. void operator()(const std::string& name, const LLSD& event) const; /// Call a registered callable with an explicitly-specified name and /// return true. If no such callable exists, return - /// false. If the @a event fails to match the @a required - /// prototype specified at add() time, die with LL_ERRS. + /// false. It is an error if the @a event fails to match the @a + /// required prototype specified at add() time. bool try_call(const std::string& name, const LLSD& event) const; /// Extract the @a key value from the incoming @a event, and call the - /// callable whose name is specified by that map @a key. If no such - /// callable exists, die with LL_ERRS. If the @a event fails to match the - /// @a required prototype specified at add() time, die with LL_ERRS. + /// callable whose name is specified by that map @a key. It is an error if + /// no such callable exists. It is an error if the @a event fails to match + /// the @a required prototype specified at add() time. void operator()(const LLSD& event) const; /// Extract the @a key value from the incoming @a event, call the callable /// whose name is specified by that map @a key and return true. - /// If no such callable exists, return false. If the @a event - /// fails to match the @a required prototype specified at add() time, die - /// with LL_ERRS. + /// If no such callable exists, return false. It is an error if + /// the @a event fails to match the @a required prototype specified at + /// add() time. bool try_call(const LLSD& event) const; /// @name Iterate over defined names @@ -340,6 +340,13 @@ private: } } void addFail(const std::string& name, const std::string& classname) const; + std::string try_call_log(const std::string& key, const std::string& name, + const LLSD& event) const; + std::string try_call(const std::string& key, const std::string& name, + const LLSD& event) const; + // Implement "it is an error" semantics for attempted call operations: if + // the incoming event includes a "reply" key, log and send an error reply. + void callFail(const LLSD& event, const std::string& msg) const; std::string mDesc, mKey; DispatchMap mDispatch; diff --git a/indra/llcommon/tests/lleventdispatcher_test.cpp b/indra/llcommon/tests/lleventdispatcher_test.cpp index 9da1ecfd67..82a0ddf61b 100644 --- a/indra/llcommon/tests/lleventdispatcher_test.cpp +++ b/indra/llcommon/tests/lleventdispatcher_test.cpp @@ -20,6 +20,7 @@ #include "../test/lltut.h" #include "llsd.h" #include "llsdutil.h" +#include "llevents.h" #include "stringize.h" #include "tests/wrapllerrs.h" #include "../test/catch_and_store_what_in.h" @@ -644,12 +645,45 @@ namespace tut outer.find(inner) != std::string::npos); } - void call_exc(const std::string& func, const LLSD& args, const std::string& exc_frag) + std::string call_exc(const std::string& func, const LLSD& args, const std::string& exc_frag) { - std::string threw = catch_what([this, &func, &args](){ - work(func, args); - }); - ensure_has(threw, exc_frag); + // This method was written when LLEventDispatcher responded to + // name or argument errors with LL_ERRS, hence the name: we used + // to have to intercept LL_ERRS by making it throw. Now we set up + // to catch an error response instead. But -- for that we need to + // be able to sneak a "reply" key into args, which must be a Map. + if (! (args.isUndefined() or args.isMap())) + fail(stringize("can't test call_exc() with ", args)); + LLEventStream replypump("reply"); + LLSD reply; + LLTempBoundListener bound{ + replypump.listen( + "listener", + [&reply](const LLSD& event) + { + reply = event; + return false; + }) }; + LLSD modargs{ args }; + modargs["reply"] = replypump.getName(); + if (func.empty()) + { + work(modargs); + } + else + { + work(func, modargs); + } + ensure("no error response", reply.has("error")); + ensure_has(reply["error"], exc_frag); + return reply["error"]; + } + + void call_logerr(const std::string& func, const LLSD& args, const std::string& frag) + { + CaptureLog capture; + work(func, args); + capture.messageWith(frag); } LLSD getMetadata(const std::string& name) @@ -1031,13 +1065,7 @@ namespace tut { set_test_name("call with bad name"); call_exc("freek", LLSD(), "not found"); - // We don't have a comparable helper function for the one-arg - // operator() method, and it's not worth building one just for this - // case. Write it out. - std::string threw = catch_what([this](){ - work(LLSDMap("op", "freek")); - }); - ensure_has(threw, "bad"); + std::string threw = call_exc("", LLSDMap("op", "freek"), "bad"); ensure_has(threw, "op"); ensure_has(threw, "freek"); } @@ -1087,7 +1115,7 @@ namespace tut ensure_equals("answer mismatch", tr.llsd, answer); // Should NOT be able to pass 'answer' to Callables registered // with 'required'. - call_exc(tr.name_req, answer, "bad request"); + call_logerr(tr.name_req, answer, "bad request"); // But SHOULD be able to pass 'matching' to Callables registered // with 'required'. work(tr.name_req, matching); @@ -1107,11 +1135,11 @@ namespace tut // args. We should only need to engage it for one map-style // registration and one array-style registration. std::string array_exc("needs an args array"); - call_exc("free0_array", 17, array_exc); - call_exc("free0_array", LLSDMap("pi", 3.14), array_exc); + call_logerr("free0_array", 17, array_exc); + call_logerr("free0_array", LLSDMap("pi", 3.14), array_exc); std::string map_exc("needs a map"); - call_exc("free0_map", 17, map_exc); + call_logerr("free0_map", 17, map_exc); // Passing an array to a map-style function works now! No longer an // error case! // call_exc("free0_map", LLSDArray("a")("b"), map_exc); @@ -1158,7 +1186,7 @@ namespace tut { foreach(const llsd::MapEntry& e, inMap(funcsab)) { - call_exc(e.second, tooshort, "requires more arguments"); + call_logerr(e.second, tooshort, "requires more arguments"); } } } -- cgit v1.2.3 From f134eace911eca8a231a7c1d556d7d891e8b21ca Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 6 Apr 2022 21:49:03 -0400 Subject: DRTVWR-558: LLEventAPI allows all LLEventDispatcher add() overloads. Previously, LLEventAPI intentionally hid all but one of the many add() overloads supported by its LLEventDispatcher base class. The reason was that certain of the add() methods take an optional fourth parameter that's an LLSD::Map describing the expected parameter structure, while others take a fourth templated parameter that's an instance getter callable. This led to ambiguity, especially when passed an LLSDMap instance that's convertible to LLSD but isn't literally LLSD. At the time, it was simpler to constrain the add() methods inherited from LLEventDispatcher. But by adding new std::enable_if constraints to certain LLEventDispatcher add() methods, we've resolved the ambiguities, so LLEventAPI subclasses can now use any add() overload (as claimed on the relevant Confluence page). LLEventDispatcher comments have always loftily claimed that an instance getter callable may return either a pointer or a reference, doesn't matter. But it does when trying to pass the getter's result to boost::fusion::push_back(): a reference must be wrapped with std::ref() while a pointer cannot be. std::ref(pointer) produces errors. Introduce LLEventDispatcher::invoker:: bindable() overloads to Do The Right Thing whether passed a pointer or a reference. (cherry picked from commit 743f487c2e123171c9fc6d5b84d768f1d856d569) --- indra/llcommon/lleventapi.h | 13 ------ indra/llcommon/lleventdispatcher.h | 89 ++++++++++++++++++++++++++------------ 2 files changed, 61 insertions(+), 41 deletions(-) (limited to 'indra') diff --git a/indra/llcommon/lleventapi.h b/indra/llcommon/lleventapi.h index 5991fe8fd5..ed62fa064a 100644 --- a/indra/llcommon/lleventapi.h +++ b/indra/llcommon/lleventapi.h @@ -64,19 +64,6 @@ public: /// Get the documentation string std::string getDesc() const { return mDesc; } - /** - * Publish only selected add() methods from LLEventDispatcher. - * Every LLEventAPI add() @em must have a description string. - */ - template - void add(const std::string& name, - const std::string& desc, - CALLABLE callable, - const LLSD& required=LLSD()) - { - LLEventDispatcher::add(name, desc, callable, required); - } - /** * Instantiate a Response object in any LLEventAPI subclass method that * wants to guarantee a reply (if requested) will be sent on exit from the diff --git a/indra/llcommon/lleventdispatcher.h b/indra/llcommon/lleventdispatcher.h index b78c77f8b3..f1e4fe2df7 100644 --- a/indra/llcommon/lleventdispatcher.h +++ b/indra/llcommon/lleventdispatcher.h @@ -61,7 +61,6 @@ static const auto& nil(nil_); #include #include #include -#include #include #include #include @@ -167,10 +166,11 @@ public: * converted to the corresponding parameter type using LLSDParam. */ template - typename boost::enable_if< boost::function_types::is_nonmember_callable_builtin - >::type add(const std::string& name, - const std::string& desc, - Function f); + typename std::enable_if< + boost::function_types::is_nonmember_callable_builtin::value + >::type add(const std::string& name, + const std::string& desc, + Function f); /** * Register a nonstatic class method with arbitrary parameters. @@ -190,11 +190,14 @@ public: * converted to the corresponding parameter type using LLSDParam. */ template - typename boost::enable_if< boost::function_types::is_member_function_pointer - >::type add(const std::string& name, - const std::string& desc, - Method f, - const InstanceGetter& getter); + typename std::enable_if< + boost::function_types::is_member_function_pointer::value && + ! std::is_same::value && + ! std::is_same::value + >::type add(const std::string& name, + const std::string& desc, + Method f, + const InstanceGetter& getter); /** * Register a free function with arbitrary parameters. (This also works @@ -212,12 +215,13 @@ public: * to the corresponding parameter type using LLSDParam. */ template - typename boost::enable_if< boost::function_types::is_nonmember_callable_builtin - >::type add(const std::string& name, - const std::string& desc, - Function f, - const LLSD& params, - const LLSD& defaults=LLSD()); + typename std::enable_if< + boost::function_types::is_nonmember_callable_builtin::value + >::type add(const std::string& name, + const std::string& desc, + Function f, + const LLSD& params, + const LLSD& defaults=LLSD()); /** * Register a nonstatic class method with arbitrary parameters. @@ -241,13 +245,16 @@ public: * to the corresponding parameter type using LLSDParam. */ template - typename boost::enable_if< boost::function_types::is_member_function_pointer - >::type add(const std::string& name, - const std::string& desc, - Method f, - const InstanceGetter& getter, - const LLSD& params, - const LLSD& defaults=LLSD()); + typename std::enable_if< + boost::function_types::is_member_function_pointer::value && + ! std::is_same::value && + ! std::is_same::value + >::type add(const std::string& name, + const std::string& desc, + Method f, + const InstanceGetter& getter, + const LLSD& params, + const LLSD& defaults=LLSD()); //@} @@ -434,7 +441,25 @@ struct LLEventDispatcher::invoker // Instead of grabbing the first item from argsrc and making an // LLSDParam of it, call getter() and pass that as the instance param. invoker::apply - ( func, argsrc, boost::fusion::push_back(boost::fusion::nil(), boost::ref(getter()))); + ( func, argsrc, boost::fusion::push_back(boost::fusion::nil(), bindable(getter()))); + } + + template + static inline + auto bindable(T&& value, + typename std::enable_if::value, bool>::type=true) + { + // if passed a pointer, just return that pointer + return std::forward(value); + } + + template + static inline + auto bindable(T&& value, + typename std::enable_if::value, bool>::type=true) + { + // if passed a reference, wrap it for binding + return std::ref(std::forward(value)); } }; @@ -454,7 +479,7 @@ struct LLEventDispatcher::invoker }; template -typename boost::enable_if< boost::function_types::is_nonmember_callable_builtin >::type +typename std::enable_if< boost::function_types::is_nonmember_callable_builtin::value >::type LLEventDispatcher::add(const std::string& name, const std::string& desc, Function f) { // Construct an invoker_function, a callable accepting const args_source&. @@ -465,7 +490,11 @@ LLEventDispatcher::add(const std::string& name, const std::string& desc, Functio } template -typename boost::enable_if< boost::function_types::is_member_function_pointer >::type +typename std::enable_if< + boost::function_types::is_member_function_pointer::value && + ! std::is_same::value && + ! std::is_same::value +>::type LLEventDispatcher::add(const std::string& name, const std::string& desc, Method f, const InstanceGetter& getter) { @@ -476,7 +505,7 @@ LLEventDispatcher::add(const std::string& name, const std::string& desc, Method } template -typename boost::enable_if< boost::function_types::is_nonmember_callable_builtin >::type +typename std::enable_if< boost::function_types::is_nonmember_callable_builtin::value >::type LLEventDispatcher::add(const std::string& name, const std::string& desc, Function f, const LLSD& params, const LLSD& defaults) { @@ -485,7 +514,11 @@ LLEventDispatcher::add(const std::string& name, const std::string& desc, Functio } template -typename boost::enable_if< boost::function_types::is_member_function_pointer >::type +typename std::enable_if< + boost::function_types::is_member_function_pointer::value && + ! std::is_same::value && + ! std::is_same::value +>::type LLEventDispatcher::add(const std::string& name, const std::string& desc, Method f, const InstanceGetter& getter, const LLSD& params, const LLSD& defaults) -- cgit v1.2.3 From 4e7b4bab79be8cf2de9af242e5cd23347fba8bb2 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 6 Apr 2022 22:19:10 -0400 Subject: DRTVWR-558: Generalize LLEventDispatcher::add() constraints. Instead of checking whether an add() parameter is exactly LLSD or LLSDMap, check whether it's convertible to LLSD -- which handles those cases and more. (cherry picked from commit fa168c11f64771dadc5df86d14ca2f07eba3b8ba) --- indra/llcommon/lleventdispatcher.h | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) (limited to 'indra') diff --git a/indra/llcommon/lleventdispatcher.h b/indra/llcommon/lleventdispatcher.h index f1e4fe2df7..1b3e834aeb 100644 --- a/indra/llcommon/lleventdispatcher.h +++ b/indra/llcommon/lleventdispatcher.h @@ -192,8 +192,7 @@ public: template typename std::enable_if< boost::function_types::is_member_function_pointer::value && - ! std::is_same::value && - ! std::is_same::value + ! std::is_convertible::value >::type add(const std::string& name, const std::string& desc, Method f, @@ -247,8 +246,7 @@ public: template typename std::enable_if< boost::function_types::is_member_function_pointer::value && - ! std::is_same::value && - ! std::is_same::value + ! std::is_convertible::value >::type add(const std::string& name, const std::string& desc, Method f, @@ -492,8 +490,7 @@ LLEventDispatcher::add(const std::string& name, const std::string& desc, Functio template typename std::enable_if< boost::function_types::is_member_function_pointer::value && - ! std::is_same::value && - ! std::is_same::value + ! std::is_convertible::value >::type LLEventDispatcher::add(const std::string& name, const std::string& desc, Method f, const InstanceGetter& getter) @@ -516,8 +513,7 @@ LLEventDispatcher::add(const std::string& name, const std::string& desc, Functio template typename std::enable_if< boost::function_types::is_member_function_pointer::value && - ! std::is_same::value && - ! std::is_same::value + ! std::is_convertible::value >::type LLEventDispatcher::add(const std::string& name, const std::string& desc, Method f, const InstanceGetter& getter, -- cgit v1.2.3 From dc2e2cd76f387ea6e80787fb94adcbc269cd1f25 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Sat, 18 Jun 2022 11:53:57 -0400 Subject: DRTVWR-564: Add LL::apply(): call function, passing args from tuple. This anticipates C++17's std::apply(), and in fact once we detect C++17, we'll just use that. But in C++14 we must still provide our own implementation. --- indra/llcommon/apply.h | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) create mode 100644 indra/llcommon/apply.h (limited to 'indra') diff --git a/indra/llcommon/apply.h b/indra/llcommon/apply.h new file mode 100644 index 0000000000..ef4a8fd68b --- /dev/null +++ b/indra/llcommon/apply.h @@ -0,0 +1,51 @@ +/** + * @file apply.h + * @author Nat Goodspeed + * @date 2022-06-18 + * @brief C++14 version of std::apply() + * + * $LicenseInfo:firstyear=2022&license=viewerlgpl$ + * Copyright (c) 2022, Linden Research, Inc. + * $/LicenseInfo$ + */ + +#if ! defined(LL_APPLY_H) +#define LL_APPLY_H + +#include + +namespace LL +{ + +#if __cplusplus >= 201703L + +// C++17 implementation +using std::apply; + +#else // C++14 + +// Derived from https://stackoverflow.com/a/20441189 +// and https://en.cppreference.com/w/cpp/utility/apply +template +auto apply_impl(CALLABLE&& func, TUPLE&& args, std::index_sequence) +{ + // call func(unpacked args) + return std::forward(func)(std::move(std::get(args))...); +} + +template +auto apply(CALLABLE&& func, std::tuple&& args) +{ + // std::index_sequence_for is the magic sauce here, generating an argument + // pack of indexes for each entry in args. apply_impl() can then pass + // those to std::get() to unpack args into individual arguments. + return apply_impl(std::forward(func), + std::forward>(args), + std::index_sequence_for{}); +} + +#endif // C++14 + +} // namespace LL + +#endif /* ! defined(LL_APPLY_H) */ -- cgit v1.2.3 From af4fbc1f8a99a3c5370cb6db45435e67f9ce92d2 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Sat, 18 Jun 2022 11:57:10 -0400 Subject: DRTVWR-564: WIP: Add LazyEventAPI and tests. Tests don't yet pass. LazyEventAPI is a registrar that implicitly instantiates some particular LLEventAPI subclass on demand: that is, when LLEventPumps::obtain() tries to find an LLEventPump by the registered name. This leverages the new LLEventPumps::registerPumpFactory() machinery. Fix registerPumpFactory() to adapt the passed PumpFactory to accept TypeFactory parameters (two of which it ignores). Supplement it with unregisterPumpFactory() to support LazyEventAPI instances with lifespans shorter than the process -- which may be mostly test programs, but still a hole worth closing. Similarly, add unregisterTypeFactory(). A LazyEventAPI subclass takes over responsibility for specifying the LLEventAPI's name, desc, field, plus whatever add() calls will be needed to register the LLEventAPI's operations. This is so we can (later) enhance LLLeapListener to consult LazyEventAPI instances for not-yet-instantiated LLEventAPI metadata, as well as enumerating existing LLEventAPI instances. The trickiest part of this is capturing calls to the various LLEventDispatcher::add() overloads in such a way that, when the LLEventAPI subclass is eventually instantiated, we can replay them in the new instance. LLEventAPI acquires a new protected constructor specifically for use by a subclass registered by a companion LazyEventAPI. It accepts a const reference to LazyEventAPIParams, intended to be opaque to the LLEventAPI subclass; the subclass must declare a constructor that accepts and forwards the parameter block to the new LLEventAPI constructor. The implementation delegates to the existing LLEventAPI constructor, plus it runs deferred add() calls. LLDispatchListener now derives from LLEventStream instead of containing it as a data member. The reason is that if LLEventPumps::obtain() implicitly instantiates it, LLEventPumps's destructor will try to destroy it by deleting the LLEventPump*. If the LLEventPump returned by the factory function is a data member of an outer class, that won't work so well. But if LLDispatchListener (and by implication, LLEventAPI and any subclass) is derived from LLEventPump, then the virtual destructor will Do The Right Thing. Change LLDispatchListener to *not* allow tweaking the LLEventPump name. Since the overwhelming use case for LLDispatchListener is LLEventAPI, accepting but silently renaming an LLEventAPI subclass would ensure nobody could reach it. Change LLEventDispatcher's use of std::enable_if to control the set of add() overloads available for the intended use cases. Apparently this formulation is just as functional at the method declaration point, while avoiding the need to restate the whole enable_if expression at the method definition point. Add lazyeventapi_test.cpp to exercise. --- indra/llcommon/CMakeLists.txt | 4 + indra/llcommon/lazyeventapi.cpp | 53 ++++++++ indra/llcommon/lazyeventapi.h | 204 +++++++++++++++++++++++++++++ indra/llcommon/lleventapi.cpp | 8 ++ indra/llcommon/lleventapi.h | 23 +++- indra/llcommon/lleventdispatcher.cpp | 13 +- indra/llcommon/lleventdispatcher.h | 105 +++++++-------- indra/llcommon/llevents.cpp | 29 +++- indra/llcommon/llevents.h | 6 +- indra/llcommon/tests/lazyeventapi_test.cpp | 89 +++++++++++++ 10 files changed, 466 insertions(+), 68 deletions(-) create mode 100644 indra/llcommon/lazyeventapi.cpp create mode 100644 indra/llcommon/lazyeventapi.h create mode 100644 indra/llcommon/tests/lazyeventapi_test.cpp (limited to 'indra') diff --git a/indra/llcommon/CMakeLists.txt b/indra/llcommon/CMakeLists.txt index ca8b5e946f..36b2e09dc5 100644 --- a/indra/llcommon/CMakeLists.txt +++ b/indra/llcommon/CMakeLists.txt @@ -30,6 +30,7 @@ include_directories( set(llcommon_SOURCE_FILES indra_constants.cpp + lazyeventapi.cpp llallocator.cpp llallocator_heap_profile.cpp llapp.cpp @@ -128,10 +129,12 @@ set(llcommon_SOURCE_FILES set(llcommon_HEADER_FILES CMakeLists.txt + apply.h chrono.h ctype_workaround.h fix_macros.h indra_constants.h + lazyeventapi.h linden_common.h llalignedarray.h llallocator.h @@ -338,6 +341,7 @@ if (LL_TESTS) ${BOOST_SYSTEM_LIBRARY}) LL_ADD_INTEGRATION_TEST(commonmisc "" "${test_libs}") LL_ADD_INTEGRATION_TEST(bitpack "" "${test_libs}") + LL_ADD_INTEGRATION_TEST(lazyeventapi "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llbase64 "" "${test_libs}") LL_ADD_INTEGRATION_TEST(llcond "" "${test_libs}") LL_ADD_INTEGRATION_TEST(lldate "" "${test_libs}") diff --git a/indra/llcommon/lazyeventapi.cpp b/indra/llcommon/lazyeventapi.cpp new file mode 100644 index 0000000000..aefc2db6da --- /dev/null +++ b/indra/llcommon/lazyeventapi.cpp @@ -0,0 +1,53 @@ +/** + * @file lazyeventapi.cpp + * @author Nat Goodspeed + * @date 2022-06-17 + * @brief Implementation for lazyeventapi. + * + * $LicenseInfo:firstyear=2022&license=viewerlgpl$ + * Copyright (c) 2022, Linden Research, Inc. + * $/LicenseInfo$ + */ + +// Precompiled header +#include "linden_common.h" +// associated header +#include "lazyeventapi.h" +// STL headers +// std headers +// external library headers +// other Linden headers +#include "llevents.h" + +LL::LazyEventAPIBase::LazyEventAPIBase( + const std::string& name, const std::string& desc, const std::string& field) +{ + // populate embedded LazyEventAPIParams instance + mParams.name = name; + mParams.desc = desc; + mParams.field = field; + // mParams.init and mOperations are populated by subsequent add() calls. + + // Our raison d'etre: register as an LLEventPumps::PumpFactory + // so obtain() will notice any request for this name and call us. + // Of course, our subclass constructor must finish running (making add() + // calls) before mParams will be fully populated, but we expect that to + // happen well before the first LLEventPumps::obtain(name) call. + mRegistered = LLEventPumps::instance().registerPumpFactory( + name, + [this](const std::string& name){ return construct(name); }); +} + +LL::LazyEventAPIBase::~LazyEventAPIBase() +{ + // If our constructor's registerPumpFactory() call was unsuccessful, that + // probably means somebody else claimed the name first. If that's the + // case, do NOT unregister their name out from under them! + // If this is a static instance being destroyed at process shutdown, + // LLEventPumps will probably have been cleaned up already. + if (mRegistered && ! LLEventPumps::wasDeleted()) + { + // unregister the callback to this doomed instance + LLEventPumps::instance().unregisterPumpFactory(mParams.name); + } +} diff --git a/indra/llcommon/lazyeventapi.h b/indra/llcommon/lazyeventapi.h new file mode 100644 index 0000000000..2e947899dc --- /dev/null +++ b/indra/llcommon/lazyeventapi.h @@ -0,0 +1,204 @@ +/** + * @file lazyeventapi.h + * @author Nat Goodspeed + * @date 2022-06-16 + * @brief Declaring a static module-scope LazyEventAPI registers a specific + * LLEventAPI for future on-demand instantiation. + * + * $LicenseInfo:firstyear=2022&license=viewerlgpl$ + * Copyright (c) 2022, Linden Research, Inc. + * $/LicenseInfo$ + */ + +#if ! defined(LL_LAZYEVENTAPI_H) +#define LL_LAZYEVENTAPI_H + +#include "apply.h" +#include "lleventapi.h" +#include "llinstancetracker.h" +#include +#include +#include +#include // std::pair +#include + +namespace LL +{ + /** + * Bundle params we want to pass to LLEventAPI's protected constructor. We + * package them this way so a subclass constructor can simply forward an + * opaque reference to the LLEventAPI constructor. + */ + // This is a class instead of a plain struct mostly so when we forward- + // declare it we don't have to remember the distinction. + class LazyEventAPIParams + { + public: + // package the parameters used by the normal LLEventAPI constructor + std::string name, desc, field; + // bundle LLEventAPI::add() calls collected by LazyEventAPI::add(), so + // the special LLEventAPI constructor we engage can "play back" those + // add() calls + boost::signals2::signal init; + }; + + // The tricky part is: can we capture a sequence of add() calls in the + // LazyEventAPI subclass constructor and then, in effect, replay those + // add() calls on instantiation of the registered LLEventAPI subclass? so + // we don't have to duplicate the add() calls in both constructors? + + // Derive a subclass from LazyEventAPI. Its constructor must pass + // LazyEventAPI's constructor the name, desc, field params. Moreover the + // constructor body must call add(name, desc, *args) for any of the + // LLEventDispatcher add() methods, referencing the LLEventAPI subclass + // methods. + + // LazyEventAPI will store the name, desc, field params for the overall + // LLEventAPI. It will support a single generic add() call accepting name, + // desc, parameter pack. + + // It will hold a std::vector> for each operation. + // It will make all these strings available to LLLeapListener. + + // Maybe what we want is to store a vector of callables (a + // boost::signals2!) and populate it with lambdas, each of which accepts + // LLEventAPI* and calls the relevant add() method by forwarding exactly + // the name, desc and parameter pack. Then, on constructing the target + // LLEventAPI, we just fire the signal, passing the new instance pointer. + + /** + * LazyEventAPIBase implements most of the functionality of LazyEventAPI + * (q.v.), but we need the LazyEventAPI template subclass so we can accept + * the specific LLEventAPI subclass type. + */ + // No LLInstanceTracker key: we don't need to find a specific instance, + // LLLeapListener just needs to be able to enumerate all instances. + class LazyEventAPIBase: public LLInstanceTracker + { + public: + LazyEventAPIBase(const std::string& name, const std::string& desc, + const std::string& field); + virtual ~LazyEventAPIBase(); + + // Do not copy or move: once constructed, LazyEventAPIBase must stay + // put: we bind its instance pointer into a callback. + LazyEventAPIBase(const LazyEventAPIBase&) = delete; + LazyEventAPIBase(LazyEventAPIBase&&) = delete; + LazyEventAPIBase& operator=(const LazyEventAPIBase&) = delete; + LazyEventAPIBase& operator=(LazyEventAPIBase&&) = delete; + + // actually instantiate the companion LLEventAPI subclass + virtual LLEventPump* construct(const std::string& name) = 0; + + // capture add() calls we want to play back on LLEventAPI construction + template + void add(const std::string& name, const std::string& desc, ARGS&&... rest) + { + // capture the metadata separately + mOperations.push_back(std::make_pair(name, desc)); + // Use connect_extended() so the lambda is passed its own + // connection. + // We can't bind an unexpanded parameter pack into a lambda -- + // shame really. Instead, capture it as a std::tuple and then, in + // the lambda, use apply() to convert back to function args. + mParams.init.connect_extended( + [name, desc, rest = std::make_tuple(std::forward(rest)...)] + (const boost::signals2::connection& conn, LLEventAPI* instance) + { + // we only need this connection once + conn.disconnect(); + // Our add() method distinguishes name and desc because we + // capture them separately. But now, because apply() + // expects a tuple specifying ALL the arguments, expand to + // a tuple including add_trampoline() arguments: instance, + // name, desc, rest. + // apply() can't accept a template per se; it needs a + // particular specialization. + apply(&LazyEventAPIBase::add_trampoline, + std::tuple_cat(std::make_tuple(instance, name, desc), + rest)); + }); + } + + // metadata that might be queried by LLLeapListener + std::vector> mOperations; + // Params with which to instantiate the companion LLEventAPI subclass + LazyEventAPIParams mParams; + + private: + // Passing an overloaded function to any function that accepts an + // arbitrary callable is a PITB because you have to specify the + // correct overload. What we want is for the compiler to select the + // correct overload, based on the carefully-wrought enable_ifs in + // LLEventDispatcher. This (one and only) add_trampoline() method + // exists solely to pass to LL::apply(). Once add_trampoline() is + // called with the expanded arguments, we hope the compiler will Do + // The Right Thing in selecting the correct LLEventAPI::add() + // overload. + template + static + void add_trampoline(LLEventAPI* instance, ARGS&&... args) + { + instance->add(std::forward(args)...); + } + + bool mRegistered; + }; + + /** + * LazyEventAPI provides a way to register a particular LLEventAPI to be + * instantiated on demand, that is, when its name is passed to + * LLEventPumps::obtain(). + * + * Derive your listener from LLEventAPI as usual, with its various + * operation methods, but code your constructor to accept + * (const LL::LazyEventAPIParams& params) + * and forward that reference to (the protected) + * LLEventAPI(const LL::LazyEventAPIParams&) constructor. + * + * Then derive your listener registrar from + * LazyEventAPI. The constructor should + * look very like a traditional LLEventAPI constructor: + * + * * pass (name, desc [, field]) to LazyEventAPI's constructor + * * in the body, make a series of add() calls referencing your LLEventAPI + * subclass methods. + * + * You may use any LLEventAPI::add() methods, that is, any + * LLEventDispatcher::add() methods. But the target methods you pass to + * add() must belong to your LLEventAPI subclass, not the LazyEventAPI + * subclass. + * + * Declare a static instance of your LazyEventAPI listener registrar + * class. When it's constructed at static initialization time, it will + * register your LLEventAPI subclass with LLEventPumps. It will also + * collect metadata for the LLEventAPI and its operations to provide to + * LLLeapListener's introspection queries. + * + * When someone later calls LLEventPumps::obtain() to post an event to + * your LLEventAPI subclass, obtain() will instantiate it using + * LazyEventAPI's name, desc, field and add() calls. + */ + template + class LazyEventAPI: public LazyEventAPIBase + { + public: + // for subclass constructor to reference handler methods + using listener = EVENTAPI; + + LazyEventAPI(const std::string& name, const std::string& desc, + const std::string& field="op"): + // Forward ctor params to LazyEventAPIBase + LazyEventAPIBase(name, desc, field) + {} + + LLEventPump* construct(const std::string& /*name*/) override + { + // base class has carefully assembled LazyEventAPIParams embedded + // in this instance, just pass to LLEventAPI subclass constructor + return new EVENTAPI(mParams); + } + }; +} // namespace LL + +#endif /* ! defined(LL_LAZYEVENTAPI_H) */ diff --git a/indra/llcommon/lleventapi.cpp b/indra/llcommon/lleventapi.cpp index ff5459c1eb..3d46ef1034 100644 --- a/indra/llcommon/lleventapi.cpp +++ b/indra/llcommon/lleventapi.cpp @@ -35,6 +35,7 @@ // external library headers // other Linden headers #include "llerror.h" +#include "lazyeventapi.h" LLEventAPI::LLEventAPI(const std::string& name, const std::string& desc, const std::string& field): lbase(name, field), @@ -43,6 +44,13 @@ LLEventAPI::LLEventAPI(const std::string& name, const std::string& desc, const s { } +LLEventAPI::LLEventAPI(const LL::LazyEventAPIParams& params): + LLEventAPI(params.name, params.desc, params.field) +{ + // call initialization functions with our brand-new instance pointer + params.init(this); +} + LLEventAPI::~LLEventAPI() { } diff --git a/indra/llcommon/lleventapi.h b/indra/llcommon/lleventapi.h index ed62fa064a..a019458553 100644 --- a/indra/llcommon/lleventapi.h +++ b/indra/llcommon/lleventapi.h @@ -35,6 +35,13 @@ #include "llinstancetracker.h" #include +namespace LL +{ + template + class LazyEventAPI; + class LazyEventAPIParams; +} + /** * LLEventAPI not only provides operation dispatch functionality, inherited * from LLDispatchListener -- it also gives us event API introspection. @@ -45,6 +52,8 @@ class LL_COMMON_API LLEventAPI: public LLDispatchListener, { typedef LLDispatchListener lbase; typedef LLInstanceTracker ibase; + template + friend class LL::LazyEventAPI; public: @@ -137,16 +146,20 @@ public: * @endcode */ LLSD& operator[](const LLSD::String& key) { return mResp[key]; } - - /** - * set the response to the given data - */ - void setResponse(LLSD const & response){ mResp = response; } + + /** + * set the response to the given data + */ + void setResponse(LLSD const & response){ mResp = response; } LLSD mResp, mReq; LLSD::String mKey; }; +protected: + // constructor used only by subclasses registered by LazyEventAPI + LLEventAPI(const LL::LazyEventAPIParams&); + private: std::string mDesc; }; diff --git a/indra/llcommon/lleventdispatcher.cpp b/indra/llcommon/lleventdispatcher.cpp index 742d6cf51f..bc53ec3da0 100644 --- a/indra/llcommon/lleventdispatcher.cpp +++ b/indra/llcommon/lleventdispatcher.cpp @@ -706,8 +706,17 @@ LLSD LLEventDispatcher::getMetadata(const std::string& name) const LLDispatchListener::LLDispatchListener(const std::string& pumpname, const std::string& key): LLEventDispatcher(pumpname, key), - mPump(pumpname, true), // allow tweaking for uniqueness - mBoundListener(mPump.listen("self", boost::bind(&LLDispatchListener::process, this, _1))) + // Do NOT tweak the passed pumpname. In practice, when someone + // instantiates a subclass of our LLEventAPI subclass, they intend to + // claim that LLEventPump name in the global LLEventPumps namespace. It + // would be mysterious and distressing if we allowed name tweaking, and + // someone else claimed pumpname first for a completely unrelated + // LLEventPump. Posted events would never reach our subclass listener + // because we would have silently changed its name; meanwhile listeners + // (if any) on that other LLEventPump would be confused by the events + // intended for our subclass. + LLEventStream(pumpname, false), + mBoundListener(listen("self", [this](const LLSD& event){ return process(event); })) { } diff --git a/indra/llcommon/lleventdispatcher.h b/indra/llcommon/lleventdispatcher.h index 1b3e834aeb..ce9d3775cc 100644 --- a/indra/llcommon/lleventdispatcher.h +++ b/indra/llcommon/lleventdispatcher.h @@ -165,12 +165,12 @@ public: * When calling this name, pass an LLSD::Array. Each entry in turn will be * converted to the corresponding parameter type using LLSDParam. */ - template - typename std::enable_if< - boost::function_types::is_nonmember_callable_builtin::value - >::type add(const std::string& name, - const std::string& desc, - Function f); + // enable_if usage per https://stackoverflow.com/a/39913395/5533635 + template::value + >::type> + void add(const std::string& name, const std::string& desc, Function f); /** * Register a nonstatic class method with arbitrary parameters. @@ -189,14 +189,13 @@ public: * When calling this name, pass an LLSD::Array. Each entry in turn will be * converted to the corresponding parameter type using LLSDParam. */ - template - typename std::enable_if< - boost::function_types::is_member_function_pointer::value && - ! std::is_convertible::value - >::type add(const std::string& name, - const std::string& desc, - Method f, - const InstanceGetter& getter); + template::value && + ! std::is_convertible::value + >::type> + void add(const std::string& name, const std::string& desc, Method f, + const InstanceGetter& getter); /** * Register a free function with arbitrary parameters. (This also works @@ -213,14 +212,12 @@ public: * an LLSD::Array using LLSDArgsMapper and then convert each entry in turn * to the corresponding parameter type using LLSDParam. */ - template - typename std::enable_if< - boost::function_types::is_nonmember_callable_builtin::value - >::type add(const std::string& name, - const std::string& desc, - Function f, - const LLSD& params, - const LLSD& defaults=LLSD()); + template::value + >::type> + void add(const std::string& name, const std::string& desc, Function f, + const LLSD& params, const LLSD& defaults=LLSD()); /** * Register a nonstatic class method with arbitrary parameters. @@ -243,16 +240,14 @@ public: * an LLSD::Array using LLSDArgsMapper and then convert each entry in turn * to the corresponding parameter type using LLSDParam. */ - template - typename std::enable_if< - boost::function_types::is_member_function_pointer::value && - ! std::is_convertible::value - >::type add(const std::string& name, - const std::string& desc, - Method f, - const InstanceGetter& getter, - const LLSD& params, - const LLSD& defaults=LLSD()); + template::value && + ! std::is_convertible::value + >::type> + void add(const std::string& name, const std::string& desc, Method f, + const InstanceGetter& getter, const LLSD& params, + const LLSD& defaults=LLSD()); //@} @@ -476,9 +471,8 @@ struct LLEventDispatcher::invoker } }; -template -typename std::enable_if< boost::function_types::is_nonmember_callable_builtin::value >::type -LLEventDispatcher::add(const std::string& name, const std::string& desc, Function f) +template +void LLEventDispatcher::add(const std::string& name, const std::string& desc, Function f) { // Construct an invoker_function, a callable accepting const args_source&. // Add to DispatchMap an ArrayParamsDispatchEntry that will handle the @@ -487,13 +481,9 @@ LLEventDispatcher::add(const std::string& name, const std::string& desc, Functio boost::function_types::function_arity::value); } -template -typename std::enable_if< - boost::function_types::is_member_function_pointer::value && - ! std::is_convertible::value ->::type -LLEventDispatcher::add(const std::string& name, const std::string& desc, Method f, - const InstanceGetter& getter) +template +void LLEventDispatcher::add(const std::string& name, const std::string& desc, Method f, + const InstanceGetter& getter) { // Subtract 1 from the compile-time arity because the getter takes care of // the first parameter. We only need (arity - 1) additional arguments. @@ -501,23 +491,18 @@ LLEventDispatcher::add(const std::string& name, const std::string& desc, Method boost::function_types::function_arity::value - 1); } -template -typename std::enable_if< boost::function_types::is_nonmember_callable_builtin::value >::type -LLEventDispatcher::add(const std::string& name, const std::string& desc, Function f, - const LLSD& params, const LLSD& defaults) +template +void LLEventDispatcher::add(const std::string& name, const std::string& desc, Function f, + const LLSD& params, const LLSD& defaults) { // See comments for previous is_nonmember_callable_builtin add(). addMapParamsDispatchEntry(name, desc, make_invoker(f), params, defaults); } -template -typename std::enable_if< - boost::function_types::is_member_function_pointer::value && - ! std::is_convertible::value ->::type -LLEventDispatcher::add(const std::string& name, const std::string& desc, Method f, - const InstanceGetter& getter, - const LLSD& params, const LLSD& defaults) +template +void LLEventDispatcher::add(const std::string& name, const std::string& desc, Method f, + const InstanceGetter& getter, + const LLSD& params, const LLSD& defaults) { addMapParamsDispatchEntry(name, desc, make_invoker(f, getter), params, defaults); } @@ -560,17 +545,21 @@ LLEventDispatcher::make_invoker(Method f, const InstanceGetter& getter) * LLEventPump name and dispatch key, and add() its methods. Incoming events * will automatically be dispatched. */ -class LL_COMMON_API LLDispatchListener: public LLEventDispatcher +// Instead of containing an LLEventStream, LLDispatchListener derives from it. +// This allows an LLEventPumps::PumpFactory to return a pointer to an +// LLDispatchListener (subclass) instance, and still have ~LLEventPumps() +// properly clean it up. +class LL_COMMON_API LLDispatchListener: + public LLEventDispatcher, + public LLEventStream { public: LLDispatchListener(const std::string& pumpname, const std::string& key); - - std::string getPumpName() const { return mPump.getName(); } + virtual ~LLDispatchListener() {} private: bool process(const LLSD& event); - LLEventStream mPump; LLTempBoundListener mBoundListener; }; diff --git a/indra/llcommon/llevents.cpp b/indra/llcommon/llevents.cpp index 5725dad9cc..1a305ec3dc 100644 --- a/indra/llcommon/llevents.cpp +++ b/indra/llcommon/llevents.cpp @@ -90,6 +90,13 @@ bool LLEventPumps::registerTypeFactory(const std::string& type, const TypeFactor return true; } +void LLEventPumps::unregisterTypeFactory(const std::string& type) +{ + auto found = mFactories.find(type); + if (found != mFactories.end()) + mFactories.erase(found); +} + bool LLEventPumps::registerPumpFactory(const std::string& name, const PumpFactory& factory) { // Do we already have a pump by this name? @@ -109,10 +116,30 @@ bool LLEventPumps::registerPumpFactory(const std::string& name, const PumpFactor static std::string nul(1, '\0'); std::string type_name{ nul + name }; mTypes[name] = type_name; - mFactories[type_name] = factory; + // TypeFactory is called with (name, tweak, type), whereas PumpFactory + // accepts only name. We could adapt with std::bind(), but this lambda + // does the trick. + mFactories[type_name] = + [factory] + (const std::string& name, bool /*tweak*/, const std::string& /*type*/) + { return factory(name); }; return true; } +void LLEventPumps::unregisterPumpFactory(const std::string& name) +{ + auto tfound = mTypes.find(name); + if (tfound != mTypes.end()) + { + auto ffound = mFactories.find(tfound->second); + if (ffound != mFactories.end()) + { + mFactories.erase(ffound); + } + mTypes.erase(tfound); + } +} + LLEventPump& LLEventPumps::obtain(const std::string& name) { PumpMap::iterator found = mPumpMap.find(name); diff --git a/indra/llcommon/llevents.h b/indra/llcommon/llevents.h index 38adc31121..c1dbf4392f 100644 --- a/indra/llcommon/llevents.h +++ b/indra/llcommon/llevents.h @@ -280,6 +280,7 @@ public: * a TypeFactory for the specified @a type name. */ bool registerTypeFactory(const std::string& type, const TypeFactory& factory); + void unregisterTypeFactory(const std::string& type); /// function passed to registerPumpFactory() typedef std::function PumpFactory; @@ -304,6 +305,7 @@ public: * instantiated an LLEventPump(name), so obtain(name) returned that. */ bool registerPumpFactory(const std::string& name, const PumpFactory& factory); + void unregisterPumpFactory(const std::string& name); /** * Find the named LLEventPump instance. If it exists post the message to it. @@ -362,13 +364,13 @@ testable: typedef std::set PumpSet; PumpSet mOurPumps; // for make(), map string type name to LLEventPump subclass factory function - typedef std::map PumpFactories; + typedef std::map TypeFactories; // Data used by make(). // One might think mFactories and mTypes could reasonably be static. So // they could -- if not for the fact that make() or obtain() might be // called before this module's static variables have been initialized. // This is why we use singletons in the first place. - PumpFactories mFactories; + TypeFactories mFactories; // for obtain(), map desired string instance name to string type when // obtain() must create the instance diff --git a/indra/llcommon/tests/lazyeventapi_test.cpp b/indra/llcommon/tests/lazyeventapi_test.cpp new file mode 100644 index 0000000000..6639c5e540 --- /dev/null +++ b/indra/llcommon/tests/lazyeventapi_test.cpp @@ -0,0 +1,89 @@ +/** + * @file lazyeventapi_test.cpp + * @author Nat Goodspeed + * @date 2022-06-18 + * @brief Test for lazyeventapi. + * + * $LicenseInfo:firstyear=2022&license=viewerlgpl$ + * Copyright (c) 2022, Linden Research, Inc. + * $/LicenseInfo$ + */ + +// Precompiled header +#include "linden_common.h" +// associated header +#include "lazyeventapi.h" +// STL headers +// std headers +// external library headers +// other Linden headers +#include "../test/lltut.h" +#include "llevents.h" + +// LLEventAPI listener subclass +class MyListener: public LLEventAPI +{ +public: + MyListener(const LL::LazyEventAPIParams& params): + LLEventAPI(params) + {} + + void get(const LLSD& event) + { + std::cout << "MyListener::get() got " << event << std::endl; + } +}; + +// LazyEventAPI registrar subclass +class MyRegistrar: public LL::LazyEventAPI +{ + using super = LL::LazyEventAPI; + using super::listener; +public: + MyRegistrar(): + super("Test", "This is a test LLEventAPI") + { + add("get", "This is a get operation", &listener::get); + } +}; +// Normally we'd declare a static instance of MyRegistrar -- but because we +// may want to test with and without, defer declaration to individual test +// methods. + +/***************************************************************************** +* TUT +*****************************************************************************/ +namespace tut +{ + struct lazyeventapi_data + { + ~lazyeventapi_data() + { + // after every test, reset LLEventPumps + LLEventPumps::deleteSingleton(); + } + }; + typedef test_group lazyeventapi_group; + typedef lazyeventapi_group::object object; + lazyeventapi_group lazyeventapigrp("lazyeventapi"); + + template<> template<> + void object::test<1>() + { + set_test_name("LazyEventAPI"); + // this is where the magic (should) happen + // 'register' still a keyword until C++17 + MyRegistrar regster; + LLEventPumps::instance().obtain("Test").post("hey"); + } + + template<> template<> + void object::test<2>() + { + set_test_name("No LazyEventAPI"); + // Because the MyRegistrar declaration in test<1>() is local, because + // it has been destroyed, we fully expect NOT to reach a MyListener + // instance with this post. + LLEventPumps::instance().obtain("Test").post("moot"); + } +} // namespace tut -- cgit v1.2.3 From 490de3ab6e3a2b9dd8668c2093e265f36324f82e Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 21 Jun 2022 14:46:59 -0400 Subject: DRTVWR-564: We don't need LLEventAPI to befriend LazyEventAPI. --- indra/llcommon/lleventapi.h | 4 ---- 1 file changed, 4 deletions(-) (limited to 'indra') diff --git a/indra/llcommon/lleventapi.h b/indra/llcommon/lleventapi.h index a019458553..25f6becd8b 100644 --- a/indra/llcommon/lleventapi.h +++ b/indra/llcommon/lleventapi.h @@ -37,8 +37,6 @@ namespace LL { - template - class LazyEventAPI; class LazyEventAPIParams; } @@ -52,8 +50,6 @@ class LL_COMMON_API LLEventAPI: public LLDispatchListener, { typedef LLDispatchListener lbase; typedef LLInstanceTracker ibase; - template - friend class LL::LazyEventAPI; public: -- cgit v1.2.3 From fdc0257acbde5a2d5bb201efcc8bb723df09daf8 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 21 Jun 2022 15:23:29 -0400 Subject: DRTVWR-564: Fix LLEventDispatcher::addMethod() for LazyEventAPI. A classic LLEventAPI subclass calls LLEventDispatcher::add() methods in its own constructor. At that point, addMethod() can reliably dynamic_cast its 'this' pointer to the new subclass. But because of the way LazyEventAPI queues up add() calls, they're invoked in the (new) LLEventAPI constructor itself. The subclass constructor body hasn't even started running, and LLEventDispatcher::addMethod()'s dynamic_cast to the LLEventAPI subclass returns nullptr. addMethod() claims the new subclass isn't derived from LLEventDispatcher, which is confusing since it is. It works to change addMethod()'s dynamic_cast to static_cast. Flesh out lazyeventapi_test.cpp. post() maps with "op" keys to actually try to engage the registered operation. Give the operation an observable side effect; use ensure_mumble() to verify. Also verify that LazyEventAPI has captured the subject LLEventAPI's metadata in a way we can retrieve. --- indra/llcommon/lleventdispatcher.h | 2 +- indra/llcommon/tests/lazyeventapi_test.cpp | 47 ++++++++++++++++++++++++++---- 2 files changed, 42 insertions(+), 7 deletions(-) (limited to 'indra') diff --git a/indra/llcommon/lleventdispatcher.h b/indra/llcommon/lleventdispatcher.h index ce9d3775cc..2e140329f3 100644 --- a/indra/llcommon/lleventdispatcher.h +++ b/indra/llcommon/lleventdispatcher.h @@ -329,7 +329,7 @@ private: void addMethod(const std::string& name, const std::string& desc, const METHOD& method, const LLSD& required) { - CLASS* downcast = dynamic_cast(this); + CLASS* downcast = static_cast(this); if (! downcast) { addFail(name, typeid(CLASS).name()); diff --git a/indra/llcommon/tests/lazyeventapi_test.cpp b/indra/llcommon/tests/lazyeventapi_test.cpp index 6639c5e540..4c78fd7105 100644 --- a/indra/llcommon/tests/lazyeventapi_test.cpp +++ b/indra/llcommon/tests/lazyeventapi_test.cpp @@ -19,18 +19,25 @@ // other Linden headers #include "../test/lltut.h" #include "llevents.h" +#include "llsdutil.h" + +// observable side effect, solely for testing +static LLSD data; // LLEventAPI listener subclass class MyListener: public LLEventAPI { public: + // need this trivial forwarding constructor + // (of course do any other initialization your subclass requires) MyListener(const LL::LazyEventAPIParams& params): LLEventAPI(params) {} - void get(const LLSD& event) + // example operation, registered by LazyEventAPI subclass below + void set_data(const LLSD& event) { - std::cout << "MyListener::get() got " << event << std::endl; + data = event["data"]; } }; @@ -40,14 +47,17 @@ class MyRegistrar: public LL::LazyEventAPI using super = LL::LazyEventAPI; using super::listener; public: + // LazyEventAPI subclass initializes like a classic LLEventAPI subclass + // constructor, with API name and desc plus add() calls for the defined + // operations MyRegistrar(): super("Test", "This is a test LLEventAPI") { - add("get", "This is a get operation", &listener::get); + add("set", "This is a set operation", &listener::set_data); } }; // Normally we'd declare a static instance of MyRegistrar -- but because we -// may want to test with and without, defer declaration to individual test +// want to test both with and without, defer declaration to individual test // methods. /***************************************************************************** @@ -57,6 +67,11 @@ namespace tut { struct lazyeventapi_data { + lazyeventapi_data() + { + // before every test, reset 'data' + data.clear(); + } ~lazyeventapi_data() { // after every test, reset LLEventPumps @@ -74,7 +89,8 @@ namespace tut // this is where the magic (should) happen // 'register' still a keyword until C++17 MyRegistrar regster; - LLEventPumps::instance().obtain("Test").post("hey"); + LLEventPumps::instance().obtain("Test").post(llsd::map("op", "set", "data", "hey")); + ensure_equals("failed to set data", data.asString(), "hey"); } template<> template<> @@ -84,6 +100,25 @@ namespace tut // Because the MyRegistrar declaration in test<1>() is local, because // it has been destroyed, we fully expect NOT to reach a MyListener // instance with this post. - LLEventPumps::instance().obtain("Test").post("moot"); + LLEventPumps::instance().obtain("Test").post(llsd::map("op", "set", "data", "moot")); + ensure("accidentally set data", ! data.isDefined()); + } + + template<> template<> + void object::test<3>() + { + set_test_name("LazyEventAPI metadata"); + MyRegistrar regster; + const MyRegistrar* found = nullptr; + for (const auto& registrar : LL::LazyEventAPIBase::instance_snapshot()) + if ((found = dynamic_cast(®istrar))) + break; + ensure("Failed to find MyRegistrar via LLInstanceTracker", found); + ensure_equals("wrong API name", found->mParams.name, "Test"); + ensure_contains("wrong API desc", found->mParams.desc, "test LLEventAPI"); + ensure_equals("wrong API field", found->mParams.field, "op"); + ensure_equals("failed to find operations", found->mOperations.size(), 1); + ensure_equals("wrong operation name", found->mOperations[0].first, "set"); + ensure_contains("wrong operation desc", found->mOperations[0].second, "set operation"); } } // namespace tut -- cgit v1.2.3 From fa3a67f56b15d81bfd22f744314a7d9aa35bf90e Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 21 Jun 2022 15:50:08 -0400 Subject: DRTVWR-564: Remove implementation notes from before implementation. --- indra/llcommon/lazyeventapi.h | 24 ------------------------ 1 file changed, 24 deletions(-) (limited to 'indra') diff --git a/indra/llcommon/lazyeventapi.h b/indra/llcommon/lazyeventapi.h index 2e947899dc..7267a3e4ec 100644 --- a/indra/llcommon/lazyeventapi.h +++ b/indra/llcommon/lazyeventapi.h @@ -42,30 +42,6 @@ namespace LL boost::signals2::signal init; }; - // The tricky part is: can we capture a sequence of add() calls in the - // LazyEventAPI subclass constructor and then, in effect, replay those - // add() calls on instantiation of the registered LLEventAPI subclass? so - // we don't have to duplicate the add() calls in both constructors? - - // Derive a subclass from LazyEventAPI. Its constructor must pass - // LazyEventAPI's constructor the name, desc, field params. Moreover the - // constructor body must call add(name, desc, *args) for any of the - // LLEventDispatcher add() methods, referencing the LLEventAPI subclass - // methods. - - // LazyEventAPI will store the name, desc, field params for the overall - // LLEventAPI. It will support a single generic add() call accepting name, - // desc, parameter pack. - - // It will hold a std::vector> for each operation. - // It will make all these strings available to LLLeapListener. - - // Maybe what we want is to store a vector of callables (a - // boost::signals2!) and populate it with lambdas, each of which accepts - // LLEventAPI* and calls the relevant add() method by forwarding exactly - // the name, desc and parameter pack. Then, on constructing the target - // LLEventAPI, we just fire the signal, passing the new instance pointer. - /** * LazyEventAPIBase implements most of the functionality of LazyEventAPI * (q.v.), but we need the LazyEventAPI template subclass so we can accept -- cgit v1.2.3 From f9d810ac2a02ef96c843e214c7479146dd4f4157 Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Tue, 21 Jun 2022 17:24:03 -0400 Subject: DRTVWR-564: Per NickyD, need not test static_cast result for nullptr. --- indra/llcommon/lleventdispatcher.cpp | 7 ------- indra/llcommon/lleventdispatcher.h | 10 +--------- 2 files changed, 1 insertion(+), 16 deletions(-) (limited to 'indra') diff --git a/indra/llcommon/lleventdispatcher.cpp b/indra/llcommon/lleventdispatcher.cpp index bc53ec3da0..7ba8c5ada7 100644 --- a/indra/llcommon/lleventdispatcher.cpp +++ b/indra/llcommon/lleventdispatcher.cpp @@ -586,13 +586,6 @@ void LLEventDispatcher::add(const std::string& name, const std::string& desc, new LLSDDispatchEntry(desc, callable, required)))); } -void LLEventDispatcher::addFail(const std::string& name, const std::string& classname) const -{ - LL_ERRS("LLEventDispatcher") << "LLEventDispatcher(" << mDesc << ")::add(" << name - << "): " << classname << " is not a subclass " - << "of LLEventDispatcher" << LL_ENDL; -} - /// Unregister a callable bool LLEventDispatcher::remove(const std::string& name) { diff --git a/indra/llcommon/lleventdispatcher.h b/indra/llcommon/lleventdispatcher.h index 2e140329f3..6d1df86fea 100644 --- a/indra/llcommon/lleventdispatcher.h +++ b/indra/llcommon/lleventdispatcher.h @@ -330,16 +330,8 @@ private: const METHOD& method, const LLSD& required) { CLASS* downcast = static_cast(this); - if (! downcast) - { - addFail(name, typeid(CLASS).name()); - } - else - { - add(name, desc, boost::bind(method, downcast, _1), required); - } + add(name, desc, boost::bind(method, downcast, _1), required); } - void addFail(const std::string& name, const std::string& classname) const; std::string try_call_log(const std::string& key, const std::string& name, const LLSD& event) const; std::string try_call(const std::string& key, const std::string& name, -- cgit v1.2.3 From 6b53036f7499a4e42813378009050eaf02c0b69d Mon Sep 17 00:00:00 2001 From: Nat Goodspeed Date: Wed, 22 Jun 2022 10:51:11 -0400 Subject: DRTVWR-564: Allow LLLeapListener to report LazyEventAPIs too. One important factor in the design of LazyEventAPI was the desire to allow LLLeapListener to query metadata for an LLEventAPI even if it hasn't yet been instantiated by LazyEventAPI. That's why LazyEventAPI requires the same metadata required by a classic LLEventAPI. Instead of just publicly exposing its data members, give LazyEventAPI a query API mimicking LLEventAPI / LLEventDispatcher. Protect data members and private methods. Adapt lazyeventapi_test.cpp accordingly. Extend LLLeapListener::getAPIs() and getAPI() to look through LazyEventAPIBase instances after first checking existing LLEventAPI instances. Because the query API for LazyEventAPIBase mimics LLEventAPI's, extract getAPI()'s actual metadata reporting to a new internal template function reportAPI(). While we're touching LLLeapListener, we no longer need BOOST_FOREACH(). --- indra/llcommon/lazyeventapi.cpp | 19 ++++++++ indra/llcommon/lazyeventapi.h | 36 ++++++++++++--- indra/llcommon/llleaplistener.cpp | 70 ++++++++++++++++++++++-------- indra/llcommon/tests/lazyeventapi_test.cpp | 24 +++++++--- 4 files changed, 120 insertions(+), 29 deletions(-) (limited to 'indra') diff --git a/indra/llcommon/lazyeventapi.cpp b/indra/llcommon/lazyeventapi.cpp index aefc2db6da..028af9f33f 100644 --- a/indra/llcommon/lazyeventapi.cpp +++ b/indra/llcommon/lazyeventapi.cpp @@ -15,9 +15,11 @@ #include "lazyeventapi.h" // STL headers // std headers +#include // std::find_if // external library headers // other Linden headers #include "llevents.h" +#include "llsdutil.h" LL::LazyEventAPIBase::LazyEventAPIBase( const std::string& name, const std::string& desc, const std::string& field) @@ -51,3 +53,20 @@ LL::LazyEventAPIBase::~LazyEventAPIBase() LLEventPumps::instance().unregisterPumpFactory(mParams.name); } } + +LLSD LL::LazyEventAPIBase::getMetadata(const std::string& name) const +{ + // Since mOperations is a vector rather than a map, just search. + auto found = std::find_if(mOperations.begin(), mOperations.end(), + [&name](const auto& namedesc) + { return (namedesc.first == name); }); + if (found == mOperations.end()) + return {}; + + // LLEventDispatcher() supplements the returned metadata in different + // ways, depending on metadata provided to the specific add() method. + // Don't try to emulate all that. At some point we might consider more + // closely unifying LLEventDispatcher machinery with LazyEventAPI, but for + // now this will have to do. + return llsd::map("name", found->first, "desc", found->second); +} diff --git a/indra/llcommon/lazyeventapi.h b/indra/llcommon/lazyeventapi.h index 7267a3e4ec..a815b119f0 100644 --- a/indra/llcommon/lazyeventapi.h +++ b/indra/llcommon/lazyeventapi.h @@ -63,9 +63,6 @@ namespace LL LazyEventAPIBase& operator=(const LazyEventAPIBase&) = delete; LazyEventAPIBase& operator=(LazyEventAPIBase&&) = delete; - // actually instantiate the companion LLEventAPI subclass - virtual LLEventPump* construct(const std::string& name) = 0; - // capture add() calls we want to play back on LLEventAPI construction template void add(const std::string& name, const std::string& desc, ARGS&&... rest) @@ -96,12 +93,40 @@ namespace LL }); } + // The following queries mimic the LLEventAPI / LLEventDispatcher + // query API. + + // Get the string name of the subject LLEventAPI + std::string getName() const { return mParams.name; } + // Get the documentation string + std::string getDesc() const { return mParams.desc; } + // Retrieve the LLSD key we use for dispatching + std::string getDispatchKey() const { return mParams.field; } + + // operations + using NameDesc = std::pair; + + private: // metadata that might be queried by LLLeapListener - std::vector> mOperations; + std::vector mOperations; + + public: + using const_iterator = decltype(mOperations)::const_iterator; + const_iterator begin() const { return mOperations.begin(); } + const_iterator end() const { return mOperations.end(); } + LLSD getMetadata(const std::string& name) const; + + protected: // Params with which to instantiate the companion LLEventAPI subclass LazyEventAPIParams mParams; private: + // true if we successfully registered our LLEventAPI on construction + bool mRegistered; + + // actually instantiate the companion LLEventAPI subclass + virtual LLEventPump* construct(const std::string& name) = 0; + // Passing an overloaded function to any function that accepts an // arbitrary callable is a PITB because you have to specify the // correct overload. What we want is for the compiler to select the @@ -117,8 +142,6 @@ namespace LL { instance->add(std::forward(args)...); } - - bool mRegistered; }; /** @@ -168,6 +191,7 @@ namespace LL LazyEventAPIBase(name, desc, field) {} + private: LLEventPump* construct(const std::string& /*name*/) override { // base class has carefully assembled LazyEventAPIParams embedded diff --git a/indra/llcommon/llleaplistener.cpp b/indra/llcommon/llleaplistener.cpp index 11bfec1b31..471f52e91c 100644 --- a/indra/llcommon/llleaplistener.cpp +++ b/indra/llcommon/llleaplistener.cpp @@ -14,14 +14,16 @@ // associated header #include "llleaplistener.h" // STL headers -#include +#include // std::find_if #include +#include +#include // std headers // external library headers -#include // other Linden headers -#include "lluuid.h" +#include "lazyeventapi.h" #include "llsdutil.h" +#include "lluuid.h" #include "stringize.h" /***************************************************************************** @@ -110,7 +112,7 @@ LLLeapListener::~LLLeapListener() // value_type, and Bad Things would happen if you copied an // LLTempBoundListener. (Destruction of the original would disconnect the // listener, invalidating every stored connection.) - BOOST_FOREACH(ListenersMap::value_type& pair, mListeners) + for (ListenersMap::value_type& pair : mListeners) { pair.second.disconnect(); } @@ -208,31 +210,65 @@ void LLLeapListener::getAPIs(const LLSD& request) const { Response reply(LLSD(), request); + // first, traverse existing LLEventAPI instances + std::set instances; for (auto& ea : LLEventAPI::instance_snapshot()) { - LLSD info; - info["desc"] = ea.getDesc(); - reply[ea.getName()] = info; + // remember which APIs are actually instantiated + instances.insert(ea.getName()); + reply[ea.getName()] = llsd::map("desc", ea.getDesc()); + } + // supplement that with *potential* instances: that is, instances of + // LazyEventAPI that can each instantiate an LLEventAPI on demand + for (const auto& lea : LL::LazyEventAPIBase::instance_snapshot()) + { + // skip any LazyEventAPI that's already instantiated its LLEventAPI + if (instances.find(lea.getName()) == instances.end()) + { + reply[lea.getName()] = llsd::map("desc", lea.getDesc()); + } } } +// Because LazyEventAPI deliberately mimics LLEventAPI's query API, this +// function can be passed either -- even though they're unrelated types. +template +void reportAPI(LLEventAPI::Response& reply, const API& api) +{ + reply["name"] = api.getName(); + reply["desc"] = api.getDesc(); + reply["key"] = api.getDispatchKey(); + LLSD ops; + for (const auto& namedesc : api) + { + ops.append(api.getMetadata(namedesc.first)); + } + reply["ops"] = ops; +} + void LLLeapListener::getAPI(const LLSD& request) const { Response reply(LLSD(), request); - auto found = LLEventAPI::getInstance(request["api"]); - if (found) + // check first among existing LLEventAPI instances + auto foundea = LLEventAPI::getInstance(request["api"]); + if (foundea) + { + reportAPI(reply, *foundea); + } + else { - reply["name"] = found->getName(); - reply["desc"] = found->getDesc(); - reply["key"] = found->getDispatchKey(); - LLSD ops; - for (LLEventAPI::const_iterator oi(found->begin()), oend(found->end()); - oi != oend; ++oi) + // Here the requested LLEventAPI doesn't yet exist, but do we have a + // registered LazyEventAPI for it? + LL::LazyEventAPIBase::instance_snapshot snap; + auto foundlea = std::find_if(snap.begin(), snap.end(), + [api = request["api"].asString()] + (const auto& lea) + { return (lea.getName() == api); }); + if (foundlea != snap.end()) { - ops.append(found->getMetadata(oi->first)); + reportAPI(reply, *foundlea); } - reply["ops"] = ops; } } diff --git a/indra/llcommon/tests/lazyeventapi_test.cpp b/indra/llcommon/tests/lazyeventapi_test.cpp index 4c78fd7105..31b2d6d17f 100644 --- a/indra/llcommon/tests/lazyeventapi_test.cpp +++ b/indra/llcommon/tests/lazyeventapi_test.cpp @@ -109,16 +109,28 @@ namespace tut { set_test_name("LazyEventAPI metadata"); MyRegistrar regster; + // Of course we have 'regster' in hand; we don't need to search for + // it. But this next test verifies that we can find (all) LazyEventAPI + // instances using LazyEventAPIBase::instance_snapshot. Normally we + // wouldn't search; normally we'd just look at each instance in the + // loop body. const MyRegistrar* found = nullptr; for (const auto& registrar : LL::LazyEventAPIBase::instance_snapshot()) if ((found = dynamic_cast(®istrar))) break; ensure("Failed to find MyRegistrar via LLInstanceTracker", found); - ensure_equals("wrong API name", found->mParams.name, "Test"); - ensure_contains("wrong API desc", found->mParams.desc, "test LLEventAPI"); - ensure_equals("wrong API field", found->mParams.field, "op"); - ensure_equals("failed to find operations", found->mOperations.size(), 1); - ensure_equals("wrong operation name", found->mOperations[0].first, "set"); - ensure_contains("wrong operation desc", found->mOperations[0].second, "set operation"); + + ensure_equals("wrong API name", found->getName(), "Test"); + ensure_contains("wrong API desc", found->getDesc(), "test LLEventAPI"); + ensure_equals("wrong API field", found->getDispatchKey(), "op"); + // Normally we'd just iterate over *found. But for test purposes, + // actually capture the range of NameDesc pairs in a vector. + std::vector ops{ found->begin(), found->end() }; + ensure_equals("failed to find operations", ops.size(), 1); + ensure_equals("wrong operation name", ops[0].first, "set"); + ensure_contains("wrong operation desc", ops[0].second, "set operation"); + LLSD metadata{ found->getMetadata(ops[0].first) }; + ensure_equals("bad metadata name", metadata["name"].asString(), ops[0].first); + ensure_equals("bad metadata desc", metadata["desc"].asString(), ops[0].second); } } // namespace tut -- cgit v1.2.3