From 6fb934355a368fb050845f294e30b2047e302a52 Mon Sep 17 00:00:00 2001 From: Jon Simantov Date: Mon, 2 Dec 2024 10:43:49 -0800 Subject: [PATCH 1/9] Add new GCS service account key file. --- scripts/gha-encrypted/gcs_key_file.json.gpg | Bin 1720 -> 1745 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/scripts/gha-encrypted/gcs_key_file.json.gpg b/scripts/gha-encrypted/gcs_key_file.json.gpg index d792ec8bb6f13a6be0ab208e54d551df1a5b9413..99a77ae423957cfebac7981979201e03cf7b65fb 100644 GIT binary patch literal 1745 zcmV;?1}^!G4Fm}T0-Oi8Q|-ip7yr`g0n{QY|IBn%DGY2n1E++f%tU}MbNJ#73U9Wr zAI3JPM96}874d=<@oB$x*>Vy=&ySVVD0tXVUJ`R=JQCmnqzAosJ; z5~T5dJ!6(DxVVPO9R(1`2Y6~Htz4Vq97UyimF%Cz4Yp1rUE-!8w4+*~Lox3K#eq8j z_s&;r8>9eK`VU`;x)7z;@BNv?YuiveAmH?gS7>yc!GV@t6em{<)1wz@*Va|}KPJ1Z z=Dr!+22j74_w{Sy_0(ROr25hNor%iMupKcQ2^5|ADg@oxFGTBc7}-;HVj5?Xwyz); zG*Biw9`S;t5<3huPv@{ywEn)_e+4Zh4N!dY8MQbHRMLtLn}3p`+E#|vkWhami{pyZ zculOK7|!QU$BCahw+OsH7_J)kG3FFq4h61=Vh-S&;pcDGlv%n)eo2-fu=|;0o|i(h zF;XTRq{bMq(xBf(P2bCxHZLdoAd~wT^!4eD7p%YW?pNv*6vGkdfrFaLIci7laCZDu z;PDuk3a#NDrol>_Mq5?JyPm#h161xvx`N8^-F-*pIrTxd?F55F?=}ZCER*3h@8&oP z@SJ1$rR^45NQlOw7Lh_GtQ_ecRKSm*L% zhDDt85SexE3LrFR7~BX>*}b<~V~iPsJ3riuG?U@Kr7XVekH%IYRxi#5O2t9dT7Lb? zrS(W2LklnY7egz$1q%f;M!?H7w__96$Nk>oYu8x&L7n2V!@`GSfL2Y z8jEAPXfAsa#@!_-jH?(P%ut-km-~%l1KQ0O$*pAn?MUvtDgYU!6kz6Mp(IKuvCKok z0bIl4Uc>uhuS~@l!wG(amz1#nRMKTkukK=6)-aVykwmNFEJMj^zG~D{VzJ^A&)zqMsp8>_1F+&k>wrVA{$K9C+bAO7tkw7(y6l2KBYZ)6%toJY^lsH-_VMH@sEFo zA>!RDbJJ^q=?o}enS_7iuRC5RY&A3w z1RU(vMRDaEK!vKb$T()kn|N{F#iz6Ojs=eJ>-`N?AX*(oQK@(h2!1N4z(PlLM0`7T z+4Psj!LsAek>*A236sf=8QVds9Qka&m3HjmHK!0OtBr6OlM-?=qSpL=GN z3)_&XI^Vin#eZG1)B4P+Q&%6~$*1o%K9l?(hB*^7kyUCSCHX>LRu?ZIrCcAMT((4L za&k^+xPhHTV|E&hj>O=bhnv2a*rh z=y6`8&7faS@&m2uKLfx!jSP1HV(CLqn1GuW`29$Ox$4zpC6~$N$x?c%(gs+kBMLdx z6ZRLBvkj9bkatIU-k{zpx<>ZfciszRj1+0zJT#E`+GshL|1JLlF-`g$7RjwFrJyVV z0Nk~~m6_aHi=mfJX;1=x*KFxPsFsinAs(4<;&7D=0fKeP;6<3)ov1z_Q7k_!tfJe> ztu7JH_Tae|2}~75Oq+|a*?5Gc7Slry;*-yaS%JXZYvUq=zs(hu@wbC4oVW9AhD3$P z*9=jb$BasPCwVngsiorb-z|Yg)#-(c{!u!|P#oK=;f0ojY}7KvDeAMieb_HWR+Sx< z;lA9C+GC$m$&G=UkzMYGuMa(ndHaG(%Ep$ds@hmiw|@?%*6>V$!J`YI6y&nOIwG?c zu);kJ8hZ{ESrHmaE$5SZ(a?<#_9>@c9W<6F(h2AWH69uy=#)*)S=6w$xZ)Rsn46gz z6!MDMXZl)3%37^Q(jnKZ*~jUw5UAtjI*Hqks|PB!DRRrdFPJK6%KWaP~K! zm?X?%jqCdpT@NA(lQc&|GICVuZ#plRb6a1G1BPcyl&t8NhoOrQRlt}pu;YpKrvY)B zq*lqk>b8+bUyp|ihhycR82u{1V@E+A7TO>ALGkR1ggMU|cehrv!LQ%k+oV8OiW%+n z_TvGh6C_C}ZhZ5L<|M3MSg@GnD z@z4qAdq>(J`qAUjoM4mBuUX7m=b;w5`YE^n=zI2>H~Gml`_ICGPKZ6W{D<$e{Ey@P z{lzF#hS`y(FT}x~YH{~2+E+apIqI5d!jpG|ra_~KIJwZ-d|5yC`IPxx)0^vr<@b9h zP#T^OTVI?E%@ zX<-KtGEXgRNQj=ck6um^7r}i4ed(4KbTp_6cR77KCg7?-E@?6X>P`+#3Z0>!&F;`J zO{UIMMOgL~;=q;ElOfH+f`wl{aV-%*$|A{IE5SB21vyAf0;n2r%wvrfRFBXgu`rxD z99n<)ZCUP!Qg9id4y7-3*}8RQXwyW6g%&7HDRwk5PmhSME_?4aRI1kUzS)|F*FRbTI(9 z2p1G=R~=&$mGR4PAYnP6v=7j;pl9#&d#sYpnXUm(9TO?a9Zgt)489W=(cf?NJ5JV) zuxMSxd@$KhyQU$E1sC?CM>ZcN45`%^!l{a$He$CPbJ}j=lY(oN3TpezwU|6DPXO6- zvPiPMG<%7HVrrsAXzS04IGpoReY;`XOf--yrpQ)(R0UfC7im9#w?46|`L7XQj3uk} z5)DYUjLhjQL76n@EEz5^s{B@9LeV~Wd^#JKBsDIBra z5)StDeDfsZrU;V%GdjK7r1O3&Xh=SEUJ@yakyhrJXPCbIVVyH&MFb5F29*-}!wG7g*&eGW( z;CC;ZeiWAtAmylz?gz)f)}kzMH{?OnS2B>ZHN3`h$wWuN6)Clj|BG|ibk$E9XlTiP zkIs%4Kj|=g!e9HBT1a(nf2%eAzf`=-bBhBYP=MI+%EgxZ^_M5ZPG~ynCqq=}>nUTY zu;AdA!}~s$0*fa!x}OI?D(4@*E3{(x-icTax-@maYZgfPP>}_uDi>-&+o!o+cQYGm!wU0ApP7 zbsW@a&ejjHGiEeP$o#q`UYdN1jln4MPJy!7ld6O9nIT&DQ$GlF7wEM_wrcU=AI OP5ep(Nd{G0epWUSJzH!5 From 243334de4ef84e717b423f17fbeadf86c99723b9 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 23 Jun 2025 18:08:22 +0000 Subject: [PATCH 2/9] Update C++ style guide to allow C++11 features and remove internal links --- STYLE_GUIDE.md | 375 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 375 insertions(+) create mode 100644 STYLE_GUIDE.md diff --git a/STYLE_GUIDE.md b/STYLE_GUIDE.md new file mode 100644 index 0000000000..bcbd3b06ae --- /dev/null +++ b/STYLE_GUIDE.md @@ -0,0 +1,375 @@ +# C++ API Guidelines + +**WIP** - *please feel free to improve* + +Intended for Firebase APIs, but also applicable to any C++ or Game APIs. + +# Code Style + +Please comply with the +[Google C++ Style Guide](https://google.github.io/styleguide/cppguide.html) +as much as possible. Refresh your memory of this document before you start :) + +# C++ API Design + +### Don't force any particular usage pattern upon the client. + +C++ is a huge language, with a great variety of ways in which things can be +done, compared to other languages. As a consequence, C++ projects can be very +particular about what features of the language they use or don't use, how they +represent their data, and structure their code. + +An API that forces the use of a feature or structure the client doesn't use +will be very unwelcome. A good API uses only the simplest common denominator +data types and features, and will be useable by all. This can generally be done +with minimal impact on your API’s simplicity, or at least should form the +baseline API. + +Examples of typical Do's and Don'ts: + +* Don't force the use of a particular data structure to supply or receive data. + Typical examples: + * `std::vector`: If the client doesn't have the data already in a + `std::vector` (or only wants to use part of a vector), they are forced + to copy/allocate a new one, and C++ programmers don't like unnecessary + copies/allocations. + Instead, your primary interface should always take a + `(const T *, size_t)` instead. You can still supply an optional helper + method that takes a `std::vector &` and then calls the former if + you anticipate it to be called very frequently. + * `std::string`: Unlike Java, these things aren't pooled, they're mutable + and copied. A common mistake is to take a `const std::string &` argument, + which forces all callers that supply a `const char *` to go thru a + strlen+malloc+copy that is possibly of no use to the callee. Prefer to + take a `const char *` instead for things that are names/identifiers, + especially if they possibly are compile-time constant. If you're + unsetting a string property, prefer to pass nullptr rather than an + empty string. (There are + [COW implementations](https://en.wikipedia.org/wiki/Copy-on-write), + but you can't rely on that). + * `std::map`: This is a costly data structure involving many + allocations. If all you wanted is for the caller to supply a list of + key/value pairs, take a `const char **` (yes, 2 stars!). Or + `const SimpleStruct *` instead, which allows the user to create this data + statically. +* Per-product configuration should be accomplished using an options struct + passed to the library's `firebase::::Initialize` function. Default + options should be provided by the options struct's default constructor. The + `Initialize` function should be overloaded with a version that does not take + the options struct (which is how the Google style guide prefers that we pass + default parameters). + + For example, + +```c++ + struct LibraryOptions { + LibraryOptions() : do_the_thing(false) {} + + bool do_the_thing; + }; + + InitResult Initialize(const App& app) { + return Initialize(app, LibraryOptions()); + } + InitResult Initialize(const App& app, const LibraryOptions& options); +``` + +* Don't make your client responsible for data you allocate or take ownership + of client data. Typical C++ APIs are written such that both the client + and the library own their own memory, and they take full responsibility for + managing it, regardless of what the other does. Any data exchanged is + typically done through weak references and copies. + An exception may be a file loading API where buffers exchanged may be really + big. If you are going to pass ownership, make this super obvious in all + comments and documentation (C++ programmers typically won't expect it), + and document which function should be used to free the data (free, delete, + or a custom one). + Alternatively, a simple way to pass ownership of a large new buffer to the + client is to ask the client to supply a std::string *, which you then + resize(), and write directly into its owned memory. This somewhat violates + the rule about use of std::string above, though. + +* Don't use exceptions. This one is worth mentioning separately. Though + exceptions are great in theory, in C++ hardly any libraries use them, and + many code-bases disallow them entirely. They also require the use of RTTI + which some environments turn off. Oh, yes, also don't use RTTI. + +* Go easy on templates when possible. Yes, they make your code more general, + but they also pull a lot of implementation detail into your API, lengthen + compile times and bloat binaries. In C++ they are glorified macros, so they + result in hard to understand errors, and can make correct use of your API + harder to understand. + +* Utilize C++11 features where appropriate. This project has adopted C++11, + and features such as `std::unique_ptr`, `std::shared_ptr`, `std::make_unique`, + and `std::move` are encouraged to improve code safety and readability. + However, avoid features from C++14 or newer standards. + +* Go easy on objectifying everything, and prefer value types. In languages + like Java it is common to give each "concept" your API deals with its own + class, such that methods on it have a nice home. In C++ this isn't always + desirable, because objects need to be managed, stored and allocated, and you + run into ownership/lifetime questions mentioned above. Instead: + + * For simple data, prefer their management to happen in the parent class + that owns them. Actions on them are methods in the parent. If at all + possible, prefer not to refer to them by pointer/reference (which + creates ownership and lifetime issues) but by index/id, or string + if not performance sensitive (for example, when referring to file + resources, since the cost of loading a file dwarfs the cost of a string + lookup). + + * If you must create objects, and objects are not heavyweight (only scalars + and non-owned pointers), make use of these objects by value (return by + value, receive by const reference). This makes ownership and lifetime + management trivial and efficient. + +* If at all possible, don't depend on external libraries. C++ compilation, + linking, dependency management, testing (especially cross platform) are + generally way harder than any other language. Every dependency is a potential + source of build complexity, conflicts, efficiency issues, and in general more + dependencies means less adoption. + + * Don't pull in large libraries (e.g. BOOST) just for your convenience, + especially if their use is exposed in headers. + + * Only use external libraries that have hard to replicate essential + functionality (e.g. compression, serialization, image loading, networking + etc.). Make sure to only access them in implementation files. + + * If possible, make a dependency optional, e.g. if what your API does + benefits from compression, make the client responsible for doing so, + or add an interface for it. Add sample glue code or an optional API for + working with the external library that is by default off in the build + files, and can be switched on if desired. + +* Take cross-platform-ness seriously: design the API to work on ALL platforms + even if you don't intend to supply implementations for all. Hide platform + issues in the implementation. Don't ever include platform specific headers in + your own headers. Have graceful fallback for platforms you don't support, such + that some level of building / testing can happen anywhere. + +* If your API is meant to be VERY widespread in use, VERY general, and very + simple (e.g. a compression API), consider making at least the API (if not all + of it) in C, as you'll reach an even wider audience. C has a more consistent + ABI and is easier to access from a variety of systems / languages. This is + especially useful if the library implementation is intended to be provided in + binary. + +* Be careful not to to use idioms from other languages that may be foreign to + C++. + + * An example of this is a "Builder" API (common in Java). Prefer to use + regular constructors, with additional set_ methods for less common + parameters if the number of them gets overwhelming. + +* Do not expose your own UI to the user as part of an API. Give the developer + the data to work with, and let them handle displaying it to the user in the + way they see fit. + + * Rare exceptions can be made to this rule on a case-by-case basis. For + example, authentication libraries may need to display a sign-in UI for the + user to enter their credentials. Your API may work with data owned by + Google or by the user (e.g. the user's contacts) that we don't want to + expose to the app; in those cases, it is appropriate to expose a UI (but + to limit the scope of the UI to the minimum necessary). + + * In these types of exceptional cases, the UI should be in an isolated + component, separate from the rest of the API. We do allow UIs to be + exposed to the user UI-specific libraries, e.g. FirebaseUI, which should + be open-source so developers can apply any customizations they need. + +# Game API Design + +### Performance matters + +Most of this is already encoded in C++ API design above, but it bears repeating: +C++ game programmers can be more fanatic about performance than you expect. + +It is easy to add a layer of usability on top of fast code, it is very hard to +impossible to "add performance" to an API that has performance issues baked into +its design. + +### Don't rely on state persisting for longer than one frame. + +Games have an interesting program structure very unlike apps or web pages: they +do all processing (and rendering) of almost all functionality of the game within +a *frame* (usually 1/60th of a second), and then start anew for the next frame. + +It is common for all or part of the state of a game to be wiped out from one +frame to the next (e.g when going into the menu, loading a new level, starting a +cut-scene..). + +The consequence of this is that the state kept between frames is the only record +of what is currently going on, and that managing this state is a source of +complexity, especially when part of it is reflected in external code: + +* Prefer API design that is stateless, or if it is stateful, is so only within a + frame (i.e. between the start of the frame and the start of the next one). + This really simplifies the client's use of your API: they can't forget to + "reset" your API's state whenever they change state themselves. + +* Prefer not to use cross-frame callbacks at all (non-escaping callbacks are + fine). Callbacks can be problematic in other contexts, but they're even more + problematic in games. Since they will execute at a future time, there's no + guarantee that the state that was there when the callback started will still + be there. There's no easy way to robustly "clear" pending callbacks that don't + make sense anymore when state changes. Instead, make your API based on + *polling*. + Yes, everyone learned in school that polling is bad because it uses CPU, but + that's what games are based on anyway: they check a LOT of things every frame + (and only at 60hz, which is very friendly compared to busy-wait polling). If + your API can be in various states of a state machine (e.g. a networking based + API), make sure the client can poll the state you're in. This can then easily + be translated to user feedback. + If you have to use asynchronous callbacks, see the section on async operations + below. + +* Be robust to the client needing to change state. If work done in your API + involves multiple steps, and the client never gets to the end of those steps + before starting a new sequence, don't be "stuck", but deal with this + gracefully. If the game's state got reset, it will have no record of what it + was doing before. Try to not make the client responsible for knowing what it + was doing. + +* Interaction with threading: + + * If you are going to use threading at all, make sure the use of that is + internal to the library, and any issues of thread-safety don't leak into + the client. Allow what appears to be synchronous access to a possibly + asynchronous implementation. If the asynchronous nature will be + externally visible, see the section on async operations below. + + * Games are typically hard to thread (since it’s hard to combine with its + per-frame nature), so the client typically should have full control over + it: it is often better to make a fully synchronous single-threaded library + and leave threading it to the client. Do not try to make your API itself + thread-safe, as your API is unlikely the threading boundary (if your + client is threaded, use of your library is typically isolated to one + thread, and they do not want to pay for synchronization cost they don't + use). + + * When you do spin up threads to reduce a workload, it is often a good idea + to do that once per frame, as avoid the above mentioned state based + problems, and while starting threads isn't cheap, you may find it not a + problem to do 60x per second. Alternatively you can pool them, and make + sure you have an explicit way to wait for their idleness at the end of a + frame. + +* Games typically use their own memory allocator (for efficiency, but also to be + able to control and budget usage on memory constrained systems). For this + reason, most game APIs tend to provide allocation hooks that will be used for + all internal allocation. This is even more important if you wish to be able to + transfer ownership of memory. + +* Generally prefer solutions that are low on total memory usage. Games are + always constrained on memory, and having your game be killed by the OS because + the library you use has decided it is efficient to cache everything is + problematic. + + * Prefer to recompute values when possible. + + * When you do cache, give the client control over total memory used for + this purpose. + + * Your memory usage should be predictable and ideally have no peaks. + +# Async Operations + +### Application Initiated Async Operations + +* Use the Future / State Pattern. +* Add a `*Result()` method for each async operation method to allow the caller + to poll and not save state. + +e.g. + +```c++ + // Start async operation. + Future SignInWithCredential(...); + // Get the result of the pending / last async operation for the method. + Future GetSignInWithCredentialResult(); + + Usage examples: + // call and register callback + auto& result = SignInWithCredential(); + result.set_callback([](result) { if (result == kComplete) { do_something_neat(); wake_up(); } }); + // wait + + // call and poll #1 (saving result) + auto& result = SignInWithCredential(); + while (result.value() != kComplete) { + // wait + } + + // call and poll #2 (result stored in API) + SignInWithCredential(); + while (GetSignInWithCredentialResult().value() != kComplete) { + } +``` + +### API Initiated Async Event Handling + +* Follow the + [listener / observer pattern](https://en.wikipedia.org/wiki/Observer_pattern) + for API initiated (i.e where the caller doesn't initiate the event) + async events. +* Provide a queued interface to allow users to poll for events. + +e.g. + +```c++ + class GcmListener { + public: + virtual void OnDeletedMessage() {} + virtual void OnMessageReceived(const MessageReceivedData* data) {} + }; + + class GcmListenerQueue : private GcmListener { + public: + enum EventType { + kEventTypeMessageDeleted, + kEventTypeMessageReceived, + }; + + struct Event { + EventType type; + MessageReceivedData data; // Set when type == kEventTypeMessageReceived + }; + + // Returns true when an event is retrieved from the queue. + bool PollEvent(Event *event); + }; + + // Wait for callbacks + class MyListener : public GcmListener { + public: + virtual void OnDeletedMessage() { /* do stuff */ } + virtual void OnMessageReceived() { /* display message */ } + }; + MyListener listener; + gcm::Initialize(app, &listener); + + // Poll + GcmListenerQueue queued_listener; + gcm::Initialize(app, &queued_listener); + GcmListenerQueue::Event event; + while (queued_listener(&event)) { + switch (event.type) { + case kEventTypeMessageDeleted: + // do stuff + break; + case kEventTypeMessageReceived: + // display event.data + break; + } + } +``` + +# Data Types + +* Time: [Milliseconds since the epoch](https://en.wikipedia.org/wiki/Unix_time) + +# More on C++ and games + +* Performance Oriented C++ for Game Developers (very incomplete). From bfec241b91e545ab6db9afbda3ad57c9e73148f5 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 23 Jun 2025 18:09:39 +0000 Subject: [PATCH 3/9] Update async operation pattern to *LastResult in style guide --- STYLE_GUIDE.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/STYLE_GUIDE.md b/STYLE_GUIDE.md index bcbd3b06ae..d27037cf5e 100644 --- a/STYLE_GUIDE.md +++ b/STYLE_GUIDE.md @@ -279,7 +279,7 @@ complexity, especially when part of it is reflected in external code: ### Application Initiated Async Operations * Use the Future / State Pattern. -* Add a `*Result()` method for each async operation method to allow the caller +* Add a `*LastResult()` method for each async operation method to allow the caller to poll and not save state. e.g. @@ -288,7 +288,7 @@ e.g. // Start async operation. Future SignInWithCredential(...); // Get the result of the pending / last async operation for the method. - Future GetSignInWithCredentialResult(); + Future SignInWithCredentialLastResult(); Usage examples: // call and register callback @@ -304,7 +304,7 @@ e.g. // call and poll #2 (result stored in API) SignInWithCredential(); - while (GetSignInWithCredentialResult().value() != kComplete) { + while (SignInWithCredentialLastResult().value() != kComplete) { } ``` From 073dd6296b151b64569c4e41e2bf8b9ec615585a Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 23 Jun 2025 18:52:13 +0000 Subject: [PATCH 4/9] Reformat STYLE_GUIDE.md to wrap at 80 characters --- Jules.md | 4 +- STYLE_GUIDE.md | 377 +++++++++++++++++++++++++------------------------ 2 files changed, 197 insertions(+), 184 deletions(-) diff --git a/Jules.md b/Jules.md index 7da3ccf1b0..f60b83cb57 100644 --- a/Jules.md +++ b/Jules.md @@ -300,9 +300,11 @@ API documentation. ## Coding Style +* **Firebase C++ Style Guide**: For specific C++ API design and coding + conventions relevant to this SDK, refer to the [STYLE_GUIDE.md](STYLE_GUIDE.md). * **Google C++ Style Guide**: Adhere to the [Google C++ Style Guide](https://google.github.io/styleguide/cppguide.html) - as mentioned in `CONTRIBUTING.md`. + as mentioned in `CONTRIBUTING.md` for general C++ style. * **Formatting**: Use `python3 scripts/format_code.py -git_diff -verbose` to format your code before committing. diff --git a/STYLE_GUIDE.md b/STYLE_GUIDE.md index d27037cf5e..a2a5d4c946 100644 --- a/STYLE_GUIDE.md +++ b/STYLE_GUIDE.md @@ -21,43 +21,43 @@ represent their data, and structure their code. An API that forces the use of a feature or structure the client doesn't use will be very unwelcome. A good API uses only the simplest common denominator -data types and features, and will be useable by all. This can generally be done -with minimal impact on your API’s simplicity, or at least should form the -baseline API. +data types and features, and will be useable by all. This can generally be +done with minimal impact on your API’s simplicity, or at least should form +the baseline API. Examples of typical Do's and Don'ts: -* Don't force the use of a particular data structure to supply or receive data. - Typical examples: +* Don't force the use of a particular data structure to supply or receive + data. Typical examples: * `std::vector`: If the client doesn't have the data already in a - `std::vector` (or only wants to use part of a vector), they are forced - to copy/allocate a new one, and C++ programmers don't like unnecessary - copies/allocations. - Instead, your primary interface should always take a - `(const T *, size_t)` instead. You can still supply an optional helper - method that takes a `std::vector &` and then calls the former if - you anticipate it to be called very frequently. + `std::vector` (or only wants to use part of a vector), they are forced + to copy/allocate a new one, and C++ programmers don't like unnecessary + copies/allocations. + Instead, your primary interface should always take a + `(const T *, size_t)` instead. You can still supply an optional helper + method that takes a `std::vector &` and then calls the former if + you anticipate it to be called very frequently. * `std::string`: Unlike Java, these things aren't pooled, they're mutable - and copied. A common mistake is to take a `const std::string &` argument, - which forces all callers that supply a `const char *` to go thru a - strlen+malloc+copy that is possibly of no use to the callee. Prefer to - take a `const char *` instead for things that are names/identifiers, - especially if they possibly are compile-time constant. If you're - unsetting a string property, prefer to pass nullptr rather than an - empty string. (There are - [COW implementations](https://en.wikipedia.org/wiki/Copy-on-write), - but you can't rely on that). + and copied. A common mistake is to take a `const std::string &` + argument, which forces all callers that supply a `const char *` to go + thru a strlen+malloc+copy that is possibly of no use to the callee. + Prefer to take a `const char *` instead for things that are + names/identifiers, especially if they possibly are compile-time + constant. If you're unsetting a string property, prefer to pass + nullptr rather than an empty string. (There are + [COW implementations](https://en.wikipedia.org/wiki/Copy-on-write), + but you can't rely on that). * `std::map`: This is a costly data structure involving many - allocations. If all you wanted is for the caller to supply a list of - key/value pairs, take a `const char **` (yes, 2 stars!). Or - `const SimpleStruct *` instead, which allows the user to create this data - statically. + allocations. If all you wanted is for the caller to supply a list of + key/value pairs, take a `const char **` (yes, 2 stars!). Or + `const SimpleStruct *` instead, which allows the user to create this + data statically. * Per-product configuration should be accomplished using an options struct - passed to the library's `firebase::::Initialize` function. Default - options should be provided by the options struct's default constructor. The - `Initialize` function should be overloaded with a version that does not take - the options struct (which is how the Google style guide prefers that we pass - default parameters). + passed to the library's `firebase::::Initialize` function. + Default options should be provided by the options struct's default + constructor. The `Initialize` function should be overloaded with a version + that does not take the options struct (which is how the Google style guide + prefers that we pass default parameters). For example, @@ -76,18 +76,18 @@ Examples of typical Do's and Don'ts: * Don't make your client responsible for data you allocate or take ownership of client data. Typical C++ APIs are written such that both the client - and the library own their own memory, and they take full responsibility for - managing it, regardless of what the other does. Any data exchanged is + and the library own their own memory, and they take full responsibility + for managing it, regardless of what the other does. Any data exchanged is typically done through weak references and copies. - An exception may be a file loading API where buffers exchanged may be really - big. If you are going to pass ownership, make this super obvious in all - comments and documentation (C++ programmers typically won't expect it), - and document which function should be used to free the data (free, delete, - or a custom one). - Alternatively, a simple way to pass ownership of a large new buffer to the - client is to ask the client to supply a std::string *, which you then - resize(), and write directly into its owned memory. This somewhat violates - the rule about use of std::string above, though. + An exception may be a file loading API where buffers exchanged may be + really big. If you are going to pass ownership, make this super obvious + in all comments and documentation (C++ programmers typically won't + expect it), and document which function should be used to free the data + (free, delete, or a custom one). + Alternatively, a simple way to pass ownership of a large new buffer to + the client is to ask the client to supply a std::string *, which you then + resize(), and write directly into its owned memory. This somewhat + violates the rule about use of std::string above, though. * Don't use exceptions. This one is worth mentioning separately. Though exceptions are great in theory, in C++ hardly any libraries use them, and @@ -96,181 +96,192 @@ Examples of typical Do's and Don'ts: * Go easy on templates when possible. Yes, they make your code more general, but they also pull a lot of implementation detail into your API, lengthen - compile times and bloat binaries. In C++ they are glorified macros, so they - result in hard to understand errors, and can make correct use of your API - harder to understand. + compile times and bloat binaries. In C++ they are glorified macros, so + they result in hard to understand errors, and can make correct use of + your API harder to understand. * Utilize C++11 features where appropriate. This project has adopted C++11, - and features such as `std::unique_ptr`, `std::shared_ptr`, `std::make_unique`, - and `std::move` are encouraged to improve code safety and readability. - However, avoid features from C++14 or newer standards. + and features such as `std::unique_ptr`, `std::shared_ptr`, + `std::make_unique`, and `std::move` are encouraged to improve code safety + and readability. However, avoid features from C++14 or newer standards. * Go easy on objectifying everything, and prefer value types. In languages like Java it is common to give each "concept" your API deals with its own - class, such that methods on it have a nice home. In C++ this isn't always - desirable, because objects need to be managed, stored and allocated, and you - run into ownership/lifetime questions mentioned above. Instead: - - * For simple data, prefer their management to happen in the parent class - that owns them. Actions on them are methods in the parent. If at all - possible, prefer not to refer to them by pointer/reference (which - creates ownership and lifetime issues) but by index/id, or string - if not performance sensitive (for example, when referring to file - resources, since the cost of loading a file dwarfs the cost of a string - lookup). - - * If you must create objects, and objects are not heavyweight (only scalars - and non-owned pointers), make use of these objects by value (return by - value, receive by const reference). This makes ownership and lifetime - management trivial and efficient. + class, such that methods on it have a nice home. In C++ this isn't + always desirable, because objects need to be managed, stored and + allocated, and you run into ownership/lifetime questions mentioned above. + Instead: + + * For simple data, prefer their management to happen in the parent + class that owns them. Actions on them are methods in the parent. If + at all possible, prefer not to refer to them by pointer/reference + (which creates ownership and lifetime issues) but by index/id, or + string if not performance sensitive (for example, when referring to + file resources, since the cost of loading a file dwarfs the cost of + a string lookup). + + * If you must create objects, and objects are not heavyweight (only + scalars and non-owned pointers), make use of these objects by value + (return by value, receive by const reference). This makes ownership + and lifetime management trivial and efficient. * If at all possible, don't depend on external libraries. C++ compilation, linking, dependency management, testing (especially cross platform) are - generally way harder than any other language. Every dependency is a potential - source of build complexity, conflicts, efficiency issues, and in general more - dependencies means less adoption. + generally way harder than any other language. Every dependency is a + potential source of build complexity, conflicts, efficiency issues, and + in general more dependencies means less adoption. - * Don't pull in large libraries (e.g. BOOST) just for your convenience, - especially if their use is exposed in headers. + * Don't pull in large libraries (e.g. BOOST) just for your + convenience, especially if their use is exposed in headers. * Only use external libraries that have hard to replicate essential - functionality (e.g. compression, serialization, image loading, networking - etc.). Make sure to only access them in implementation files. + functionality (e.g. compression, serialization, image loading, + networking etc.). Make sure to only access them in implementation + files. * If possible, make a dependency optional, e.g. if what your API does benefits from compression, make the client responsible for doing so, - or add an interface for it. Add sample glue code or an optional API for - working with the external library that is by default off in the build - files, and can be switched on if desired. + or add an interface for it. Add sample glue code or an optional API + for working with the external library that is by default off in the + build files, and can be switched on if desired. -* Take cross-platform-ness seriously: design the API to work on ALL platforms - even if you don't intend to supply implementations for all. Hide platform - issues in the implementation. Don't ever include platform specific headers in - your own headers. Have graceful fallback for platforms you don't support, such - that some level of building / testing can happen anywhere. +* Take cross-platform-ness seriously: design the API to work on ALL + platforms even if you don't intend to supply implementations for all. + Hide platform issues in the implementation. Don't ever include platform + specific headers in your own headers. Have graceful fallback for + platforms you don't support, such that some level of building / testing + can happen anywhere. * If your API is meant to be VERY widespread in use, VERY general, and very - simple (e.g. a compression API), consider making at least the API (if not all - of it) in C, as you'll reach an even wider audience. C has a more consistent - ABI and is easier to access from a variety of systems / languages. This is - especially useful if the library implementation is intended to be provided in - binary. - -* Be careful not to to use idioms from other languages that may be foreign to - C++. - - * An example of this is a "Builder" API (common in Java). Prefer to use - regular constructors, with additional set_ methods for less common - parameters if the number of them gets overwhelming. - -* Do not expose your own UI to the user as part of an API. Give the developer - the data to work with, and let them handle displaying it to the user in the - way they see fit. - - * Rare exceptions can be made to this rule on a case-by-case basis. For - example, authentication libraries may need to display a sign-in UI for the - user to enter their credentials. Your API may work with data owned by - Google or by the user (e.g. the user's contacts) that we don't want to - expose to the app; in those cases, it is appropriate to expose a UI (but - to limit the scope of the UI to the minimum necessary). + simple (e.g. a compression API), consider making at least the API (if + not all of it) in C, as you'll reach an even wider audience. C has a + more consistent ABI and is easier to access from a variety of systems / + languages. This is especially useful if the library implementation is + intended to be provided in binary. + +* Be careful not to to use idioms from other languages that may be foreign + to C++. + + * An example of this is a "Builder" API (common in Java). Prefer to + use regular constructors, with additional set_ methods for less + common parameters if the number of them gets overwhelming. + +* Do not expose your own UI to the user as part of an API. Give the + developer the data to work with, and let them handle displaying it to + the user in the way they see fit. + + * Rare exceptions can be made to this rule on a case-by-case basis. + For example, authentication libraries may need to display a sign-in + UI for the user to enter their credentials. Your API may work with + data owned by Google or by the user (e.g. the user's contacts) that + we don't want to expose to the app; in those cases, it is + appropriate to expose a UI (but to limit the scope of the UI to the + minimum necessary). * In these types of exceptional cases, the UI should be in an isolated component, separate from the rest of the API. We do allow UIs to be - exposed to the user UI-specific libraries, e.g. FirebaseUI, which should - be open-source so developers can apply any customizations they need. + exposed to the user UI-specific libraries, e.g. FirebaseUI, which + should be open-source so developers can apply any customizations + they need. # Game API Design ### Performance matters -Most of this is already encoded in C++ API design above, but it bears repeating: -C++ game programmers can be more fanatic about performance than you expect. +Most of this is already encoded in C++ API design above, but it bears +repeating: C++ game programmers can be more fanatic about performance than +you expect. -It is easy to add a layer of usability on top of fast code, it is very hard to -impossible to "add performance" to an API that has performance issues baked into -its design. +It is easy to add a layer of usability on top of fast code, it is very +hard to impossible to "add performance" to an API that has performance +issues baked into its design. ### Don't rely on state persisting for longer than one frame. -Games have an interesting program structure very unlike apps or web pages: they -do all processing (and rendering) of almost all functionality of the game within -a *frame* (usually 1/60th of a second), and then start anew for the next frame. - -It is common for all or part of the state of a game to be wiped out from one -frame to the next (e.g when going into the menu, loading a new level, starting a -cut-scene..). - -The consequence of this is that the state kept between frames is the only record -of what is currently going on, and that managing this state is a source of -complexity, especially when part of it is reflected in external code: - -* Prefer API design that is stateless, or if it is stateful, is so only within a - frame (i.e. between the start of the frame and the start of the next one). - This really simplifies the client's use of your API: they can't forget to - "reset" your API's state whenever they change state themselves. - -* Prefer not to use cross-frame callbacks at all (non-escaping callbacks are - fine). Callbacks can be problematic in other contexts, but they're even more - problematic in games. Since they will execute at a future time, there's no - guarantee that the state that was there when the callback started will still - be there. There's no easy way to robustly "clear" pending callbacks that don't - make sense anymore when state changes. Instead, make your API based on - *polling*. - Yes, everyone learned in school that polling is bad because it uses CPU, but - that's what games are based on anyway: they check a LOT of things every frame - (and only at 60hz, which is very friendly compared to busy-wait polling). If - your API can be in various states of a state machine (e.g. a networking based - API), make sure the client can poll the state you're in. This can then easily - be translated to user feedback. - If you have to use asynchronous callbacks, see the section on async operations - below. - -* Be robust to the client needing to change state. If work done in your API - involves multiple steps, and the client never gets to the end of those steps - before starting a new sequence, don't be "stuck", but deal with this - gracefully. If the game's state got reset, it will have no record of what it - was doing before. Try to not make the client responsible for knowing what it - was doing. +Games have an interesting program structure very unlike apps or web pages: +they do all processing (and rendering) of almost all functionality of the +game within a *frame* (usually 1/60th of a second), and then start anew +for the next frame. + +It is common for all or part of the state of a game to be wiped out from +one frame to the next (e.g when going into the menu, loading a new level, +starting a cut-scene..). + +The consequence of this is that the state kept between frames is the only +record of what is currently going on, and that managing this state is a +source of complexity, especially when part of it is reflected in external +code: + +* Prefer API design that is stateless, or if it is stateful, is so only + within a frame (i.e. between the start of the frame and the start of + the next one). This really simplifies the client's use of your API: + they can't forget to "reset" your API's state whenever they change + state themselves. + +* Prefer not to use cross-frame callbacks at all (non-escaping callbacks + are fine). Callbacks can be problematic in other contexts, but they're + even more problematic in games. Since they will execute at a future + time, there's no guarantee that the state that was there when the + callback started will still be there. There's no easy way to robustly + "clear" pending callbacks that don't make sense anymore when state + changes. Instead, make your API based on *polling*. + Yes, everyone learned in school that polling is bad because it uses + CPU, but that's what games are based on anyway: they check a LOT of + things every frame (and only at 60hz, which is very friendly compared + to busy-wait polling). If your API can be in various states of a state + machine (e.g. a networking based API), make sure the client can poll + the state you're in. This can then easily be translated to user + feedback. + If you have to use asynchronous callbacks, see the section on async + operations below. + +* Be robust to the client needing to change state. If work done in your + API involves multiple steps, and the client never gets to the end of + those steps before starting a new sequence, don't be "stuck", but deal + with this gracefully. If the game's state got reset, it will have no + record of what it was doing before. Try to not make the client + responsible for knowing what it was doing. * Interaction with threading: - * If you are going to use threading at all, make sure the use of that is - internal to the library, and any issues of thread-safety don't leak into - the client. Allow what appears to be synchronous access to a possibly - asynchronous implementation. If the asynchronous nature will be - externally visible, see the section on async operations below. - - * Games are typically hard to thread (since it’s hard to combine with its - per-frame nature), so the client typically should have full control over - it: it is often better to make a fully synchronous single-threaded library - and leave threading it to the client. Do not try to make your API itself - thread-safe, as your API is unlikely the threading boundary (if your - client is threaded, use of your library is typically isolated to one - thread, and they do not want to pay for synchronization cost they don't - use). - - * When you do spin up threads to reduce a workload, it is often a good idea - to do that once per frame, as avoid the above mentioned state based - problems, and while starting threads isn't cheap, you may find it not a - problem to do 60x per second. Alternatively you can pool them, and make - sure you have an explicit way to wait for their idleness at the end of a - frame. - -* Games typically use their own memory allocator (for efficiency, but also to be - able to control and budget usage on memory constrained systems). For this - reason, most game APIs tend to provide allocation hooks that will be used for - all internal allocation. This is even more important if you wish to be able to - transfer ownership of memory. - -* Generally prefer solutions that are low on total memory usage. Games are - always constrained on memory, and having your game be killed by the OS because - the library you use has decided it is efficient to cache everything is - problematic. + * If you are going to use threading at all, make sure the use of that + is internal to the library, and any issues of thread-safety don't + leak into the client. Allow what appears to be synchronous access + to a possibly asynchronous implementation. If the asynchronous + nature will be externally visible, see the section on async + operations below. + + * Games are typically hard to thread (since it’s hard to combine with + its per-frame nature), so the client typically should have full + control over it: it is often better to make a fully synchronous + single-threaded library and leave threading it to the client. Do + not try to make your API itself thread-safe, as your API is + unlikely the threading boundary (if your client is threaded, use + of your library is typically isolated to one thread, and they do + not want to pay for synchronization cost they don't use). + + * When you do spin up threads to reduce a workload, it is often a + good idea to do that once per frame, as avoid the above mentioned + state based problems, and while starting threads isn't cheap, you + may find it not a problem to do 60x per second. Alternatively you + can pool them, and make sure you have an explicit way to wait for + their idleness at the end of a frame. + +* Games typically use their own memory allocator (for efficiency, but + also to be able to control and budget usage on memory constrained + systems). For this reason, most game APIs tend to provide allocation + hooks that will be used for all internal allocation. This is even more + important if you wish to be able to transfer ownership of memory. + +* Generally prefer solutions that are low on total memory usage. Games + are always constrained on memory, and having your game be killed by + the OS because the library you use has decided it is efficient to + cache everything is problematic. * Prefer to recompute values when possible. - * When you do cache, give the client control over total memory used for - this purpose. + * When you do cache, give the client control over total memory used + for this purpose. * Your memory usage should be predictable and ideally have no peaks. @@ -279,8 +290,8 @@ complexity, especially when part of it is reflected in external code: ### Application Initiated Async Operations * Use the Future / State Pattern. -* Add a `*LastResult()` method for each async operation method to allow the caller - to poll and not save state. +* Add a `*LastResult()` method for each async operation method to allow + the caller to poll and not save state. e.g. From 45c2a5a463788f68c4ff10e91b099dcd030e8e9c Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 23 Jun 2025 21:44:25 +0000 Subject: [PATCH 5/9] Reformat Jules.md to wrap at 80 characters --- Jules.md | 525 +++++++++++++++++++++++++++++-------------------------- 1 file changed, 273 insertions(+), 252 deletions(-) diff --git a/Jules.md b/Jules.md index f60b83cb57..344f59db1f 100644 --- a/Jules.md +++ b/Jules.md @@ -12,8 +12,8 @@ platform (Android, iOS, tvOS, macOS, Windows, Linux), refer to the official The Firebase C++ SDKs for desktop platforms (Windows, Linux, macOS) are entirely open source and hosted in the main `firebase/firebase-cpp-sdk` GitHub repository. The C++ SDKs for mobile platforms (iOS, tvOS, Android) are built -on top of the respective native open-source Firebase SDKs (Firebase iOS SDK and -Firebase Android SDK). +on top of the respective native open-source Firebase SDKs (Firebase iOS SDK +and Firebase Android SDK). The goal is to enable agents to understand the existing conventions and contribute effectively to the codebase. @@ -29,13 +29,13 @@ instructions for your specific platform. * **CMake**: Version 3.7 or newer. * **Python**: Version 3.7 or newer. * **Abseil-py**: Python package. -* **OpenSSL**: Required for Realtime Database and Cloud Firestore (especially - for desktop builds). -* **Android SDK & NDK**: Required for building Android libraries. `sdkmanager` - can be used for installation. CMake for Android (version 3.10.2 - recommended) is also needed. -* **(Windows Only) Strings**: From Microsoft Sysinternals, required for Android - builds on Windows. +* **OpenSSL**: Required for Realtime Database and Cloud Firestore + (especially for desktop builds). +* **Android SDK & NDK**: Required for building Android libraries. + `sdkmanager` can be used for installation. CMake for Android (version + 3.10.2 recommended) is also needed. +* **(Windows Only) Strings**: From Microsoft Sysinternals, required for + Android builds on Windows. * **Cocoapods**: Required for building iOS or tvOS libraries. ## Building the SDK @@ -44,7 +44,8 @@ The SDK uses CMake for C++ compilation and Gradle for Android-specific parts. ### CMake (Desktop, iOS, tvOS) -1. Create a build directory (e.g., `mkdir desktop_build && cd desktop_build`). +1. Create a build directory (e.g., + `mkdir desktop_build && cd desktop_build`). 2. Run CMake to configure: `cmake ..` * For iOS: `cmake -DCMAKE_TOOLCHAIN_FILE=../cmake/toolchains/ios.cmake ..` @@ -61,23 +62,25 @@ third-party dependency locations. ### Gradle (Android) -Each Firebase C++ library is a Gradle subproject. To build a specific library -(e.g., Analytics): +Each Firebase C++ library is a Gradle subproject. To build a specific +library (e.g., Analytics): ```bash ./gradlew :analytics:assembleRelease ``` -This command should be run from the root of the repository. Proguard files are -generated in each library's build directory (e.g., +This command should be run from the root of the repository. Proguard files +are generated in each library's build directory (e.g., `analytics/build/analytics.pro`). ### Desktop Platform Setup Details -When setting up for desktop, if you are using an iOS `GoogleService-Info.plist` -file, convert it to the required `google-services-desktop.json` using the -script: `python generate_xml_from_google_services_json.py --plist -i GoogleService-Info.plist` -(run this from the script's directory, ensuring the plist file is accessible). +When setting up for desktop, if you are using an iOS +`GoogleService-Info.plist` file, convert it to the required +`google-services-desktop.json` using the script: +`python generate_xml_from_google_services_json.py --plist -i GoogleService-Info.plist` +(run this from the script's directory, ensuring the plist file is +accessible). The desktop SDK searches for configuration files in the current working directory, first for `google-services-desktop.json`, then for @@ -105,8 +108,8 @@ target_link_libraries([[Your CMake Target]] firebase_analytics firebase_app) ### Android Gradle Projects -In addition to CMake setup, use `Android/firebase_dependencies.gradle` in your -`build.gradle`: +In addition to CMake setup, use `Android/firebase_dependencies.gradle` in +your `build.gradle`: ```gradle apply from: "[[Path to the Firebase C++ SDK]]/Android/firebase_dependencies.gradle" @@ -124,18 +127,18 @@ For more detailed instructions and examples, always refer to the main ## Testing Strategy The primary method for testing in this repository is through **integration -tests** for each Firebase library. While the `README.md` mentions unit tests -run via CTest, the current and preferred approach is to ensure comprehensive -coverage within the integration tests. +tests** for each Firebase library. While the `README.md` mentions unit +tests run via CTest, the current and preferred approach is to ensure +comprehensive coverage within the integration tests. ## Running Tests -* **Integration Test Location**: Integration tests for each Firebase product - (e.g., Firestore, Auth) are typically located in the `integration_test/` - directory within that product's module (e.g., +* **Integration Test Location**: Integration tests for each Firebase + product (e.g., Firestore, Auth) are typically located in the + `integration_test/` directory within that product's module (e.g., `firestore/integration_test/`). -* **Test Scripts**: The root of the repository contains scripts for running - tests on various platforms, such as: +* **Test Scripts**: The root of the repository contains scripts for + running tests on various platforms, such as: * `test_windows_x32.bat` * `test_windows_x64.bat` * `test_linux.sh` @@ -143,9 +146,9 @@ coverage within the integration tests. * `test_mac_ios.sh` * `test_mac_ios_simulator.sh` - These scripts typically build the SDKs and then execute the relevant tests - (primarily integration tests) via CTest or other platform-specific test - runners. + These scripts typically build the SDKs and then execute the relevant + tests (primarily integration tests) via CTest or other platform-specific + test runners. ## Writing Tests @@ -153,8 +156,8 @@ When adding new features or fixing bugs: * Prioritize adding or updating integration tests within the respective product's `integration_test/` directory. -* Ensure tests cover the new functionality thoroughly and verify interactions - with the Firebase backend or other relevant components. +* Ensure tests cover the new functionality thoroughly and verify + interactions with the Firebase backend or other relevant components. * Follow existing patterns within the integration tests for consistency. # API Surface @@ -169,57 +172,58 @@ Database). 1. **`firebase::App`**: This is the central entry point for the SDK. * It must be initialized first using `firebase::App::Create(...)`. - * On Android, this requires passing the JNI environment (`JNIEnv*`) and - the Android Activity (`jobject`). - * `firebase::AppOptions` can be used to configure the app with specific - parameters if not relying on a `google-services.json` or + * On Android, this requires passing the JNI environment (`JNIEnv*`) + and the Android Activity (`jobject`). + * `firebase::AppOptions` can be used to configure the app with + specific parameters if not relying on a `google-services.json` or `GoogleService-Info.plist` file. -2. **Service Instances**: Once `firebase::App` is initialized, you generally - obtain instances of specific Firebase services using a static `GetInstance()` - method on the service's class, passing the `firebase::App` object. +2. **Service Instances**: Once `firebase::App` is initialized, you + generally obtain instances of specific Firebase services using a static + `GetInstance()` method on the service's class, passing the + `firebase::App` object. * Examples for services like Auth, Database, Storage, Firestore: * `firebase::auth::Auth* auth = firebase::auth::Auth::GetAuth(app, &init_result);` * `firebase::database::Database* database = firebase::database::Database::GetInstance(app, &init_result);` * `firebase::storage::Storage* storage = firebase::storage::Storage::GetInstance(app, &init_result);` * Always check the `init_result` (an `InitResult` enum, often - `firebase::kInitResultSuccess` on success) to ensure these services - were initialized successfully. - * **Note on Analytics**: Some products, like Firebase Analytics, have a - different pattern. Analytics is typically initialized with + `firebase::kInitResultSuccess` on success) to ensure these + services were initialized successfully. + * **Note on Analytics**: Some products, like Firebase Analytics, have + a different pattern. Analytics is typically initialized with `firebase::analytics::Initialize(const firebase::App& app)` (often handled automatically for the default `App` instance). After this, - Analytics functions (e.g., `firebase::analytics::LogEvent(...)`) are - called as global functions within the `firebase::analytics` namespace, - rather than on an instance object obtained via `GetInstance()`. - Refer to the specific product's header file for its exact - initialization mechanism if it deviates from the common `GetInstance(app, ...)` - pattern. + Analytics functions (e.g., `firebase::analytics::LogEvent(...)`) + are called as global functions within the `firebase::analytics` + namespace, rather than on an instance object obtained via + `GetInstance()`. Refer to the specific product's header file for + its exact initialization mechanism if it deviates from the common + `GetInstance(app, ...)` pattern. ### Asynchronous Operations: `firebase::Future` -All asynchronous operations in the SDK return a `firebase::Future` object, -where `T` is the type of the expected result. +All asynchronous operations in the SDK return a `firebase::Future` +object, where `T` is the type of the expected result. -* **Status Checking**: Use `future.status()` to check if the operation is - `kFutureStatusPending`, `kFutureStatusComplete`, or +* **Status Checking**: Use `future.status()` to check if the operation + is `kFutureStatusPending`, `kFutureStatusComplete`, or `kFutureStatusInvalid`. * **Getting Results**: Once `future.status() == kFutureStatusComplete`: * Check for errors: `future.error()`. A value of `0` (e.g., - `firebase::auth::kAuthErrorNone`, `firebase::database::kErrorNone`) - usually indicates success. + `firebase::auth::kAuthErrorNone`, + `firebase::database::kErrorNone`) usually indicates success. * Get the error message: `future.error_message()`. - * Get the result: `future.result()`. This returns a pointer to the result - object of type `T`. The result is only valid if `future.error()` - indicates success. + * Get the result: `future.result()`. This returns a pointer to the + result object of type `T`. The result is only valid if + `future.error()` indicates success. * **Completion Callbacks**: Use `future.OnCompletion(...)` to register a - callback function (lambda or function pointer) that will be invoked when - the future completes. The callback receives the completed future as an - argument. + callback function (lambda or function pointer) that will be invoked + when the future completes. The callback receives the completed future + as an argument. ### Core Classes and Operations (Examples from Auth and Database) -While each Firebase product has its own specific classes, the following examples -illustrate common API patterns: +While each Firebase product has its own specific classes, the following +examples illustrate common API patterns: * **`firebase::auth::Auth`**: The main entry point for Firebase Authentication. @@ -232,20 +236,21 @@ illustrate common API patterns: successful and the pointer is checked). * Example: `firebase::auth::User* current_user = auth->current_user();` * Methods for user creation/authentication: - `CreateUserWithEmailAndPassword()`, `SignInWithEmailAndPassword()`, - `SignInWithCredential()`. + `CreateUserWithEmailAndPassword()`, + `SignInWithEmailAndPassword()`, `SignInWithCredential()`. * **`firebase::auth::User`**: - * Represents a user account. Data is typically accessed via its methods. + * Represents a user account. Data is typically accessed via its + methods. * Methods for profile updates: `UpdateEmail()`, `UpdatePassword()`, `UpdateUserProfile()`. * Other operations: `SendEmailVerification()`, `Delete()`. * **`firebase::database::Database`**: The main entry point for Firebase Realtime Database. - * Used to get `DatabaseReference` objects to specific locations in the - database. + * Used to get `DatabaseReference` objects to specific locations in + the database. * **`firebase::database::DatabaseReference`**: - * Represents a reference to a specific location (path) in the Realtime - Database. + * Represents a reference to a specific location (path) in the + Realtime Database. * Methods for navigation: `Child()`, `Parent()`, `Root()`. * Methods for data manipulation: `GetValue()`, `SetValue()`, `UpdateChildren()`, `RemoveValue()`. @@ -253,19 +258,19 @@ illustrate common API patterns: * **`firebase::database::Query`**: * Used to retrieve data from a Realtime Database location based on specific criteria. Obtained from a `DatabaseReference`. - * **Filtering**: `OrderByChild()`, `OrderByKey()`, `OrderByValue()`, - `EqualTo()`, `StartAt()`, `EndAt()`. + * **Filtering**: `OrderByChild()`, `OrderByKey()`, + `OrderByValue()`, `EqualTo()`, `StartAt()`, `EndAt()`. * **Limiting**: `LimitToFirst()`, `LimitToLast()`. * Execution: `GetValue()` returns a `Future`. -* **`firebase::database::DataSnapshot`**: Contains data read from a Realtime - Database location (either directly or as a result of a query). Accessed - via `future.result()` or through listeners. - * Methods: `value()` (returns a `firebase::Variant` representing the - data), `children_count()`, `children()`, `key()`, `exists()`. +* **`firebase::database::DataSnapshot`**: Contains data read from a + Realtime Database location (either directly or as a result of a + query). Accessed via `future.result()` or through listeners. + * Methods: `value()` (returns a `firebase::Variant` representing + the data), `children_count()`, `children()`, `key()`, `exists()`. * **`firebase::Variant`**: A type that can hold various data types like integers, strings, booleans, vectors (arrays), and maps (objects), - commonly used for reading and writing data with Realtime Database and other - services. + commonly used for reading and writing data with Realtime Database and + other services. * **Operation-Specific Options**: Some operations might take optional parameters to control behavior, though not always through a dedicated "Options" class like Firestore's `SetOptions`. For example, @@ -273,209 +278,221 @@ illustrate common API patterns: ### Listeners for Real-time Updates -Many Firebase services support real-time data synchronization using listeners. +Many Firebase services support real-time data synchronization using +listeners. * **`firebase::database::ValueListener` / - `firebase::database::ChildListener`**: Implemented by the developer and - registered with a `DatabaseReference`. + `firebase::database::ChildListener`**: Implemented by the developer + and registered with a `DatabaseReference`. * `ValueListener::OnValueChanged(const firebase::database::DataSnapshot& snapshot)` is called when the data at that location changes. - * `ChildListener` has methods like `OnChildAdded()`, `OnChildChanged()`, - `OnChildRemoved()`. -* **`firebase::auth::AuthStateListener`**: Implemented by the developer and - registered with `firebase::auth::Auth`. - * `AuthStateListener::OnAuthStateChanged(firebase::auth::Auth* auth)` is - called when the user's sign-in state changes. + * `ChildListener` has methods like `OnChildAdded()`, + `OnChildChanged()`, `OnChildRemoved()`. +* **`firebase::auth::AuthStateListener`**: Implemented by the developer + and registered with `firebase::auth::Auth`. + * `AuthStateListener::OnAuthStateChanged(firebase::auth::Auth* auth)` + is called when the user's sign-in state changes. * **Removing Listeners**: Listeners are typically removed by passing the - listener instance to a corresponding `Remove...Listener()` method (e.g., - `reference->RemoveValueListener(my_listener);`, + listener instance to a corresponding `Remove...Listener()` method + (e.g., `reference->RemoveValueListener(my_listener);`, `auth->RemoveAuthStateListener(my_auth_listener);`). -This overview provides a general understanding. Always refer to the specific -header files in `firebase/app/client/cpp/include/firebase/` and -`firebase/product_name/client/cpp/include/firebase/product_name/` for detailed -API documentation. +This overview provides a general understanding. Always refer to the +specific header files in `firebase/app/client/cpp/include/firebase/` and +`firebase/product_name/client/cpp/include/firebase/product_name/` for +detailed API documentation. # Best Practices ## Coding Style * **Firebase C++ Style Guide**: For specific C++ API design and coding - conventions relevant to this SDK, refer to the [STYLE_GUIDE.md](STYLE_GUIDE.md). + conventions relevant to this SDK, refer to the + [STYLE_GUIDE.md](STYLE_GUIDE.md). * **Google C++ Style Guide**: Adhere to the [Google C++ Style Guide](https://google.github.io/styleguide/cppguide.html) as mentioned in `CONTRIBUTING.md` for general C++ style. -* **Formatting**: Use `python3 scripts/format_code.py -git_diff -verbose` to - format your code before committing. +* **Formatting**: Use `python3 scripts/format_code.py -git_diff -verbose` + to format your code before committing. ## Comments -* Write clear and concise comments where necessary to explain complex logic or - non-obvious behavior. -* **Avoid overly verbose comments**: Do not state the obvious. The code should - be as self-documenting as possible. +* Write clear and concise comments where necessary to explain complex + logic or non-obvious behavior. +* **Avoid overly verbose comments**: Do not state the obvious. The code + should be as self-documenting as possible. * Follow existing comment styles within the module you are working on. -* "Avoid adding comments next to `#include` directives merely to explain why - the include is necessary; the code usage should make this clear, or it can - be part of a broader comment block if truly non-obvious for a section of - code." -* "Do not include comments that narrate the AI agent's iterative development - process (e.g., 'Removed old logic here', 'Changed variable name from X to - Y because...', 'Attempted Z but it did not work'). Comments should focus on - explaining the current state of the code for future maintainers, not its - development history or the AI's thought process." +* "Avoid adding comments next to `#include` directives merely to explain + why the include is necessary; the code usage should make this clear, + or it can be part of a broader comment block if truly non-obvious for + a section of code." +* "Do not include comments that narrate the AI agent's iterative + development process (e.g., 'Removed old logic here', 'Changed + variable name from X to Y because...', 'Attempted Z but it did not + work'). Comments should focus on explaining the current state of the + code for future maintainers, not its development history or the AI's + thought process." ## Error Handling -* **Check `Future` status and errors**: Always check `future.status()` and - `future.error()` before attempting to use `future.result()`. +* **Check `Future` status and errors**: Always check `future.status()` + and `future.error()` before attempting to use `future.result()`. * A common success code is `0` (e.g., - `firebase::auth::kAuthErrorNone`, `firebase::database::kErrorNone`). - Other specific error codes are defined per module (e.g., + `firebase::auth::kAuthErrorNone`, + `firebase::database::kErrorNone`). Other specific error codes are + defined per module (e.g., `firebase::auth::kAuthErrorUserNotFound`). -* **Callback error parameters**: When using listeners or other callbacks, - always check the provided error code and message before processing the - data. +* **Callback error parameters**: When using listeners or other + callbacks, always check the provided error code and message before + processing the data. * Provide meaningful error messages if your code introduces new error conditions. ## Resource Management * **`firebase::Future`**: `Future` objects manage their own resources. - `Future::Release()` can be called, but it's often handled by RAII when - futures go out of scope. Be mindful of `Future` lifetimes if they are - stored as members or passed around. + `Future::Release()` can be called, but it's often handled by RAII + when futures go out of scope. Be mindful of `Future` lifetimes if + they are stored as members or passed around. * **ListenerRegistrations**: When a listener is no longer needed, call - `ListenerRegistration::Remove()` to detach it. Failure to do so can lead - to memory leaks and unnecessary network/CPU usage. + `ListenerRegistration::Remove()` to detach it. Failure to do so can + lead to memory leaks and unnecessary network/CPU usage. * **Pointers**: Standard C++ smart pointers (`std::unique_ptr`, `std::shared_ptr`) should be used where appropriate for managing dynamically allocated memory. -* **`Future` Lifecycle**: Ensure `Future` objects returned from API calls are - properly managed. While `Future`s handle their own internal memory for the - result, the asynchronous operations they represent need to complete to - reliably free all associated operational resources or to ensure actions - (like writes to a database) are definitely finalized. Abandoning a `Future` - (letting it go out of scope without checking its result, attaching an - `OnCompletion` callback, or explicitly `Wait()`ing for it) can sometimes - lead to operations not completing as expected or resources not being - cleaned up promptly by the underlying services, especially if the `Future` - is the only handle to that operation. Prefer using `OnCompletion` or - otherwise ensuring the `Future` completes its course, particularly for - operations with side effects or those that allocate significant backend - resources. +* **`Future` Lifecycle**: Ensure `Future` objects returned from API + calls are properly managed. While `Future`s handle their own internal + memory for the result, the asynchronous operations they represent + need to complete to reliably free all associated operational + resources or to ensure actions (like writes to a database) are + definitely finalized. Abandoning a `Future` (letting it go out of + scope without checking its result, attaching an `OnCompletion` + callback, or explicitly `Wait()`ing for it) can sometimes lead to + operations not completing as expected or resources not being cleaned + up promptly by the underlying services, especially if the `Future` is + the only handle to that operation. Prefer using `OnCompletion` or + otherwise ensuring the `Future` completes its course, particularly + for operations with side effects or those that allocate significant + backend resources. ## Immutability * Be aware that some objects, like `firebase::firestore::Query`, are - immutable. Methods that appear to modify them (e.g., `query.Where(...)`) - actually return a new instance with the modification. + immutable. Methods that appear to modify them (e.g., + `query.Where(...)`) actually return a new instance with the + modification. ## Platform-Specific Code -* Firebase C++ SDK supports multiple platforms (Android, iOS, tvOS, desktop - - Windows, macOS, Linux). +* Firebase C++ SDK supports multiple platforms (Android, iOS, tvOS, + desktop - Windows, macOS, Linux). * Platform-specific code is typically organized into subdirectories: * `android/` (for Java/JNI related code) * `ios/` (for Objective-C/Swift interoperability code) * `desktop/` (for Windows, macOS, Linux specific implementations) * `common/` (for C++ code shared across all platforms) * Use preprocessor directives (e.g., `#if FIREBASE_PLATFORM_ANDROID`, - `#if FIREBASE_PLATFORM_IOS`) to conditionally compile platform-specific - sections when necessary, but prefer separate implementation files where - possible for better organization. + `#if FIREBASE_PLATFORM_IOS`) to conditionally compile + platform-specific sections when necessary, but prefer separate + implementation files where possible for better organization. ## Platform-Specific Considerations * **Realtime Database (Desktop)**: The C++ SDK for Realtime Database on desktop platforms (Windows, macOS, Linux) uses a REST-based implementation. This means that any queries involving - `Query::OrderByChild()` require corresponding indexes to be defined in your - Firebase project's Realtime Database rules. Without these indexes, queries - may fail or not return expected results. + `Query::OrderByChild()` require corresponding indexes to be defined + in your Firebase project's Realtime Database rules. Without these + indexes, queries may fail or not return expected results. * **iOS Method Swizzling**: Be aware that some Firebase products on iOS (e.g., Dynamic Links, Cloud Messaging) use method swizzling to - automatically attach handlers to your `AppDelegate`. While this simplifies - integration, it can occasionally be a factor to consider when debugging app - delegate behavior or integrating with other libraries that also perform - swizzling. + automatically attach handlers to your `AppDelegate`. While this + simplifies integration, it can occasionally be a factor to consider + when debugging app delegate behavior or integrating with other + libraries that also perform swizzling. ## Class and File Structure * Follow the existing pattern of internal and common classes within each - Firebase library (e.g., `firestore/src/common`, `firestore/src/main`, - `firestore/src/android`). + Firebase library (e.g., `firestore/src/common`, + `firestore/src/main`, `firestore/src/android`). * Public headers defining the API are typically in `src/include/firebase/product_name/`. -* Internal implementation classes often use the `Internal` suffix (e.g., - `DocumentReferenceInternal`). +* Internal implementation classes often use the `Internal` suffix + (e.g., `DocumentReferenceInternal`). # Common Patterns ## Pimpl (Pointer to Implementation) Idiom -* Many public API classes (e.g., `firebase::storage::StorageReference`, +* Many public API classes (e.g., + `firebase::storage::StorageReference`, `firebase::storage::Metadata`) use the Pimpl idiom. * They hold a pointer to an internal implementation class (e.g., `StorageReferenceInternal`, `MetadataInternal`). -* This pattern helps decouple the public interface from its implementation, - reducing compilation dependencies and hiding internal details. -* When working with these classes, you will primarily interact with the public - interface. Modifications to the underlying implementation are done in the - `*Internal` classes. - -A common convention for this Pimpl pattern in the codebase (e.g., as seen in -`firebase::storage::Metadata`) is that the public class owns the raw pointer to -its internal implementation (`internal_`). This requires careful manual memory -management: -* **Creation**: The `internal_` object is typically allocated with `new` in - the public class's constructor(s). For instance, the default constructor - might do `internal_ = new ClassNameInternal(...);`, and a copy constructor - would do `internal_ = new ClassNameInternal(*other.internal_);` for a - deep copy. -* **Deletion**: The `internal_` object is `delete`d in the public class's - destructor (e.g., `delete internal_;`). It's good practice to set - `internal_ = nullptr;` immediately after deletion if the destructor isn't - the absolute last point of usage, or if helper delete functions are used - (as seen with `MetadataInternalCommon::DeleteInternal` which nullifies the - pointer in the public class instance before deleting). +* This pattern helps decouple the public interface from its + implementation, reducing compilation dependencies and hiding internal + details. +* When working with these classes, you will primarily interact with the + public interface. Modifications to the underlying implementation are + done in the `*Internal` classes. + +A common convention for this Pimpl pattern in the codebase (e.g., as seen +in `firebase::storage::Metadata`) is that the public class owns the raw +pointer to its internal implementation (`internal_`). This requires +careful manual memory management: +* **Creation**: The `internal_` object is typically allocated with `new` + in the public class's constructor(s). For instance, the default + constructor might do `internal_ = new ClassNameInternal(...);`, and a + copy constructor would do + `internal_ = new ClassNameInternal(*other.internal_);` for a deep + copy. +* **Deletion**: The `internal_` object is `delete`d in the public + class's destructor (e.g., `delete internal_;`). It's good practice + to set `internal_ = nullptr;` immediately after deletion if the + destructor isn't the absolute last point of usage, or if helper + delete functions are used (as seen with + `MetadataInternalCommon::DeleteInternal` which nullifies the pointer + in the public class instance before deleting). * **Copy Semantics**: - * **Copy Constructor**: If supported, a deep copy is usually performed. A - new `internal_` instance is created, and the contents from the source - object's `internal_` are copied into this new instance. - * **Copy Assignment Operator**: If supported, it must also perform a deep - copy. This typically involves deleting the current `internal_` object, - then creating a new `internal_` instance and copying data from the - source's `internal_` object. Standard self-assignment checks should be - present. + * **Copy Constructor**: If supported, a deep copy is usually + performed. A new `internal_` instance is created, and the + contents from the source object's `internal_` are copied into + this new instance. + * **Copy Assignment Operator**: If supported, it must also perform a + deep copy. This typically involves deleting the current + `internal_` object, then creating a new `internal_` instance and + copying data from the source's `internal_` object. Standard + self-assignment checks should be present. * **Move Semantics**: - * **Move Constructor**: If supported, ownership of the `internal_` pointer - is transferred from the source object to the new object. The source - object's `internal_` pointer is then set to `nullptr` to prevent - double deletion. + * **Move Constructor**: If supported, ownership of the `internal_` + pointer is transferred from the source object to the new object. + The source object's `internal_` pointer is then set to `nullptr` + to prevent double deletion. * **Move Assignment Operator**: Similar to the move constructor, it involves deleting the current object's `internal_`, then taking - ownership of the source's `internal_` pointer, and finally setting the - source's `internal_` to `nullptr`. + ownership of the source's `internal_` pointer, and finally + setting the source's `internal_` to `nullptr`. * **Cleanup Registration (Advanced)**: In some cases, like - `firebase::storage::Metadata`, there might be an additional registration - with a central cleanup mechanism (e.g., via `StorageInternal`). This acts - as a safeguard or part of a larger resource management strategy within the - module, but the fundamental responsibility for creation and deletion in - typical scenarios lies with the Pimpl class itself. + `firebase::storage::Metadata`, there might be an additional + registration with a central cleanup mechanism (e.g., via + `StorageInternal`). This acts as a safeguard or part of a larger + resource management strategy within the module, but the fundamental + responsibility for creation and deletion in typical scenarios lies + with the Pimpl class itself. -It's crucial to correctly implement all these aspects (constructors, destructor, -copy/move operators) when dealing with raw pointer Pimpls to prevent memory -leaks, dangling pointers, or double deletions. +It's crucial to correctly implement all these aspects (constructors, +destructor, copy/move operators) when dealing with raw pointer Pimpls to +prevent memory leaks, dangling pointers, or double deletions. ## Namespace Usage -* Code for each Firebase product is typically within its own namespace under - `firebase`. +* Code for each Firebase product is typically within its own namespace + under `firebase`. * Example: `firebase::firestore`, `firebase::auth`, `firebase::database`. -* Internal or platform-specific implementation details might be in nested - namespaces like `firebase::firestore::internal` or +* Internal or platform-specific implementation details might be in + nested namespaces like `firebase::firestore::internal` or `firebase::firestore::android`. ## Futures for Asynchronous Operations @@ -489,27 +506,28 @@ leaks, dangling pointers, or double deletions. ## Internal Classes and Helpers * Each module often has a set of internal classes (often suffixed with - `Internal` or residing in an `internal` namespace) that manage the core - logic, platform-specific interactions, and communication with the Firebase - backend. -* Utility functions and helper classes are common within the `common/` or - `util/` subdirectories of a product. + `Internal` or residing in an `internal` namespace) that manage the + core logic, platform-specific interactions, and communication with + the Firebase backend. +* Utility functions and helper classes are common within the `common/` + or `util/` subdirectories of a product. ## Singleton Usage (Limited) * The primary singleton is `firebase::App::GetInstance()`. -* Service entry points like `firebase::firestore::Firestore::GetInstance()` - also provide singleton-like access to service instances (scoped to an `App` +* Service entry points like + `firebase::firestore::Firestore::GetInstance()` also provide + singleton-like access to service instances (scoped to an `App` instance). -* Beyond these entry points, direct creation or use of singletons for core - data objects or utility classes is not a dominant pattern. Dependencies are - generally passed explicitly. +* Beyond these entry points, direct creation or use of singletons for + core data objects or utility classes is not a dominant pattern. + Dependencies are generally passed explicitly. ## Platform Abstraction * For operations that differ by platform, there's often a common C++ - interface defined in `common/` or `main/`, with specific implementations - in `android/`, `ios/`, and `desktop/` directories. + interface defined in `common/` or `main/`, with specific + implementations in `android/`, `ios/`, and `desktop/` directories. * JNI is used extensively in `android/` directories for C++ to Java communication. * Objective-C++ (`.mm` files) is used in `ios/` directories for C++ to @@ -523,58 +541,61 @@ leaks, dangling pointers, or double deletions. ## Builder/Fluent Interface for Queries -* Classes like `firebase::firestore::Query` use a fluent interface (method - chaining) to construct complex queries by progressively adding filters, - ordering, and limits. Each call typically returns a new instance of the - query object. +* Classes like `firebase::firestore::Query` use a fluent interface + (method chaining) to construct complex queries by progressively + adding filters, ordering, and limits. Each call typically returns a + new instance of the query object. # Updating This Document -This document is a living guide. As the Firebase C++ SDK evolves, new patterns -may emerge, or existing practices might change. If you introduce a new common -pattern, significantly alter a build process, or establish a new best practice -during your work, please take a moment to update this `Jules.md` file -accordingly. +This document is a living guide. As the Firebase C++ SDK evolves, new +patterns may emerge, or existing practices might change. If you introduce +a new common pattern, significantly alter a build process, or establish a +new best practice during your work, please take a moment to update this +`Jules.md` file accordingly. -Keeping this document current will greatly benefit future AI agents and human -developers working on this repository. +Keeping this document current will greatly benefit future AI agents and +human developers working on this repository. # Prompting Jules AI ## Recommended General Prompt Instruction -When working on this task, please consistently refer to the `Jules.md` guide -for all repository-specific conventions, including setup procedures, coding -style, common architectural patterns, and API usage. Pay close attention to the -testing strategies outlined, ensuring your implementation includes -comprehensive integration tests with detailed test cases in your plan. Implement -robust error handling for any new or modified public API methods, following the -patterns for `firebase::Future` error reporting. If the feature involves -platform-specific code, ensure the public API remains consistent across all -platforms, as discussed in `Jules.md`. Write clear, maintainable code, -adhering to the commenting guidelines, and if you need to add new third-party -dependencies, document the rationale and update build configurations according -to the established practices. Ensure your changes align with the overall best -practices detailed in `Jules.md`. +When working on this task, please consistently refer to the `Jules.md` +guide for all repository-specific conventions, including setup +procedures, coding style, common architectural patterns, and API usage. +Pay close attention to the testing strategies outlined, ensuring your +implementation includes comprehensive integration tests with detailed test +cases in your plan. Implement robust error handling for any new or +modified public API methods, following the patterns for `firebase::Future` +error reporting. If the feature involves platform-specific code, ensure +the public API remains consistent across all platforms, as discussed in +`Jules.md`. Write clear, maintainable code, adhering to the commenting +guidelines, and if you need to add new third-party dependencies, document +the rationale and update build configurations according to the established +practices. Ensure your changes align with the overall best practices +detailed in `Jules.md`. ## Key Directives for Jules AI * **Prioritize `Jules.md`**: This document (`Jules.md`) contains - repository-specific guidelines. Prioritize this information when making - decisions about coding style, testing procedures, architectural patterns, - and API usage. -* **Adherence to Patterns**: Strive to follow the common patterns and best - practices outlined here. This ensures consistency across the codebase. -* **Clarification for Deviations**: If the existing patterns in `Jules.md` do - not seem suitable for a specific task, or if a deviation is necessary, - please explicitly state this in your plan or request clarification before - proceeding with implementation. + repository-specific guidelines. Prioritize this information when + making decisions about coding style, testing procedures, + architectural patterns, and API usage. +* **Adherence to Patterns**: Strive to follow the common patterns and + best practices outlined here. This ensures consistency across the + codebase. +* **Clarification for Deviations**: If the existing patterns in + `Jules.md` do not seem suitable for a specific task, or if a + deviation is necessary, please explicitly state this in your plan or + request clarification before proceeding with implementation. * **Updating `Jules.md`**: If you introduce a new, broadly applicable - pattern, or if a significant change to the build process or best practices - occurs as part of your task, please include a step in your plan to update - `Jules.md` to reflect these changes. + pattern, or if a significant change to the build process or best + practices occurs as part of your task, please include a step in your + plan to update `Jules.md` to reflect these changes. * **Testing**: Remember that integration tests are the primary method of - testing. Ensure new features are accompanied by corresponding integration - tests as described in the 'Testing' section of `Jules.md`. + testing. Ensure new features are accompanied by corresponding + integration tests as described in the 'Testing' section of + `Jules.md`. * **Commit Messages**: Follow standard commit message guidelines. A brief summary line, followed by a more detailed explanation if necessary. From 6ff900c6a079cf484d690e6297c4ffc950a059da Mon Sep 17 00:00:00 2001 From: Jon Simantov Date: Wed, 25 Jun 2025 16:44:00 -0700 Subject: [PATCH 6/9] Revert formatting changes. --- AGENTS.md | 456 ++++++++++++++++++++++++------------------------------ 1 file changed, 198 insertions(+), 258 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index c73a121ef4..95fdb653eb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,9 +1,5 @@ # Introduction -> **Note on Document Formatting:** This document (`AGENTS.md`) should be -> maintained with lines word-wrapped to a maximum of 80 characters to ensure -> readability across various editors and terminals. - This document provides context and guidance for AI agents (like Jules) when making changes to the Firebase C++ SDK repository. It covers essential information about the repository's structure, setup, testing procedures, API @@ -16,8 +12,8 @@ platform (Android, iOS, tvOS, macOS, Windows, Linux), refer to the official The Firebase C++ SDKs for desktop platforms (Windows, Linux, macOS) are entirely open source and hosted in the main `firebase/firebase-cpp-sdk` GitHub repository. The C++ SDKs for mobile platforms (iOS, tvOS, Android) are built -on top of the respective native open-source Firebase SDKs (Firebase iOS SDK -and Firebase Android SDK). +on top of the respective native open-source Firebase SDKs (Firebase iOS SDK and +Firebase Android SDK). The goal is to enable agents to understand the existing conventions and contribute effectively to the codebase. @@ -38,8 +34,8 @@ instructions for your specific platform. * **Android SDK & NDK**: Required for building Android libraries. `sdkmanager` can be used for installation. CMake for Android (version 3.10.2 recommended) is also needed. -* **(Windows Only) Strings**: From Microsoft Sysinternals, required for - Android builds on Windows. +* **(Windows Only) Strings**: From Microsoft Sysinternals, required for Android + builds on Windows. * **Cocoapods**: Required for building iOS or tvOS libraries. ## Building the SDK @@ -48,8 +44,7 @@ The SDK uses CMake for C++ compilation and Gradle for Android-specific parts. ### CMake (Desktop, iOS, tvOS) -1. Create a build directory (e.g., - `mkdir desktop_build && cd desktop_build`). +1. Create a build directory (e.g., `mkdir desktop_build && cd desktop_build`). 2. Run CMake to configure: `cmake ..` * For iOS: `cmake -DCMAKE_TOOLCHAIN_FILE=../cmake/toolchains/ios.cmake ..` @@ -66,23 +61,22 @@ third-party dependency locations. ### Gradle (Android) -Each Firebase C++ library is a Gradle subproject. To build a specific -library (e.g., Analytics): +Each Firebase C++ library is a Gradle subproject. To build a specific library +(e.g., Analytics): ```bash ./gradlew :analytics:assembleRelease ``` -This command should be run from the root of the repository. Proguard files -are generated in each library's build directory (e.g., +This command should be run from the root of the repository. Proguard files are +generated in each library's build directory (e.g., `analytics/build/analytics.pro`). ### Desktop Platform Setup Details -When setting up for desktop, if you are using an iOS -`GoogleService-Info.plist` file, convert it to the required -`google-services-desktop.json` using the script: -`python generate_xml_from_google_services_json.py --plist -i GoogleService-Info.plist` +When setting up for desktop, if you are using an iOS `GoogleService-Info.plist` +file, convert it to the required `google-services-desktop.json` using the +script: `python generate_xml_from_google_services_json.py --plist -i GoogleService-Info.plist` (run this from the script's directory, ensuring the plist file is accessible). The desktop SDK searches for configuration files in the current working @@ -111,8 +105,8 @@ target_link_libraries([[Your CMake Target]] firebase_analytics firebase_app) ### Android Gradle Projects -In addition to CMake setup, use `Android/firebase_dependencies.gradle` in -your `build.gradle`: +In addition to CMake setup, use `Android/firebase_dependencies.gradle` in your +`build.gradle`: ```gradle apply from: "[[Path to the Firebase C++ SDK]]/Android/firebase_dependencies.gradle" @@ -130,18 +124,18 @@ For more detailed instructions and examples, always refer to the main ## Testing Strategy The primary method for testing in this repository is through **integration -tests** for each Firebase library. While the `README.md` mentions unit -tests run via CTest, the current and preferred approach is to ensure -comprehensive coverage within the integration tests. +tests** for each Firebase library. While the `README.md` mentions unit tests +run via CTest, the current and preferred approach is to ensure comprehensive +coverage within the integration tests. ## Running Tests -* **Integration Test Location**: Integration tests for each Firebase - product (e.g., Firestore, Auth) are typically located in the - `integration_test/` directory within that product's module (e.g., +* **Integration Test Location**: Integration tests for each Firebase product + (e.g., Firestore, Auth) are typically located in the `integration_test/` + directory within that product's module (e.g., `firestore/integration_test/`). -* **Test Scripts**: The root of the repository contains scripts for - running tests on various platforms, such as: +* **Test Scripts**: The root of the repository contains scripts for running + tests on various platforms, such as: * `test_windows_x32.bat` * `test_windows_x64.bat` * `test_linux.sh` @@ -149,9 +143,9 @@ comprehensive coverage within the integration tests. * `test_mac_ios.sh` * `test_mac_ios_simulator.sh` - These scripts typically build the SDKs and then execute the relevant - tests (primarily integration tests) via CTest or other platform-specific - test runners. + These scripts typically build the SDKs and then execute the relevant tests + (primarily integration tests) via CTest or other platform-specific test + runners. ## Writing Tests @@ -159,8 +153,8 @@ When adding new features or fixing bugs: * Prioritize adding or updating integration tests within the respective product's `integration_test/` directory. -* Ensure tests cover the new functionality thoroughly and verify - interactions with the Firebase backend or other relevant components. +* Ensure tests cover the new functionality thoroughly and verify interactions + with the Firebase backend or other relevant components. * Follow existing patterns within the integration tests for consistency. # API Surface @@ -175,58 +169,57 @@ Database). 1. **`firebase::App`**: This is the central entry point for the SDK. * It must be initialized first using `firebase::App::Create(...)`. - * On Android, this requires passing the JNI environment (`JNIEnv*`) - and the Android Activity (`jobject`). - * `firebase::AppOptions` can be used to configure the app with - specific parameters if not relying on a `google-services.json` or + * On Android, this requires passing the JNI environment (`JNIEnv*`) and + the Android Activity (`jobject`). + * `firebase::AppOptions` can be used to configure the app with specific + parameters if not relying on a `google-services.json` or `GoogleService-Info.plist` file. 2. **Service Instances**: Once `firebase::App` is initialized, you generally - obtain instances of specific Firebase services using a static - `GetInstance()` method on the service's class, passing the `firebase::App` - object. + obtain instances of specific Firebase services using a static `GetInstance()` + method on the service's class, passing the `firebase::App` object. * Examples for services like Auth, Database, Storage, Firestore: * `firebase::auth::Auth* auth = firebase::auth::Auth::GetAuth(app, &init_result);` * `firebase::database::Database* database = firebase::database::Database::GetInstance(app, &init_result);` * `firebase::storage::Storage* storage = firebase::storage::Storage::GetInstance(app, &init_result);` * Always check the `init_result` (an `InitResult` enum, often - `firebase::kInitResultSuccess` on success) to ensure these - services were initialized successfully. - * **Note on Analytics**: Some products, like Firebase Analytics, have - a different pattern. Analytics is typically initialized with + `firebase::kInitResultSuccess` on success) to ensure these services + were initialized successfully. + * **Note on Analytics**: Some products, like Firebase Analytics, have a + different pattern. Analytics is typically initialized with `firebase::analytics::Initialize(const firebase::App& app)` (often handled automatically for the default `App` instance). After this, Analytics functions (e.g., `firebase::analytics::LogEvent(...)`) are called as global functions within the `firebase::analytics` namespace, rather than on an instance object obtained via `GetInstance()`. Refer to the specific product's header file for its exact - initialization mechanism if it deviates from the common - `GetInstance(app, ...)` pattern. + initialization mechanism if it deviates from the common `GetInstance(app, ...)` + pattern. ### Asynchronous Operations: `firebase::Future` -All asynchronous operations in the SDK return a `firebase::Future` -object, where `T` is the type of the expected result. +All asynchronous operations in the SDK return a `firebase::Future` object, +where `T` is the type of the expected result. -* **Status Checking**: Use `future.status()` to check if the operation - is `kFutureStatusPending`, `kFutureStatusComplete`, or +* **Status Checking**: Use `future.status()` to check if the operation is + `kFutureStatusPending`, `kFutureStatusComplete`, or `kFutureStatusInvalid`. * **Getting Results**: Once `future.status() == kFutureStatusComplete`: * Check for errors: `future.error()`. A value of `0` (e.g., - `firebase::auth::kAuthErrorNone`, - `firebase::database::kErrorNone`) usually indicates success. + `firebase::auth::kAuthErrorNone`, `firebase::database::kErrorNone`) + usually indicates success. * Get the error message: `future.error_message()`. - * Get the result: `future.result()`. This returns a pointer to the - result object of type `T`. The result is only valid if - `future.error()` indicates success. + * Get the result: `future.result()`. This returns a pointer to the result + object of type `T`. The result is only valid if `future.error()` + indicates success. * **Completion Callbacks**: Use `future.OnCompletion(...)` to register a - callback function (lambda or function pointer) that will be invoked - when the future completes. The callback receives the completed future - as an argument. + callback function (lambda or function pointer) that will be invoked when + the future completes. The callback receives the completed future as an + argument. ### Core Classes and Operations (Examples from Auth and Database) -While each Firebase product has its own specific classes, the following -examples illustrate common API patterns: +While each Firebase product has its own specific classes, the following examples +illustrate common API patterns: * **`firebase::auth::Auth`**: The main entry point for Firebase Authentication. @@ -239,21 +232,20 @@ examples illustrate common API patterns: successful and the pointer is checked). * Example: `firebase::auth::User* current_user = auth->current_user();` * Methods for user creation/authentication: - `CreateUserWithEmailAndPassword()`, - `SignInWithEmailAndPassword()`, `SignInWithCredential()`. + `CreateUserWithEmailAndPassword()`, `SignInWithEmailAndPassword()`, + `SignInWithCredential()`. * **`firebase::auth::User`**: - * Represents a user account. Data is typically accessed via its - methods. + * Represents a user account. Data is typically accessed via its methods. * Methods for profile updates: `UpdateEmail()`, `UpdatePassword()`, `UpdateUserProfile()`. * Other operations: `SendEmailVerification()`, `Delete()`. * **`firebase::database::Database`**: The main entry point for Firebase Realtime Database. - * Used to get `DatabaseReference` objects to specific locations in - the database. + * Used to get `DatabaseReference` objects to specific locations in the + database. * **`firebase::database::DatabaseReference`**: - * Represents a reference to a specific location (path) in the - Realtime Database. + * Represents a reference to a specific location (path) in the Realtime + Database. * Methods for navigation: `Child()`, `Parent()`, `Root()`. * Methods for data manipulation: `GetValue()`, `SetValue()`, `UpdateChildren()`, `RemoveValue()`. @@ -261,19 +253,19 @@ examples illustrate common API patterns: * **`firebase::database::Query`**: * Used to retrieve data from a Realtime Database location based on specific criteria. Obtained from a `DatabaseReference`. - * **Filtering**: `OrderByChild()`, `OrderByKey()`, - `OrderByValue()`, `EqualTo()`, `StartAt()`, `EndAt()`. + * **Filtering**: `OrderByChild()`, `OrderByKey()`, `OrderByValue()`, + `EqualTo()`, `StartAt()`, `EndAt()`. * **Limiting**: `LimitToFirst()`, `LimitToLast()`. * Execution: `GetValue()` returns a `Future`. -* **`firebase::database::DataSnapshot`**: Contains data read from a - Realtime Database location (either directly or as a result of a - query). Accessed via `future.result()` or through listeners. - * Methods: `value()` (returns a `firebase::Variant` representing - the data), `children_count()`, `children()`, `key()`, `exists()`. +* **`firebase::database::DataSnapshot`**: Contains data read from a Realtime + Database location (either directly or as a result of a query). Accessed + via `future.result()` or through listeners. + * Methods: `value()` (returns a `firebase::Variant` representing the + data), `children_count()`, `children()`, `key()`, `exists()`. * **`firebase::Variant`**: A type that can hold various data types like integers, strings, booleans, vectors (arrays), and maps (objects), - commonly used for reading and writing data with Realtime Database and - other services. + commonly used for reading and writing data with Realtime Database and other + services. * **Operation-Specific Options**: Some operations might take optional parameters to control behavior, though not always through a dedicated "Options" class like Firestore's `SetOptions`. For example, @@ -281,29 +273,28 @@ examples illustrate common API patterns: ### Listeners for Real-time Updates -Many Firebase services support real-time data synchronization using -listeners. +Many Firebase services support real-time data synchronization using listeners. * **`firebase::database::ValueListener` / - `firebase::database::ChildListener`**: Implemented by the developer - and registered with a `DatabaseReference`. + `firebase::database::ChildListener`**: Implemented by the developer and + registered with a `DatabaseReference`. * `ValueListener::OnValueChanged(const firebase::database::DataSnapshot& snapshot)` is called when the data at that location changes. - * `ChildListener` has methods like `OnChildAdded()`, - `OnChildChanged()`, `OnChildRemoved()`. -* **`firebase::auth::AuthStateListener`**: Implemented by the developer - and registered with `firebase::auth::Auth`. - * `AuthStateListener::OnAuthStateChanged(firebase::auth::Auth* auth)` - is called when the user's sign-in state changes. + * `ChildListener` has methods like `OnChildAdded()`, `OnChildChanged()`, + `OnChildRemoved()`. +* **`firebase::auth::AuthStateListener`**: Implemented by the developer and + registered with `firebase::auth::Auth`. + * `AuthStateListener::OnAuthStateChanged(firebase::auth::Auth* auth)` is + called when the user's sign-in state changes. * **Removing Listeners**: Listeners are typically removed by passing the - listener instance to a corresponding `Remove...Listener()` method - (e.g., `reference->RemoveValueListener(my_listener);`, + listener instance to a corresponding `Remove...Listener()` method (e.g., + `reference->RemoveValueListener(my_listener);`, `auth->RemoveAuthStateListener(my_auth_listener);`). -This overview provides a general understanding. Always refer to the -specific header files in `firebase/app/client/cpp/include/firebase/` and -`firebase/product_name/client/cpp/include/firebase/product_name/` for -detailed API documentation. +This overview provides a general understanding. Always refer to the specific +header files in `firebase/app/client/cpp/include/firebase/` and +`firebase/product_name/client/cpp/include/firebase/product_name/` for detailed +API documentation. # Best Practices @@ -317,55 +308,47 @@ detailed API documentation. as mentioned in `CONTRIBUTING.md`. * **Formatting**: Use `python3 scripts/format_code.py -git_diff -verbose` to format your code before committing. -* **Naming Precision for Dynamic Systems**: Function names should precisely - reflect their behavior, especially in systems with dynamic or asynchronous - interactions. For example, a function that processes a list of items should - be named differently from one that operates on a single, specific item - captured asynchronously. Regularly re-evaluate function names as - requirements evolve to maintain clarity. ## Comments -* Write clear and concise comments where necessary to explain complex - logic or non-obvious behavior. -* **Avoid overly verbose comments**: Do not state the obvious. The code - should be as self-documenting as possible. +* Write clear and concise comments where necessary to explain complex logic or + non-obvious behavior. +* **Avoid overly verbose comments**: Do not state the obvious. The code should + be as self-documenting as possible. * Follow existing comment styles within the module you are working on. -* "Avoid adding comments next to `#include` directives merely to explain - why the include is necessary; the code usage should make this clear, - or it can be part of a broader comment block if truly non-obvious for - a section of code." -* "Do not include comments that narrate the AI agent's iterative - development process (e.g., 'Removed old logic here', 'Changed - variable name from X to Y because...', 'Attempted Z but it did not - work'). Comments should focus on explaining the current state of the - code for future maintainers, not its development history or the AI's - thought process." +* "Avoid adding comments next to `#include` directives merely to explain why + the include is necessary; the code usage should make this clear, or it can + be part of a broader comment block if truly non-obvious for a section of + code." +* "Do not include comments that narrate the AI agent's iterative development + process (e.g., 'Removed old logic here', 'Changed variable name from X to + Y because...', 'Attempted Z but it did not work'). Comments should focus on + explaining the current state of the code for future maintainers, not its + development history or the AI's thought process." ## Error Handling -* **Check `Future` status and errors**: Always check `future.status()` - and `future.error()` before attempting to use `future.result()`. +* **Check `Future` status and errors**: Always check `future.status()` and + `future.error()` before attempting to use `future.result()`. * A common success code is `0` (e.g., - `firebase::auth::kAuthErrorNone`, - `firebase::database::kErrorNone`). Other specific error codes are - defined per module (e.g., + `firebase::auth::kAuthErrorNone`, `firebase::database::kErrorNone`). + Other specific error codes are defined per module (e.g., `firebase::auth::kAuthErrorUserNotFound`). -* **Callback error parameters**: When using listeners or other - callbacks, always check the provided error code and message before - processing the data. +* **Callback error parameters**: When using listeners or other callbacks, + always check the provided error code and message before processing the + data. * Provide meaningful error messages if your code introduces new error conditions. ## Resource Management * **`firebase::Future`**: `Future` objects manage their own resources. - `Future::Release()` can be called, but it's often handled by RAII - when futures go out of scope. Be mindful of `Future` lifetimes if - they are stored as members or passed around. + `Future::Release()` can be called, but it's often handled by RAII when + futures go out of scope. Be mindful of `Future` lifetimes if they are + stored as members or passed around. * **ListenerRegistrations**: When a listener is no longer needed, call - `ListenerRegistration::Remove()` to detach it. Failure to do so can - lead to memory leaks and unnecessary network/CPU usage. + `ListenerRegistration::Remove()` to detach it. Failure to do so can lead + to memory leaks and unnecessary network/CPU usage. * **Pointers**: Standard C++ smart pointers (`std::unique_ptr`, `std::shared_ptr`) should be used where appropriate for managing dynamically allocated memory. @@ -382,155 +365,118 @@ detailed API documentation. otherwise ensuring the `Future` completes its course, particularly for operations with side effects or those that allocate significant backend resources. -* **Lifecycle of Queued Callbacks/Blocks**: If blocks or callbacks are queued - to be run upon an asynchronous event (e.g., an App Delegate class being set - or a Future completing), clearly define and document their lifecycle. - Determine if they are one-shot (cleared after first execution) or - persistent (intended to run for multiple or future events). This impacts - how associated data and the blocks themselves are stored and cleared, - preventing memory leaks or unexpected multiple executions. ## Immutability * Be aware that some objects, like `firebase::firestore::Query`, are - immutable. Methods that appear to modify them (e.g., - `query.Where(...)`) actually return a new instance with the - modification. + immutable. Methods that appear to modify them (e.g., `query.Where(...)`) + actually return a new instance with the modification. ## Platform-Specific Code -* Firebase C++ SDK supports multiple platforms (Android, iOS, tvOS, - desktop - Windows, macOS, Linux). +* Firebase C++ SDK supports multiple platforms (Android, iOS, tvOS, desktop - + Windows, macOS, Linux). * Platform-specific code is typically organized into subdirectories: * `android/` (for Java/JNI related code) * `ios/` (for Objective-C/Swift interoperability code) * `desktop/` (for Windows, macOS, Linux specific implementations) * `common/` (for C++ code shared across all platforms) * Use preprocessor directives (e.g., `#if FIREBASE_PLATFORM_ANDROID`, - `#if FIREBASE_PLATFORM_IOS`) to conditionally compile - platform-specific sections when necessary, but prefer separate - implementation files where possible for better organization. + `#if FIREBASE_PLATFORM_IOS`) to conditionally compile platform-specific + sections when necessary, but prefer separate implementation files where + possible for better organization. ## Platform-Specific Considerations * **Realtime Database (Desktop)**: The C++ SDK for Realtime Database on desktop platforms (Windows, macOS, Linux) uses a REST-based implementation. This means that any queries involving - `Query::OrderByChild()` require corresponding indexes to be defined - in your Firebase project's Realtime Database rules. Without these - indexes, queries may fail or not return expected results. + `Query::OrderByChild()` require corresponding indexes to be defined in your + Firebase project's Realtime Database rules. Without these indexes, queries + may fail or not return expected results. * **iOS Method Swizzling**: Be aware that some Firebase products on iOS (e.g., Dynamic Links, Cloud Messaging) use method swizzling to automatically attach handlers to your `AppDelegate`. While this simplifies integration, it can occasionally be a factor to consider when debugging app delegate behavior or integrating with other libraries that also perform swizzling. - When implementing or interacting with swizzling, especially for App Delegate - methods like `[UIApplication setDelegate:]`: - * Be highly aware that `setDelegate:` can be called multiple times - with different delegate class instances, including proxy classes - from other libraries (e.g., GUL - Google Utilities). Swizzling - logic must be robust against being invoked multiple times for the - same effective method on the same class or on classes in a - hierarchy. An idempotency check (i.e., if the method's current IMP - is already the target swizzled IMP, do nothing more for that - specific swizzle attempt) in any swizzling utility can prevent - issues like recursion. - * When tracking unique App Delegate classes (e.g., for applying hooks - or callbacks via swizzling), consider the class hierarchy. If a - superclass has already been processed, processing a subclass for - the same inherited methods might be redundant or problematic. A - strategy to check if a newly set delegate is a subclass of an - already processed delegate can prevent such issues. - * For code that runs very early in the application lifecycle on - iOS/macOS (e.g., `+load` methods, static initializers involved in - swizzling), prefer using `NSLog` directly over custom logging - frameworks if there's any uncertainty about whether the custom - framework is fully initialized, to avoid crashes during logging - itself. ## Class and File Structure * Follow the existing pattern of internal and common classes within each - Firebase library (e.g., `firestore/src/common`, - `firestore/src/main`, `firestore/src/android`). + Firebase library (e.g., `firestore/src/common`, `firestore/src/main`, + `firestore/src/android`). * Public headers defining the API are typically in `src/include/firebase/product_name/`. -* Internal implementation classes often use the `Internal` suffix - (e.g., `DocumentReferenceInternal`). +* Internal implementation classes often use the `Internal` suffix (e.g., + `DocumentReferenceInternal`). # Common Patterns ## Pimpl (Pointer to Implementation) Idiom -* Many public API classes (e.g., - `firebase::storage::StorageReference`, +* Many public API classes (e.g., `firebase::storage::StorageReference`, `firebase::storage::Metadata`) use the Pimpl idiom. * They hold a pointer to an internal implementation class (e.g., `StorageReferenceInternal`, `MetadataInternal`). -* This pattern helps decouple the public interface from its - implementation, reducing compilation dependencies and hiding internal - details. -* When working with these classes, you will primarily interact with the - public interface. Modifications to the underlying implementation are - done in the `*Internal` classes. - -A common convention for this Pimpl pattern in the codebase (e.g., as seen -in `firebase::storage::Metadata`) is that the public class owns the raw -pointer to its internal implementation (`internal_`). This requires -careful manual memory management: -* **Creation**: The `internal_` object is typically allocated with `new` - in the public class's constructor(s). For instance, the default - constructor might do `internal_ = new ClassNameInternal(...);`, and a - copy constructor would do - `internal_ = new ClassNameInternal(*other.internal_);` for a deep - copy. -* **Deletion**: The `internal_` object is `delete`d in the public - class's destructor (e.g., `delete internal_;`). It's good practice - to set `internal_ = nullptr;` immediately after deletion if the - destructor isn't the absolute last point of usage, or if helper - delete functions are used (as seen with - `MetadataInternalCommon::DeleteInternal` which nullifies the pointer - in the public class instance before deleting). +* This pattern helps decouple the public interface from its implementation, + reducing compilation dependencies and hiding internal details. +* When working with these classes, you will primarily interact with the public + interface. Modifications to the underlying implementation are done in the + `*Internal` classes. + +A common convention for this Pimpl pattern in the codebase (e.g., as seen in +`firebase::storage::Metadata`) is that the public class owns the raw pointer to +its internal implementation (`internal_`). This requires careful manual memory +management: +* **Creation**: The `internal_` object is typically allocated with `new` in + the public class's constructor(s). For instance, the default constructor + might do `internal_ = new ClassNameInternal(...);`, and a copy constructor + would do `internal_ = new ClassNameInternal(*other.internal_);` for a + deep copy. +* **Deletion**: The `internal_` object is `delete`d in the public class's + destructor (e.g., `delete internal_;`). It's good practice to set + `internal_ = nullptr;` immediately after deletion if the destructor isn't + the absolute last point of usage, or if helper delete functions are used + (as seen with `MetadataInternalCommon::DeleteInternal` which nullifies the + pointer in the public class instance before deleting). * **Copy Semantics**: - * **Copy Constructor**: If supported, a deep copy is usually - performed. A new `internal_` instance is created, and the - contents from the source object's `internal_` are copied into - this new instance. - * **Copy Assignment Operator**: If supported, it must also perform a - deep copy. This typically involves deleting the current - `internal_` object, then creating a new `internal_` instance and - copying data from the source's `internal_` object. Standard - self-assignment checks should be present. + * **Copy Constructor**: If supported, a deep copy is usually performed. A + new `internal_` instance is created, and the contents from the source + object's `internal_` are copied into this new instance. + * **Copy Assignment Operator**: If supported, it must also perform a deep + copy. This typically involves deleting the current `internal_` object, + then creating a new `internal_` instance and copying data from the + source's `internal_` object. Standard self-assignment checks should be + present. * **Move Semantics**: - * **Move Constructor**: If supported, ownership of the `internal_` - pointer is transferred from the source object to the new object. - The source object's `internal_` pointer is then set to `nullptr` - to prevent double deletion. + * **Move Constructor**: If supported, ownership of the `internal_` pointer + is transferred from the source object to the new object. The source + object's `internal_` pointer is then set to `nullptr` to prevent + double deletion. * **Move Assignment Operator**: Similar to the move constructor, it involves deleting the current object's `internal_`, then taking - ownership of the source's `internal_` pointer, and finally - setting the source's `internal_` to `nullptr`. + ownership of the source's `internal_` pointer, and finally setting the + source's `internal_` to `nullptr`. * **Cleanup Registration (Advanced)**: In some cases, like - `firebase::storage::Metadata`, there might be an additional - registration with a central cleanup mechanism (e.g., via - `StorageInternal`). This acts as a safeguard or part of a larger - resource management strategy within the module, but the fundamental - responsibility for creation and deletion in typical scenarios lies - with the Pimpl class itself. + `firebase::storage::Metadata`, there might be an additional registration + with a central cleanup mechanism (e.g., via `StorageInternal`). This acts + as a safeguard or part of a larger resource management strategy within the + module, but the fundamental responsibility for creation and deletion in + typical scenarios lies with the Pimpl class itself. -It's crucial to correctly implement all these aspects (constructors, -destructor, copy/move operators) when dealing with raw pointer Pimpls to -prevent memory leaks, dangling pointers, or double deletions. +It's crucial to correctly implement all these aspects (constructors, destructor, +copy/move operators) when dealing with raw pointer Pimpls to prevent memory +leaks, dangling pointers, or double deletions. ## Namespace Usage -* Code for each Firebase product is typically within its own namespace - under `firebase`. +* Code for each Firebase product is typically within its own namespace under + `firebase`. * Example: `firebase::firestore`, `firebase::auth`, `firebase::database`. -* Internal or platform-specific implementation details might be in - nested namespaces like `firebase::firestore::internal` or +* Internal or platform-specific implementation details might be in nested + namespaces like `firebase::firestore::internal` or `firebase::firestore::android`. ## Futures for Asynchronous Operations @@ -544,28 +490,27 @@ prevent memory leaks, dangling pointers, or double deletions. ## Internal Classes and Helpers * Each module often has a set of internal classes (often suffixed with - `Internal` or residing in an `internal` namespace) that manage the - core logic, platform-specific interactions, and communication with - the Firebase backend. -* Utility functions and helper classes are common within the `common/` - or `util/` subdirectories of a product. + `Internal` or residing in an `internal` namespace) that manage the core + logic, platform-specific interactions, and communication with the Firebase + backend. +* Utility functions and helper classes are common within the `common/` or + `util/` subdirectories of a product. ## Singleton Usage (Limited) * The primary singleton is `firebase::App::GetInstance()`. -* Service entry points like - `firebase::firestore::Firestore::GetInstance()` also provide - singleton-like access to service instances (scoped to an `App` +* Service entry points like `firebase::firestore::Firestore::GetInstance()` + also provide singleton-like access to service instances (scoped to an `App` instance). -* Beyond these entry points, direct creation or use of singletons for - core data objects or utility classes is not a dominant pattern. - Dependencies are generally passed explicitly. +* Beyond these entry points, direct creation or use of singletons for core + data objects or utility classes is not a dominant pattern. Dependencies are + generally passed explicitly. ## Platform Abstraction * For operations that differ by platform, there's often a common C++ - interface defined in `common/` or `main/`, with specific - implementations in `android/`, `ios/`, and `desktop/` directories. + interface defined in `common/` or `main/`, with specific implementations + in `android/`, `ios/`, and `desktop/` directories. * JNI is used extensively in `android/` directories for C++ to Java communication. * Objective-C++ (`.mm` files) is used in `ios/` directories for C++ to @@ -579,27 +524,27 @@ prevent memory leaks, dangling pointers, or double deletions. ## Builder/Fluent Interface for Queries -* Classes like `firebase::firestore::Query` use a fluent interface - (method chaining) to construct complex queries by progressively - adding filters, ordering, and limits. Each call typically returns a - new instance of the query object. +* Classes like `firebase::firestore::Query` use a fluent interface (method + chaining) to construct complex queries by progressively adding filters, + ordering, and limits. Each call typically returns a new instance of the + query object. # Updating This Document This document is a living guide. As the Firebase C++ SDK evolves, new patterns may emerge, or existing practices might change. If you introduce a new common pattern, significantly alter a build process, or establish a new best practice -during your work, please take a moment to update this `AGENTS.md` file +during your work, please take a moment to update this `Jules.md` file accordingly. -Keeping this document current will greatly benefit future AI agents and -human developers working on this repository. +Keeping this document current will greatly benefit future AI agents and human +developers working on this repository. # Prompting Jules AI ## Recommended General Prompt Instruction -When working on this task, please consistently refer to the `AGENTS.md` guide +When working on this task, please consistently refer to the `Jules.md` guide for all repository-specific conventions, including setup procedures, coding style, common architectural patterns, and API usage. Pay close attention to the testing strategies outlined, ensuring your implementation includes @@ -607,35 +552,30 @@ comprehensive integration tests with detailed test cases in your plan. Implement robust error handling for any new or modified public API methods, following the patterns for `firebase::Future` error reporting. If the feature involves platform-specific code, ensure the public API remains consistent across all -platforms, as discussed in `AGENTS.md`. Write clear, maintainable code, +platforms, as discussed in `Jules.md`. Write clear, maintainable code, adhering to the commenting guidelines, and if you need to add new third-party dependencies, document the rationale and update build configurations according to the established practices. Ensure your changes align with the overall best -practices detailed in `AGENTS.md`. +practices detailed in `Jules.md`. ## Key Directives for Jules AI -* **Prioritize `AGENTS.md`**: This document (`AGENTS.md`) contains +* **Prioritize `Jules.md`**: This document (`Jules.md`) contains repository-specific guidelines. Prioritize this information when making decisions about coding style, testing procedures, architectural patterns, and API usage. * **Adherence to Patterns**: Strive to follow the common patterns and best practices outlined here. This ensures consistency across the codebase. -* **Clarification for Deviations**: If the existing patterns in `AGENTS.md` do +* **Clarification for Deviations**: If the existing patterns in `Jules.md` do not seem suitable for a specific task, or if a deviation is necessary, please explicitly state this in your plan or request clarification before proceeding with implementation. -* **Updating `AGENTS.md`**: If you introduce a new, broadly applicable +* **Updating `Jules.md`**: If you introduce a new, broadly applicable pattern, or if a significant change to the build process or best practices occurs as part of your task, please include a step in your plan to update - `AGENTS.md` to reflect these changes. + `Jules.md` to reflect these changes. * **Testing**: Remember that integration tests are the primary method of testing. Ensure new features are accompanied by corresponding integration - tests as described in the 'Testing' section of `AGENTS.md`. + tests as described in the 'Testing' section of `Jules.md`. * **Commit Messages**: Follow standard commit message guidelines. A brief summary line, followed by a more detailed explanation if necessary. -* **Tool Limitations & Path Specificity**: If codebase search tools (like - `grep` or recursive `ls`) are limited or unavailable, and initial attempts - to locate files/modules based on common directory structures are - unsuccessful, explicitly ask for more specific paths rather than assuming a - module doesn't exist or contains no relevant code. From 3aba09fc85aa0de5389bcb766c2cf0dd9e53c9d0 Mon Sep 17 00:00:00 2001 From: Jon Simantov Date: Wed, 25 Jun 2025 16:44:39 -0700 Subject: [PATCH 7/9] Fix filename. --- AGENTS.md | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 95fdb653eb..db393be673 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -534,7 +534,7 @@ leaks, dangling pointers, or double deletions. This document is a living guide. As the Firebase C++ SDK evolves, new patterns may emerge, or existing practices might change. If you introduce a new common pattern, significantly alter a build process, or establish a new best practice -during your work, please take a moment to update this `Jules.md` file +during your work, please take a moment to update this `AGENTS.md` file accordingly. Keeping this document current will greatly benefit future AI agents and human @@ -544,7 +544,7 @@ developers working on this repository. ## Recommended General Prompt Instruction -When working on this task, please consistently refer to the `Jules.md` guide +When working on this task, please consistently refer to the `AGENTS.md` guide for all repository-specific conventions, including setup procedures, coding style, common architectural patterns, and API usage. Pay close attention to the testing strategies outlined, ensuring your implementation includes @@ -552,30 +552,30 @@ comprehensive integration tests with detailed test cases in your plan. Implement robust error handling for any new or modified public API methods, following the patterns for `firebase::Future` error reporting. If the feature involves platform-specific code, ensure the public API remains consistent across all -platforms, as discussed in `Jules.md`. Write clear, maintainable code, +platforms, as discussed in `AGENTS.md`. Write clear, maintainable code, adhering to the commenting guidelines, and if you need to add new third-party dependencies, document the rationale and update build configurations according to the established practices. Ensure your changes align with the overall best -practices detailed in `Jules.md`. +practices detailed in `AGENTS.md`. ## Key Directives for Jules AI -* **Prioritize `Jules.md`**: This document (`Jules.md`) contains +* **Prioritize `AGENTS.md`**: This document (`AGENTS.md`) contains repository-specific guidelines. Prioritize this information when making decisions about coding style, testing procedures, architectural patterns, and API usage. * **Adherence to Patterns**: Strive to follow the common patterns and best practices outlined here. This ensures consistency across the codebase. -* **Clarification for Deviations**: If the existing patterns in `Jules.md` do +* **Clarification for Deviations**: If the existing patterns in `AGENTS.md` do not seem suitable for a specific task, or if a deviation is necessary, please explicitly state this in your plan or request clarification before proceeding with implementation. -* **Updating `Jules.md`**: If you introduce a new, broadly applicable +* **Updating `AGENTS.md`**: If you introduce a new, broadly applicable pattern, or if a significant change to the build process or best practices occurs as part of your task, please include a step in your plan to update - `Jules.md` to reflect these changes. + `AGENTS.md` to reflect these changes. * **Testing**: Remember that integration tests are the primary method of testing. Ensure new features are accompanied by corresponding integration - tests as described in the 'Testing' section of `Jules.md`. + tests as described in the 'Testing' section of `AGENTS.md`. * **Commit Messages**: Follow standard commit message guidelines. A brief summary line, followed by a more detailed explanation if necessary. From 3645ec40ae1ed086c116235519851c552379b30f Mon Sep 17 00:00:00 2001 From: Jon Simantov Date: Wed, 25 Jun 2025 16:51:33 -0700 Subject: [PATCH 8/9] Revert again. --- AGENTS.md | 84 +++++++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 66 insertions(+), 18 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index db393be673..fce803dc12 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,5 +1,9 @@ # Introduction +> **Note on Document Formatting:** This document (`AGENTS.md`) should be +> maintained with lines word-wrapped to a maximum of 80 characters to ensure +> readability across various editors and terminals. + This document provides context and guidance for AI agents (like Jules) when making changes to the Firebase C++ SDK repository. It covers essential information about the repository's structure, setup, testing procedures, API @@ -34,8 +38,8 @@ instructions for your specific platform. * **Android SDK & NDK**: Required for building Android libraries. `sdkmanager` can be used for installation. CMake for Android (version 3.10.2 recommended) is also needed. -* **(Windows Only) Strings**: From Microsoft Sysinternals, required for Android - builds on Windows. +* **(Windows Only) Strings**: From Microsoft Sysinternals, required for + Android builds on Windows. * **Cocoapods**: Required for building iOS or tvOS libraries. ## Building the SDK @@ -74,9 +78,10 @@ generated in each library's build directory (e.g., ### Desktop Platform Setup Details -When setting up for desktop, if you are using an iOS `GoogleService-Info.plist` -file, convert it to the required `google-services-desktop.json` using the -script: `python generate_xml_from_google_services_json.py --plist -i GoogleService-Info.plist` +When setting up for desktop, if you are using an iOS +`GoogleService-Info.plist` file, convert it to the required +`google-services-desktop.json` using the script: +`python generate_xml_from_google_services_json.py --plist -i GoogleService-Info.plist` (run this from the script's directory, ensuring the plist file is accessible). The desktop SDK searches for configuration files in the current working @@ -175,8 +180,9 @@ Database). parameters if not relying on a `google-services.json` or `GoogleService-Info.plist` file. 2. **Service Instances**: Once `firebase::App` is initialized, you generally - obtain instances of specific Firebase services using a static `GetInstance()` - method on the service's class, passing the `firebase::App` object. + obtain instances of specific Firebase services using a static + `GetInstance()` method on the service's class, passing the `firebase::App` + object. * Examples for services like Auth, Database, Storage, Firestore: * `firebase::auth::Auth* auth = firebase::auth::Auth::GetAuth(app, &init_result);` * `firebase::database::Database* database = firebase::database::Database::GetInstance(app, &init_result);` @@ -192,8 +198,8 @@ Database). called as global functions within the `firebase::analytics` namespace, rather than on an instance object obtained via `GetInstance()`. Refer to the specific product's header file for its exact - initialization mechanism if it deviates from the common `GetInstance(app, ...)` - pattern. + initialization mechanism if it deviates from the common + `GetInstance(app, ...)` pattern. ### Asynchronous Operations: `firebase::Future` @@ -205,8 +211,8 @@ where `T` is the type of the expected result. `kFutureStatusInvalid`. * **Getting Results**: Once `future.status() == kFutureStatusComplete`: * Check for errors: `future.error()`. A value of `0` (e.g., - `firebase::auth::kAuthErrorNone`, `firebase::database::kErrorNone`) - usually indicates success. + `firebase::auth::kAuthErrorNone`, + `firebase::database::kErrorNone`) usually indicates success. * Get the error message: `future.error_message()`. * Get the result: `future.result()`. This returns a pointer to the result object of type `T`. The result is only valid if `future.error()` @@ -218,8 +224,8 @@ where `T` is the type of the expected result. ### Core Classes and Operations (Examples from Auth and Database) -While each Firebase product has its own specific classes, the following examples -illustrate common API patterns: +While each Firebase product has its own specific classes, the following +examples illustrate common API patterns: * **`firebase::auth::Auth`**: The main entry point for Firebase Authentication. @@ -308,6 +314,12 @@ API documentation. as mentioned in `CONTRIBUTING.md`. * **Formatting**: Use `python3 scripts/format_code.py -git_diff -verbose` to format your code before committing. +* **Naming Precision for Dynamic Systems**: Function names should precisely + reflect their behavior, especially in systems with dynamic or asynchronous + interactions. For example, a function that processes a list of items should + be named differently from one that operates on a single, specific item + captured asynchronously. Regularly re-evaluate function names as + requirements evolve to maintain clarity. ## Comments @@ -331,8 +343,9 @@ API documentation. * **Check `Future` status and errors**: Always check `future.status()` and `future.error()` before attempting to use `future.result()`. * A common success code is `0` (e.g., - `firebase::auth::kAuthErrorNone`, `firebase::database::kErrorNone`). - Other specific error codes are defined per module (e.g., + `firebase::auth::kAuthErrorNone`, + `firebase::database::kErrorNone`). Other specific error codes are + defined per module (e.g., `firebase::auth::kAuthErrorUserNotFound`). * **Callback error parameters**: When using listeners or other callbacks, always check the provided error code and message before processing the @@ -365,6 +378,13 @@ API documentation. otherwise ensuring the `Future` completes its course, particularly for operations with side effects or those that allocate significant backend resources. +* **Lifecycle of Queued Callbacks/Blocks**: If blocks or callbacks are queued + to be run upon an asynchronous event (e.g., an App Delegate class being set + or a Future completing), clearly define and document their lifecycle. + Determine if they are one-shot (cleared after first execution) or + persistent (intended to run for multiple or future events). This impacts + how associated data and the blocks themselves are stored and cleared, + preventing memory leaks or unexpected multiple executions. ## Immutability @@ -400,6 +420,29 @@ API documentation. integration, it can occasionally be a factor to consider when debugging app delegate behavior or integrating with other libraries that also perform swizzling. + When implementing or interacting with swizzling, especially for App Delegate + methods like `[UIApplication setDelegate:]`: + * Be highly aware that `setDelegate:` can be called multiple times + with different delegate class instances, including proxy classes + from other libraries (e.g., GUL - Google Utilities). Swizzling + logic must be robust against being invoked multiple times for the + same effective method on the same class or on classes in a + hierarchy. An idempotency check (i.e., if the method's current IMP + is already the target swizzled IMP, do nothing more for that + specific swizzle attempt) in any swizzling utility can prevent + issues like recursion. + * When tracking unique App Delegate classes (e.g., for applying hooks + or callbacks via swizzling), consider the class hierarchy. If a + superclass has already been processed, processing a subclass for + the same inherited methods might be redundant or problematic. A + strategy to check if a newly set delegate is a subclass of an + already processed delegate can prevent such issues. + * For code that runs very early in the application lifecycle on + iOS/macOS (e.g., `+load` methods, static initializers involved in + swizzling), prefer using `NSLog` directly over custom logging + frameworks if there's any uncertainty about whether the custom + framework is fully initialized, to avoid crashes during logging + itself. ## Class and File Structure @@ -465,9 +508,9 @@ management: module, but the fundamental responsibility for creation and deletion in typical scenarios lies with the Pimpl class itself. -It's crucial to correctly implement all these aspects (constructors, destructor, -copy/move operators) when dealing with raw pointer Pimpls to prevent memory -leaks, dangling pointers, or double deletions. +It's crucial to correctly implement all these aspects (constructors, +destructor, copy/move operators) when dealing with raw pointer Pimpls to +prevent memory leaks, dangling pointers, or double deletions. ## Namespace Usage @@ -579,3 +622,8 @@ practices detailed in `AGENTS.md`. tests as described in the 'Testing' section of `AGENTS.md`. * **Commit Messages**: Follow standard commit message guidelines. A brief summary line, followed by a more detailed explanation if necessary. +* **Tool Limitations & Path Specificity**: If codebase search tools (like + `grep` or recursive `ls`) are limited or unavailable, and initial attempts + to locate files/modules based on common directory structures are + unsuccessful, explicitly ask for more specific paths rather than assuming a + module doesn't exist or contains no relevant code. From 5be641d998d8bb1c41ca9b009d9489df1c794d8f Mon Sep 17 00:00:00 2001 From: Jon Simantov Date: Wed, 25 Jun 2025 17:05:37 -0700 Subject: [PATCH 9/9] Remove extra lines at the end. --- STYLE_GUIDE.md | 8 -------- 1 file changed, 8 deletions(-) diff --git a/STYLE_GUIDE.md b/STYLE_GUIDE.md index a2a5d4c946..a141ee0b66 100644 --- a/STYLE_GUIDE.md +++ b/STYLE_GUIDE.md @@ -376,11 +376,3 @@ e.g. } } ``` - -# Data Types - -* Time: [Milliseconds since the epoch](https://en.wikipedia.org/wiki/Unix_time) - -# More on C++ and games - -* Performance Oriented C++ for Game Developers (very incomplete).