Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Function Cookies #40

Open
marcglasberg opened this issue Feb 8, 2024 · 5 comments
Open

Function Cookies #40

marcglasberg opened this issue Feb 8, 2024 · 5 comments
Labels
triaging Initial investigation into the issue

Comments

@marcglasberg
Copy link

marcglasberg commented Feb 8, 2024

I want to use my 18n_extension package to create translations in the Celest backend.

That's currently not possible, because the 18n_extension package needs Flutter. I'd have to extract the Dart-only code to a separate 18n_extension_core package.

That's doable. But the second problem is Celest knowing the language of the user. I could send this information as a function parameter. For example, if the user speaks Spanish:

// Return "You bought 3 IBM shares." if lang is en_US, 
// or return "Usted compró 3 acciones de IBM" if lang is sp_ES.
String result = await celest.functions.stocks.buy(Stock('IBM'), howMany: 3, lang: 'sp_ES');

But this would force me to include this lang parameter in every single function that could need translated texts.

Instead, I'd like to define a language cookie: celest.setCookie('lang': 'sp_ES') that would be sent in all function requests. And then use a function middleware to extract and use this information.

@dnys1 dnys1 added triaging Initial investigation into the issue and removed feature request labels Feb 9, 2024
@dnys1
Copy link
Member

dnys1 commented Feb 9, 2024

Hi @marcglasberg, for your first question,

I'd have to extract the Dart-only code to a separate 18n_extension_core package.

This is unfortunately true since Flutter cannot be compiled on the server and, AFAIK, even having a transitive dependency on flutter in a Dart-only codebase will throw errors when trying to build the Dart app. I will do some research to see if this is still the case.


On your second point, it sounds like you are wanting a global context of sorts to store properties which apply to all function calls. I would want to better understand this use case more.

  • Is the pain around passing lang on every function call because you would need to pull that information from somewhere locally? And so setting it once allows you to use Celest as a cache for this info?
  • Would it be possible to accidentally pass the wrong language because the Celest cache was out-of-date?
  • Would there be conflicts when a function with a lang parameter of some other type, like Enum or a custom class, is added later on? How would these conflicts be resolved?

As a general principle, I try to opt for explicitness vs. implicitness, even at the cost of verbosity. I find it's easier to reason about the behavior of code from a fresh set of eyes when all the logic is laid out plainly. For example, this is the reason we chose to have the funny annotation syntax for environment variables (@Env.myEnv required String myEnv). While it is a bit verbose, the dependencies of every function are plainly clear, and the few extra seconds of typing can potentially save you minutes or hours of debugging later.

That's also why for this feature, I would be concerned, since we would be giving this up. A global property bag would be unaware of the schema of any individual function and so would likely be dealing in strings and Object?s quite a bit. While it may be convenient in the short term, I see it potentially becoming a source of confusion and bugs later on.

Curious to hear more about your perspective, though.

@marcglasberg
Copy link
Author

marcglasberg commented Feb 10, 2024

Ok, I understand your concerns, so I think I need to explain my needs better and give you more detail.

When you discuss explicitness vs. implicitness you are basically saying you care about clean-code, right? I mean, the end goal is you want the users to be able to understand what they are doing, and you want new developers to understand the code base and onboard without much hassle. Crafting an API that developers love means applying the general principles you think are important to reach that end goal, and explicitness vs. implicitness is one of them. You most likely have other general principles, and sometimes some of them may conflict with each other. And when there is conflict, you go back to the end goal (clean-code) to decide which of those general principles "wins" under the specific circumstances.

If I suggest two extra general principles, could you please tell me if they apply to Celest or not?

First one is allowing the Tech Lead to set up the basic architecture infrastructure of the app, and not force that upon the regular developers. When I'm the Tech Lead for 3 teams of 5 people each, I want to be able to set some stuff up once, and forget about it. Those 15 developers don't need to know about it, because it's not their day-to-day work. It's my job, not theirs. And some of these are junior devs, who will forget to add some annotation or call some method they should, because it's not directly related to what they are working on.

In a way, that's what you are doing when you defined the possibility of middleware layers. Devs could add a log function call to the start of every function, but that's a nuisance and it would force those 15 developers to remember to do that whenever they create a function. Chances are, sometimes they will forget, and some functions won't be logged. And it makes their code verbose. Instead, as the Tech Lead, I set up the middleware and remove this preoccupation from my developer's heads.

Another general principle could be not forcing devs to mix separate, orthogonal concerns. When I write:

celest.functions.stocks.buy(Stock('IBM'), howMany: 3, lang: 'sp_ES');

