[Merge] lp:~thomas-voss/trust-store/add-reporting-for-cached-agent into lp:trust-store
Seth Arnold
seth.arnold at canonical.com
Tue Aug 12 23:29:15 UTC 2014
Review: Approve
Looks good to me -- I've suggested changing "User's'" to "User's", and asked if an "answer" is really a request. The log messages also feel more verbose than usual but whatever makes sense for whoever is consuming these...
Thanks
Diff comments:
> === modified file 'CMakeLists.txt'
> --- CMakeLists.txt 2014-08-06 11:22:28 +0000
> +++ CMakeLists.txt 2014-08-12 12:32:01 +0000
> @@ -44,6 +44,8 @@
> )
> endif()
>
> +pkg_check_modules(GFLAGS libgflags REQUIRED)
> +pkg_check_modules(GLOG libglog REQUIRED)
> pkg_check_modules(PROCESS_CPP process-cpp REQUIRED)
>
> set(TRUST_STORE_VERSION_MAJOR 1)
> @@ -55,6 +57,8 @@
> include_directories(
> include/
>
> + ${GFLAGS_INCLUDE_DIRS}
> + ${GLOG_INCLUDE_DIRS}
> ${PROCESS_CPP_INCLUDE_DIRS}
> )
>
>
> === modified file 'debian/control'
> --- debian/control 2014-08-07 08:05:30 +0000
> +++ debian/control 2014-08-12 12:32:01 +0000
> @@ -15,6 +15,8 @@
> libboost-system-dev,
> libdbus-cpp-dev (>= 4.0.0),
> libdbus-1-dev,
> + libgflags-dev,
> + libgoogle-glog-dev,
> libgtest-dev,
> libjson-c-dev,
> libmirclient-dev [amd64 arm64 i386 armhf],
>
> === modified file 'include/core/trust/cached_agent.h'
> --- include/core/trust/cached_agent.h 2014-08-04 07:10:52 +0000
> +++ include/core/trust/cached_agent.h 2014-08-12 12:32:01 +0000
> @@ -29,9 +29,24 @@
> class CORE_TRUST_DLL_PUBLIC CachedAgent : public core::trust::Agent
> {
> public:
> - /** @brief To safe some typing. */
> + /** @brief To save some typing. */
> typedef std::shared_ptr<CachedAgent> Ptr;
>
> + /** @brief Abstract capturer of internal events for post-mortem debugging/analysis purposes. */
> + struct Reporter
> + {
> + /** @cond */
> + Reporter() = default;
> + virtual ~Reporter() = default;
> + /** @endcond */
> +
> + /** @brief Invoked whenever the implementation was able to resolve a cached request. */
> + virtual void report_cached_answer_found(const core::trust::Agent::RequestParameters&, const core::trust::Request&);
> +
> + /** @brief Invoked whenever the implementation called out to an agent to prompt the user for trust. */
> + virtual void report_user_prompted_for_trust(const core::trust::Agent::RequestParameters&, const core::trust::Request::Answer&);
> + };
> +
> /** @brief Creation time parameters. */
> struct Configuration
> {
> @@ -39,6 +54,8 @@
> std::shared_ptr<Agent> agent;
> /** @brief The store caching user answers to trust prompts. */
> std::shared_ptr<Store> store;
> + /** @brief The reporter implementation. */
> + std::shared_ptr<Reporter> reporter;
> };
>
> /**
>
> === modified file 'src/CMakeLists.txt'
> --- src/CMakeLists.txt 2014-08-06 10:06:43 +0000
> +++ src/CMakeLists.txt 2014-08-12 12:32:01 +0000
> @@ -37,6 +37,7 @@
>
> # An agent-implementation using a store instance to cache user replies.
> core/trust/cached_agent.cpp
> + core/trust/cached_agent_glog_reporter.cpp
>
> # Agent implementations for handling request out of process.
> core/trust/remote/agent.h
> @@ -160,6 +161,8 @@
>
> ${Boost_LIBRARIES}
> ${DBUS_LIBRARIES}
> + ${GFLAGS_LDFLAGS}
> + ${GLOG_LDFLAGS}
> ${LIBAPPARMOR_LDFLAGS}
> ${MIR_CLIENT_LDFLAGS}
> ${MIR_COMMON_LDFLAGS}
>
> === modified file 'src/core/trust/cached_agent.cpp'
> --- src/core/trust/cached_agent.cpp 2014-07-24 11:30:09 +0000
> +++ src/core/trust/cached_agent.cpp 2014-08-12 12:32:01 +0000
> @@ -20,6 +20,14 @@
>
> #include <core/trust/store.h>
>
> +void core::trust::CachedAgent::Reporter::report_cached_answer_found(const core::trust::Agent::RequestParameters&, const core::trust::Request&)
> +{
> +}
> +
> +void core::trust::CachedAgent::Reporter::report_user_prompted_for_trust(const core::trust::Agent::RequestParameters&, const core::trust::Request::Answer&)
> +{
> +}
> +
> core::trust::CachedAgent::CachedAgent(const core::trust::CachedAgent::Configuration& configuration)
> : configuration(configuration)
> {
> @@ -51,6 +59,8 @@
> // We have got results and we take the most recent one as the most appropriate.
> if (query->status() == core::trust::Store::Query::Status::has_more_results)
> {
> + // Tell the reporter that we found a cached answer.
> + configuration.reporter->report_cached_answer_found(params, query->current());
> // And we are returning early.
> return query->current().answer;
> }
> @@ -66,6 +76,9 @@
> params.description
> });
>
> + // Tell the reporter that the user was successfully prompted for an answer.
> + configuration.reporter->report_user_prompted_for_trust(params, answer);
> +
> configuration.store->add(core::trust::Request
> {
> params.application.id,
>
> === added file 'src/core/trust/cached_agent_glog_reporter.cpp'
> --- src/core/trust/cached_agent_glog_reporter.cpp 1970-01-01 00:00:00 +0000
> +++ src/core/trust/cached_agent_glog_reporter.cpp 2014-08-12 12:32:01 +0000
> @@ -0,0 +1,63 @@
> +/*
> + * Copyright © 2014 Canonical Ltd.
> + *
> + * This program is free software: you can redistribute it and/or modify it
> + * under the terms of the GNU Lesser General Public License version 3,
> + * as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + *
> + * Authored by: Thomas Voß <thomas.voss at canonical.com>
> + */
> +
> +#include <core/trust/cached_agent_glog_reporter.h>
> +
> +#include <glog/logging.h>
> +
> +namespace
> +{
> +struct Initializer
> +{
> + Initializer()
> + {
> + google::InitGoogleLogging("core::trust::Daemon");
> + }
> +
> + ~Initializer()
> + {
> + google::ShutdownGoogleLogging();
> + }
> +} initializer;
> +}
> +
> +// Creates a reporter instance with the given configuration, and initializes Google logging.
> +core::trust::CachedAgentGlogReporter::CachedAgentGlogReporter(const core::trust::CachedAgentGlogReporter::Configuration& configuration)
> +{
> + FLAGS_alsologtostderr = configuration.also_log_to_stderr;
> +}
> +
> +// Invoked whenever the implementation was able to resolve a cached request.
> +void core::trust::CachedAgentGlogReporter::report_cached_answer_found(const core::trust::Agent::RequestParameters& params, const core::trust::Request& r)
> +{
> + SYSLOG(INFO) << "CachedAgent::authenticate_request_with_parameters:" << std::endl
> + << " Application pid: " << params.application.pid
> + << " Application uid: " << params.application.uid
> + << " Application id: " << params.application.id
> + << " Cached answer: " << r;
Is this an answer or a question that is being logged?
> +}
> +
> +// Invoked whenever the implementation called out to an agent to prompt the user for trust.
> +void core::trust::CachedAgentGlogReporter::report_user_prompted_for_trust(const core::trust::Agent::RequestParameters& params, const core::trust::Request::Answer& a)
> +{
> + SYSLOG(INFO) << "CachedAgent::authenticate_request_with_parameters: No cached answer, prompted user for trust:"
> + << " Application pid: " << params.application.pid
> + << " Application uid: " << params.application.uid
> + << " Application id: " << params.application.id
> + << " User's' answer: " << a;
" User's answer: " would be more idiomatic.
> +}
>
> === added file 'src/core/trust/cached_agent_glog_reporter.h'
> --- src/core/trust/cached_agent_glog_reporter.h 1970-01-01 00:00:00 +0000
> +++ src/core/trust/cached_agent_glog_reporter.h 2014-08-12 12:32:01 +0000
> @@ -0,0 +1,51 @@
> +/*
> + * Copyright © 2014 Canonical Ltd.
> + *
> + * This program is free software: you can redistribute it and/or modify it
> + * under the terms of the GNU Lesser General Public License version 3,
> + * as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> + *
> + * Authored by: Thomas Voß <thomas.voss at canonical.com>
> + */
> +#ifndef CORE_TRUST_CACHED_AGENT_GLOG_REPORTER_H_
> +#define CORE_TRUST_CACHED_AGENT_GLOG_REPORTER_H_
> +
> +#include <core/trust/cached_agent.h>
> +
> +namespace core
> +{
> +namespace trust
> +{
> +// Implements the CachedAgent::Reporter interface by leveraging Google Log.
> +class CORE_TRUST_DLL_PUBLIC CachedAgentGlogReporter : public CachedAgent::Reporter
> +{
> +public:
> + // All creation time arguments go here.
> + struct Configuration
> + {
> + // By default, the implementation reports to syslog.
> + // If this flag is set to true, logging also goes to stderr.
> + bool also_log_to_stderr{true};
> + };
> +
> + // Creates a reporter instance with the given configuration, and initializes Google logging.
> + CachedAgentGlogReporter(const Configuration& configuration);
> +
> + // Invoked whenever the implementation was able to resolve a cached request.
> + void report_cached_answer_found(const core::trust::Agent::RequestParameters& params, const core::trust::Request&);
> +
> + // Invoked whenever the implementation called out to an agent to prompt the user for trust.
> + void report_user_prompted_for_trust(const core::trust::Agent::RequestParameters& params, const core::trust::Request::Answer&);
> +};
> +}
> +}
> +
> +#endif // CORE_TRUST_CACHED_AGENT_GLOG_REPORTER_H_
>
> === modified file 'src/core/trust/daemon.cpp'
> --- src/core/trust/daemon.cpp 2014-08-06 10:06:43 +0000
> +++ src/core/trust/daemon.cpp 2014-08-12 12:32:01 +0000
> @@ -31,6 +31,8 @@
>
> #include <core/trust/terminal_agent.h>
>
> +#include <core/trust/cached_agent_glog_reporter.h>
> +
> #include <core/dbus/asio/executor.h>
>
> #include <boost/asio.hpp>
> @@ -334,7 +336,9 @@
> core::trust::CachedAgent::Configuration
> {
> local_agent,
> - local_store
> + local_store,
> + std::make_shared<core::trust::CachedAgentGlogReporter>(
> + core::trust::CachedAgentGlogReporter::Configuration{})
> });
>
> auto remote_agent = remote_agent_factory(service_name, cached_agent, dict);
>
> === modified file 'tests/cached_agent_test.cpp'
> --- tests/cached_agent_test.cpp 2014-07-29 17:00:35 +0000
> +++ tests/cached_agent_test.cpp 2014-08-12 12:32:01 +0000
> @@ -23,6 +23,15 @@
>
> namespace
> {
> +struct MockReporter : public core::trust::CachedAgent::Reporter
> +{
> + /** @brief Invoked whenever the implementation was able to resolve a cached request. */
> + MOCK_METHOD2(report_cached_answer_found, void(const core::trust::Agent::RequestParameters&, const core::trust::Request&));
> +
> + /** @brief Invoked whenever the implementation called out to an agent to prompt the user for trust. */
> + MOCK_METHOD2(report_user_prompted_for_trust, void(const core::trust::Agent::RequestParameters&, const core::trust::Request::Answer&));
> +};
> +
> core::trust::Pid the_default_pid_for_testing()
> {
> return core::trust::Pid{42};
> @@ -63,6 +72,11 @@
> return std::make_shared<testing::NiceMock<MockStore::MockQuery>>();
> }
>
> +std::shared_ptr<testing::NiceMock<MockReporter>> a_mocked_reporter()
> +{
> + return std::make_shared<testing::NiceMock<MockReporter>>();
> +}
> +
> core::trust::Agent::RequestParameters default_request_parameters_for_testing()
> {
> return core::trust::Agent::RequestParameters
> @@ -101,7 +115,8 @@
> core::trust::CachedAgent::Configuration configuration
> {
> a_null_agent(),
> - a_mocked_store()
> + a_mocked_store(),
> + a_mocked_reporter()
> };
>
> EXPECT_THROW(core::trust::CachedAgent agent{configuration},
> @@ -113,7 +128,8 @@
> core::trust::CachedAgent::Configuration configuration
> {
> a_mocked_agent(),
> - a_null_store()
> + a_null_store(),
> + a_mocked_reporter()
> };
>
> EXPECT_THROW(core::trust::CachedAgent agent{configuration},
> @@ -139,6 +155,7 @@
> auto mocked_agent = a_mocked_agent();
> auto mocked_query = a_mocked_query();
> auto mocked_store = a_mocked_store();
> + auto mocked_reporter = a_mocked_reporter();
>
> ON_CALL(*mocked_query, status())
> .WillByDefault(
> @@ -163,11 +180,16 @@
> // The setup ensures that a previously stored answer is available in the store.
> // For that, the agent should not be queried.
> EXPECT_CALL(*mocked_agent, authenticate_request_with_parameters(_)).Times(0);
> + // We expect the implementation to call into the reporter with the request
> + // resolved from the cache, and returning early, thus not reporting any user prompting.
> + EXPECT_CALL(*mocked_reporter, report_cached_answer_found(params, _)).Times(1);
> + EXPECT_CALL(*mocked_reporter, report_user_prompted_for_trust(params, _)).Times(0);
>
> core::trust::CachedAgent::Configuration configuration
> {
> mocked_agent,
> - mocked_store
> + mocked_store,
> + mocked_reporter
> };
>
> core::trust::CachedAgent agent
> @@ -207,6 +229,7 @@
> auto mocked_agent = a_mocked_agent();
> auto mocked_query = a_mocked_query();
> auto mocked_store = a_mocked_store();
> + auto mocked_reporter = a_mocked_reporter();
>
> ON_CALL(*mocked_agent, authenticate_request_with_parameters(agent_params))
> .WillByDefault(
> @@ -234,11 +257,16 @@
> // The setup ensures that a previously stored answer is available in the store.
> // For that, the agent should not be queried.
> EXPECT_CALL(*mocked_agent, authenticate_request_with_parameters(agent_params)).Times(1);
> + // We expect the implementation to *not* call into the reporter as there is no request
> + // available from the cache. Instead the user should have been prompted.
> + EXPECT_CALL(*mocked_reporter, report_cached_answer_found(params, _)).Times(0);
> + EXPECT_CALL(*mocked_reporter, report_user_prompted_for_trust(params, _)).Times(1);
>
> core::trust::CachedAgent::Configuration configuration
> {
> mocked_agent,
> - mocked_store
> + mocked_store,
> + mocked_reporter
> };
>
> core::trust::CachedAgent agent
>
--
https://code.launchpad.net/~thomas-voss/trust-store/add-reporting-for-cached-agent/+merge/230470
Your team Ubuntu Phablet Team is subscribed to branch lp:trust-store.
More information about the Ubuntu-reviews
mailing list