From: Amos Jeffries Date: Mon, 31 Aug 2015 09:38:51 +0000 (-0700) Subject: Packable API: Rename StoreEntryStream to PackableStream X-Git-Tag: SQUID_4_0_1~82 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=7e10ac870f7e7a03d3f94ab138e04b8eb1f3bc96;p=thirdparty%2Fsquid.git Packable API: Rename StoreEntryStream to PackableStream PackableStream has some implicit new properties different from the original StoreEntryStream type: * lack of Store.h dependency * ability to stream into a MemBuf if its creator desires that Meaning PackableStream can be used in a wider range of the code without increasing library dependencies. --- diff --git a/src/Makefile.am b/src/Makefile.am index f103003313..15efa8916b 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -521,7 +521,6 @@ EXTRA_squid_SOURCES = \ noinst_HEADERS = \ client_side_request.cci \ MemBuf.h \ - StoreEntryStream.h \ String.cci \ SquidString.h \ SquidTime.h @@ -2869,10 +2868,10 @@ tests_testStore_SOURCES= \ Transients.cc \ tests/stub_tools.cc \ tests/stub_UdsOp.cc \ + tests/testPackableStream.cc \ + tests/testPackableStream.h \ tests/testStore.cc \ tests/testStore.h \ - tests/testStoreEntryStream.cc \ - tests/testStoreEntryStream.h \ tests/testStoreController.cc \ tests/testStoreController.h \ tests/testStoreHashIndex.cc \ diff --git a/src/SBufStatsAction.cc b/src/SBufStatsAction.cc index 154896cd2f..a37c051fd1 100644 --- a/src/SBufStatsAction.cc +++ b/src/SBufStatsAction.cc @@ -7,12 +7,12 @@ */ #include "squid.h" +#include "base/PackableStream.h" #include "ipc/Messages.h" #include "ipc/TypedMsgHdr.h" #include "mgr/Registration.h" #include "SBufDetailedStats.h" #include "SBufStatsAction.h" -#include "StoreEntryStream.h" SBufStatsAction::SBufStatsAction(const Mgr::CommandPointer &cmd_): Action(cmd_) @@ -53,7 +53,7 @@ statHistSBufDumper(StoreEntry * sentry, int, double val, double size, int count) void SBufStatsAction::dump(StoreEntry* entry) { - StoreEntryStream ses(entry); + PackableStream ses(*entry); ses << "\n\n\nThese statistics are experimental; their format and contents " "should not be relied upon, they are bound to change as " "the SBuf feature is evolved\n"; diff --git a/src/Store.h b/src/Store.h index 28e528bb93..870375af92 100644 --- a/src/Store.h +++ b/src/Store.h @@ -184,10 +184,6 @@ public: ESIElement::Pointer cachedESITree; #endif - /** disable sending content to the clients */ - virtual void buffer(); - /** flush any buffered content */ - virtual void flush(); virtual int64_t objectLen() const; virtual int64_t contentLen() const; @@ -217,6 +213,8 @@ public: /* Packable API */ virtual void append(char const *, int); virtual void vappendf(const char *, va_list); + virtual void buffer(); + virtual void flush(); protected: void transientsAbandonmentCheck(); diff --git a/src/StoreEntryStream.h b/src/StoreEntryStream.h deleted file mode 100644 index b618dcf547..0000000000 --- a/src/StoreEntryStream.h +++ /dev/null @@ -1,102 +0,0 @@ -/* - * Copyright (C) 1996-2015 The Squid Software Foundation and contributors - * - * Squid software is distributed under GPLv2+ license and includes - * contributions from numerous individuals and organizations. - * Please see the COPYING and CONTRIBUTORS files for details. - */ - -#ifndef SQUID_STORE_ENTRY_STREAM_H -#define SQUID_STORE_ENTRY_STREAM_H - -#include "Store.h" - -#include - -/* - * This class provides a streambuf interface for writing - * to StoreEntries. Typical use is via a StoreEntryStream - * rather than direct manipulation - */ - -class StoreEntryStreamBuf : public std::streambuf -{ - -public: - StoreEntryStreamBuf(StoreEntry *anEntry) : theEntry(anEntry) { - theEntry->lock("StoreEntryStreamBuf"); - theEntry->buffer(); - } - - ~StoreEntryStreamBuf() { - theEntry->unlock("StoreEntryStreamBuf"); - } - -protected: - /* flush the current buffer and the character that is overflowing - * to the store entry. - */ - virtual int_type overflow(int_type aChar = traits_type::eof()) { - std::streamsize pending(pptr() - pbase()); - - if (pending && sync ()) - return traits_type::eof(); - - if (aChar != traits_type::eof()) { - // NP: cast because GCC promotes int_type to 32-bit type - // std::basic_streambuf::int_type {aka int} - // despite the definition with 8-bit type value. - char chars[1] = {char(aChar)}; - - if (aChar != traits_type::eof()) - theEntry->append(chars, 1); - } - - pbump (-pending); // Reset pptr(). - return aChar; - } - - /* push the buffer to the store */ - virtual int sync() { - std::streamsize pending(pptr() - pbase()); - - if (pending) - theEntry->append(pbase(), pending); - - theEntry->flush(); - - return 0; - } - - /* write multiple characters to the store entry - * - this is an optimisation method. - */ - virtual std::streamsize xsputn(const char * chars, std::streamsize number) { - if (number) - theEntry->append(chars, number); - - return number; - } - -private: - StoreEntry *theEntry; - -}; - -class StoreEntryStream : public std::ostream -{ - -public: - /* create a stream for writing text etc into theEntry */ - // See http://www.codecomments.com/archive292-2005-2-396222.html - StoreEntryStream(StoreEntry *entry): std::ostream(0), theBuffer(entry) { - rdbuf(&theBuffer); // set the buffer to now-initialized theBuffer - clear(); //clear badbit set by calling init(0) - } - -private: - StoreEntryStreamBuf theBuffer; -}; - -#endif /* SQUID_STORE_ENTRY_STREAM_H */ - diff --git a/src/base/Makefile.am b/src/base/Makefile.am index 4a43969a41..2b33f2d9de 100644 --- a/src/base/Makefile.am +++ b/src/base/Makefile.am @@ -29,6 +29,7 @@ libbase_la_SOURCES = \ LookupTable.h \ LruMap.h \ Packable.h \ + PackableStream.h \ RegexPattern.cc \ RegexPattern.h \ RunnersRegistry.cc \ diff --git a/src/base/Packable.h b/src/base/Packable.h index 85fb95d755..1df486abc8 100644 --- a/src/base/Packable.h +++ b/src/base/Packable.h @@ -69,6 +69,21 @@ public: * of side-effects */ virtual void vappendf(const char *fmt, va_list ap) = 0; + + /** start buffering appends (if relevant) + * + * Indicates that a number of small appends are about to + * follow so would be detrimental to trigger expensive + * activity on each. + */ + virtual void buffer() {} + + /** perform a buffer flush (if relevant) + * + * Used by code such as PackableStream, that assumes the + * Packable leads to some form of output buffer. + */ + virtual void flush() {} }; #endif /* SQUID_SRC_BASE_PACKABLE_H */ diff --git a/src/base/PackableStream.h b/src/base/PackableStream.h new file mode 100644 index 0000000000..b1384d2059 --- /dev/null +++ b/src/base/PackableStream.h @@ -0,0 +1,82 @@ +/* + * Copyright (C) 1996-2015 The Squid Software Foundation and contributors + * + * Squid software is distributed under GPLv2+ license and includes + * contributions from numerous individuals and organizations. + * Please see the COPYING and CONTRIBUTORS files for details. + */ + +#ifndef SQUID_SRC_BASE_PACKABLESTREAM_H +#define SQUID_SRC_BASE_PACKABLESTREAM_H + +#include "base/Packable.h" + +#include + +/** + * Provides a streambuf interface for writing to Packable objects. + * Typical use is via a PackableStream rather than direct manipulation + */ +class PackableStreamBuf : public std::streambuf +{ +public: + explicit PackableStreamBuf(Packable &p) : buf_(p) { buf_.buffer(); } + virtual ~PackableStreamBuf() = default; + +protected: + /** flush the current buffer and the character that is overflowing + * to the Packable. + */ + virtual int_type overflow(int_type aChar = traits_type::eof()) override { + std::streamsize pending(pptr() - pbase()); + + if (pending && sync()) + return traits_type::eof(); + + if (aChar != traits_type::eof()) { + const char C = static_cast(aChar); + lowAppend(&C, 1); + } + + pbump(-pending); // Reset pptr(). + return aChar; + } + + /** push the buffer to the Packable */ + virtual int sync() override { + std::streamsize pending(pptr() - pbase()); + lowAppend(pbase(), pending); + buf_.flush(); + return 0; + } + + /** write multiple characters to the Packable + * - this is an optimisation method. + */ + virtual std::streamsize xsputn(const char * chars, std::streamsize number) override { + lowAppend(chars, number); + return number; + } + +private: + void lowAppend(const char *s, const std::streamsize n) {buf_.append(s,n);} + + Packable &buf_; +}; + +class PackableStream : public std::ostream +{ +public: + /* create a stream for writing text etc into theBuffer */ + // See http://www.codecomments.com/archive292-2005-2-396222.html + explicit PackableStream(Packable &p) : std::ostream(0), theBuffer(p) { + rdbuf(&theBuffer); // set the buffer to now-initialized theBuffer + clear(); //clear badbit set by calling init(0) + } + +private: + PackableStreamBuf theBuffer; +}; + +#endif /* SQUID_SRC_BASE_PACKABLESTREAM_H */ + diff --git a/src/mem/old_api.cc b/src/mem/old_api.cc index cba5a0a611..99a2489e0d 100644 --- a/src/mem/old_api.cc +++ b/src/mem/old_api.cc @@ -11,6 +11,7 @@ #include "squid.h" #include "acl/AclDenyInfoList.h" #include "acl/AclNameList.h" +#include "base/PackableStream.h" #include "CacheDigest.h" #include "ClientInfo.h" #include "disk.h" @@ -27,10 +28,8 @@ #include "SquidList.h" #include "SquidTime.h" #include "Store.h" -#include "StoreEntryStream.h" #include -#include /* forward declarations */ static void memFree2K(void *); @@ -140,7 +139,7 @@ memBufStats(std::ostream & stream) void Mem::Stats(StoreEntry * sentry) { - StoreEntryStream stream(sentry); + PackableStream stream(*sentry); Report(stream); memStringStats(stream); memBufStats(stream); diff --git a/src/ssl/context_storage.cc b/src/ssl/context_storage.cc index 293e91d48f..3bcf22bf32 100644 --- a/src/ssl/context_storage.cc +++ b/src/ssl/context_storage.cc @@ -7,10 +7,10 @@ */ #include "squid.h" +#include "base/PackableStream.h" #include "mgr/Registration.h" #include "ssl/context_storage.h" #include "Store.h" -#include "StoreEntryStream.h" #include #if HAVE_OPENSSL_SSL_H @@ -29,7 +29,7 @@ Ssl::CertificateStorageAction::Create(const Mgr::Command::Pointer &aCmd) void Ssl::CertificateStorageAction::dump (StoreEntry *sentry) { - StoreEntryStream stream(sentry); + PackableStream stream(*sentry); const char delimiter = '\t'; const char endString = '\n'; // Page title. diff --git a/src/store.cc b/src/store.cc index 7968453bbd..d386fa2978 100644 --- a/src/store.cc +++ b/src/store.cc @@ -1756,14 +1756,21 @@ StoreEntry::createMemObject(const char *aUrl, const char *aLogUrl, const HttpReq mem_obj->setUris(aUrl, aLogUrl, aMethod); } -/* this just sets DELAY_SENDING */ +/** disable sending content to the clients. + * + * This just sets DELAY_SENDING. + */ void StoreEntry::buffer() { EBIT_SET(flags, DELAY_SENDING); } -/* this just clears DELAY_SENDING and Invokes the handlers */ +/** flush any buffered content. + * + * This just clears DELAY_SENDING and Invokes the handlers + * to begin sending anything that may be buffered. + */ void StoreEntry::flush() { diff --git a/src/tests/testStoreEntryStream.cc b/src/tests/testPackableStream.cc similarity index 79% rename from src/tests/testStoreEntryStream.cc rename to src/tests/testPackableStream.cc index da73a9181a..cd67e192c4 100644 --- a/src/tests/testStoreEntryStream.cc +++ b/src/tests/testPackableStream.cc @@ -7,26 +7,28 @@ */ #include "squid.h" +#include "base/PackableStream.h" #include "CapturingStoreEntry.h" #include "Store.h" -#include "StoreEntryStream.h" +#include "testPackableStream.h" #include "testStore.h" -#include "testStoreEntryStream.h" #include #include -CPPUNIT_TEST_SUITE_REGISTRATION( testStoreEntryStream ); +CPPUNIT_TEST_SUITE_REGISTRATION( testPackableStream ); /* init memory pools */ -void testStoreEntryStream::setUp() +void testPackableStream::setUp() { Mem::Init(); } +// TODO: test streaming to a MemBuf as well. + void -testStoreEntryStream::testGetStream() +testPackableStream::testGetStream() { /* Setup a store root so we can create a StoreEntry */ StorePointer aStore (new TestStore); @@ -34,7 +36,8 @@ testStoreEntryStream::testGetStream() CapturingStoreEntry * anEntry = new CapturingStoreEntry(); { - StoreEntryStream stream(anEntry); // locks and unlocks/deletes anEntry + anEntry->lock("test"); + PackableStream stream(*anEntry); CPPUNIT_ASSERT_EQUAL(1, anEntry->_buffer_calls); CPPUNIT_ASSERT_EQUAL(0, anEntry->_flush_calls); @@ -51,10 +54,9 @@ testStoreEntryStream::testGetStream() CPPUNIT_ASSERT(anEntry->_flush_calls > preFlushCount); CPPUNIT_ASSERT_EQUAL(1, anEntry->_buffer_calls); - - CPPUNIT_ASSERT_EQUAL(String("12345677.7 some text !."), - anEntry->_appended_text); + CPPUNIT_ASSERT_EQUAL(String("12345677.7 some text !."), anEntry->_appended_text); } + delete anEntry; // does the unlock() Store::Root(NULL); } diff --git a/src/tests/testStoreEntryStream.h b/src/tests/testPackableStream.h similarity index 63% rename from src/tests/testStoreEntryStream.h rename to src/tests/testPackableStream.h index 1e7882d7e6..fb3dc43dcf 100644 --- a/src/tests/testStoreEntryStream.h +++ b/src/tests/testPackableStream.h @@ -6,18 +6,18 @@ * Please see the COPYING and CONTRIBUTORS files for details. */ -#ifndef SQUID_SRC_TEST_STORE_ENTRY_STREAM_H -#define SQUID_SRC_TEST_STORE_ENTRY_STREAM_H +#ifndef SQUID_SRC_TESTS_TESTPACKABLESTREAM_H +#define SQUID_SRC_TESTS_TESTPACKABLESTREAM_H #include /* - * test StoreEntryStream + * test PackableStream */ -class testStoreEntryStream : public CPPUNIT_NS::TestFixture +class testPackableStream : public CPPUNIT_NS::TestFixture { - CPPUNIT_TEST_SUITE( testStoreEntryStream ); + CPPUNIT_TEST_SUITE( testPackableStream ); CPPUNIT_TEST( testGetStream ); CPPUNIT_TEST_SUITE_END(); @@ -28,5 +28,5 @@ protected: void testGetStream(); }; -#endif +#endif /* SQUID_SRC_TESTS_TESTPACKABLESTREAM_H */