The lang value is obviously irrelevant for buying stocks. Having that parameter there is even a bit funny, a code smell. As a Tech Lead, I want my developers to only think about the necessary code to buying stocks, which is complex enough. The language of the user is totally irrelevant for the process of buying stocks. It's an orthogonal concern. The only reason I need it, is that the function may throw an error, and in this case the error message must be translated to the user's language. This lang variable will not be used directly, most of the time. For instance, with the i18_extension package, we translate a string by simply appending .i18n to it. Here is the real code from my example app:

    if (...) {
      throw const UserException('Not enough money to buy stock'.i18n);
...

There are many possible orthogonal concerns here that the backend may need to know about the user/device. For example:

  • The user's id. never changes
  • The device language and region. changes very rarely
  • The frontend app version. never changes
  • The screen resolution (is it a phone or a tablet) changes only when the user rotates the phone
  • The OS (is it Android, or iOS) never changes

A complete function would be:

celest.functions.stocks.buy(
   Stock('IBM'), 
   howMany: 3, 
   userId: 'slF3bnbHowJ56HptE1gEjHGre', 
   lang: 'sp_ES', 
   res: '1920,1080', 
   os: Os.android'
);

Anyway, the point is not that I'm sending a lot of information, but that I'm sending information that's the same for all function calls (for that specific user/device) and that my developers will find mostly irrelevant and shouldn't be forced to deal with.

To answer your questions:


Is the pain around passing lang on every function call because you would need to pull that information from somewhere locally? And so setting it once allows you to use Celest as a cache for this info?

This information must be sent by the frontend on every function call. There is no need to cache it (in the backend). The point is that it's not directly related to the function call objective (i.e. "buying stock"). It's related to the user/device, and part of the environment. It never changes for that user/device, or it changes very rarely.


Would it be possible to accidentally pass the wrong language because the Celest cache was out-of-date?

No. This information never changes, or rarely changes. For example, in the example app this code currently changes the language:

var newLocale = isSpanish ? const Locale("en", "US") : const Locale("sp", "ES");
I18n.of(context).locale = newLocale;

I'd like to be able to change it also on Celest like this:

var newLocale = isSpanish ? const Locale("en", "US") : const Locale("sp", "ES");
I18n.of(context).locale = newLocale;
celest.setCookie('lang': newLocale.toString()); // Here!

Would there be conflicts when a function with a lang parameter of some other type, like Enum or a custom class, is added later on? How would these conflicts be resolved?

No. Cookies are not to be used for general communication. They can be Strings only, and they are for the Tech Lead use only, and to send the quasi-immutable environment information only.

They should not be sent as part of the JSON. This is not what I want:

POST /api/buy HTTP/1.1
Host: example.com
Content-Type: application/json
Content-Length: 56

{
  "stock": "IBM",
  "howMany": 3,
  "lang": "en_US"
}

What I actually want is this:

POST /api/buy HTTP/1.1
Host: example.com
Content-Type: application/json
Content-Length: 38
Cookie: lang=en_US; res=1920,1080

{
  "stock": "IBM",
  "howMany": 3,
}

My middleware could then extract and process the cookies. It could be something like this:

String res = ""; // Global variable that can be seen in the function.

// A middleware that extracts the frontend environment from the cookies, and make it available for the function.
class logResponses implements Middleware {

  Handler handle(Handler handler) {
    return (request) async {
       I18n.setLang(handler.cookie('lang'))
       res = handler.cookie('lang');

       return await handler(request);
    }}}

@dnys1
Copy link
Member

dnys1 commented Feb 10, 2024

Really appreciate the discussion—that helps clarify things dramatically!

I agree this is a gap in our current offering. Let me consider some ways to implement this such that it is type-safe, non-intrusive, and meshes with upcoming features like authorization, for example, where the caller's information serves similarly as "ambient context" which is not relayed via the JSON body.

@dnys1
Copy link
Member

dnys1 commented Mar 11, 2024

The latest release includes @Context.user for injecting user information which comes from the ambient context.

What are your thoughts on extending this to support arbitrary context. Something like this:

const currentLocale = Context<Locale>();

Future<String> sayHello({
  @currentLocale required Locale locale,
}) async {
  if (locale.languageCode == 'es') {
    return 'Hola, amigo!';
  }
  return 'Hello, friend!';
}

On the frontend, you could do the following to change the context at any point:

celest.context.currentLocale = const Locale('es');

@marcglasberg
Copy link
Author

marcglasberg commented Mar 12, 2024

That's really not good in this case. The user language is a cross-cutting concern, I don't want to have to deal with it explicitly.

Using my https://pub.dev/packages/i18n_extension translations package you translate by adding .i18n to the end of the text. I want to be able to write this:

Future<String> sayHello() async { 
  return 'Hello, friend!'.i18n;
}

To that end, I need to intercept it at the middleware level, not in the function level. After intercepting it at the middleware level I must call SetDefaultLocale(locale);.

My question is, how can I intercept it at the middleware level?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triaging Initial investigation into the issue
Projects
None yet
Development

No branches or pull requests

3 participants