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

Version Migration and Backwards Compatibility #4

Open
marcglasberg opened this issue Dec 29, 2023 · 8 comments
Open

Version Migration and Backwards Compatibility #4

marcglasberg opened this issue Dec 29, 2023 · 8 comments

Comments

@marcglasberg
Copy link

marcglasberg commented Dec 29, 2023

Hey, apologies for the lengthy issue. It turned out to be way more detailed than I initially planned, and took a lot longer to write than I expected. Please take your time to go through it, no need to respond quickly. At least I hope it serves as a starting point for ideas. If you think it's not suitable and end up implementing a different approach, feel free to do so!


Proposal for Version Migration

We need to make sure our server works with our client app, and the app should never break. On websites, changes happen instantly for everyone. But with mobile apps, it's different. Users have to update their app to see changes. Most people's apps update automatically within a few days, but some might use older versions for a long time.

The best thing for our users is to keep our server working with all older versions of the app. This way, everyone can keep using the app without issues. However, this isn't always doable. Sometimes, new features need new information from the app to the server. Other times, it's just too hard and costly to maintain this compatibility for a long time.

Because of this, our client apps need a way to recognize when they're too old to work with the server. When this happens, the app should lock itself and tell the user to update it.

In my opinion, the best cost-benefit is to make the server work with the version of the app right before the latest one. Then, we stop supporting the versions older than that. This works well because most users update their apps quickly. This method has a good balance between developer effort and good user experience.

But many companies would prefer to support more than one previous version, and that should be allowed as well.

This proposal suggests two things:

  • The app should check if it works with the server. If it doesn't, it should "lock" and tell the user to update the app.

  • Celest should make it as easy as possible for developers to keep the server working (backwards compatible) with a chosen set of older versions of the app.

Specific Celest Challenges

When a Flutter client connects to a TypeScript or Java server, they by definition use totally separate code. In Celest, as the API definition is shared by the client and the server, you are forced to share the classes that are present in the API signature.

Sharing the code base like this is a good thing and main selling point for Celest, but brings some specific challenges.

Imagine the server's Order class has a creditCardNumber field, but the client's version only shows the first4digits. And suppose we want our API to look like this:

var order = Order(...);
celest.apis.order.buy(order);

I'll discuss 5 alternatives to solve this problem.

1. Separate Order Classes

One alternative is different ClientOrder and ServerOrder classes for the client and the server:

abstract class Order {
  // common fields
} 

class ClientOrder extends Order {  
  int first4digits;  
}

class ServerOrder extends Order {  
  int creditCardNumber;
}

// Client code:
var clientOrder = ClientOrder(...);
celest.apis.order.buy(clientOrder);

// Server code:
Future<void> order(
  FunctionContext context,
  ClientOrder clientOrder,
) async {
     var serverOrder = ServerOrder(        
        creditCardNumber: getCreditCardNumber(first4digits),
    );
    ...
}

Disadvantages: May lead to code duplication, as similar fields and functionalities are defined in multiple classes. That's why maybe it's good to have a superclass like the Order class I defined above. It also complicates the codebase, as now we have 2 or 3 classes instead of just one.

2. One-Way Data Flow

For situations where data only goes from the client to the server (in other words, when the order is present in the parameter list, and not in the return value), we can solve this by defining special toJson and fromJson methods:

abstract class Order {
  // common fields
  
  Json toJson() => Json({ 'first4digits': first4digits });
  
  static ServerOrder fromJson(Json json) {   
    return ServerOrder(creditCardNumber: getCreditCardNumber(json['first4digits']));
  }
} 

class ClientOrder extends Order {  
  int first4digits;      
}

class ServerOrder extends Order {  
  int creditCardNumber;    
}

// Client code:
var clientOrder = ClientOrder(...);
celest.apis.order.buy(clientOrder);

// Server code:
Future<void> order(
  FunctionContext context, 
  Id userId,
  Order order,
) async {
     ServerOrder serverOrder = order as ServerOrder;
    ...
}

Disadvantages: It forces the developer to create and parse Json, which is something Celest is supposed to do for us.

3. Null Fields in Shared Class

Another approach is using a single Order class with some nullable fields in the client:

class Order {
  String? customerName;
  int first4digits;
  int? creditCardNumber;
  // other fields
}

// Client code:
var clientOrder = Order.create(...);
celest.apis.order.buy(userId, order);

// Server code:
Future<void> order(
  FunctionContext context, 
  Id userId,
  Order order,
) async {
     order.fill(
        customerName: getCustomerName(userId),
        creditCardNumber: getCreditCardNumber(first4digits),
    );
    ...
}

Disadvantages: All the nullable fields make the class harder to understand, bringing ambiguity. It violates the "Single Responsibility Principle", as the class is now responsible for different things in the client and the server. Is increases the risk of null reference exceptions, makes logic validation difficult, and makes it harder to maintain the class and write tests.

4. API-Specific Class

When we only need to send a subset of fields, we can consider using a specific Order class for the API:

class Order {
  Id userId;
  int first4digits;  
}

class ClientOrder {
  String customerName;
  int first4digits;
  // other fields
  
  Order toApi() => Order(userId, first4digits);  
}

class ServerOrder {
  String customerName;
  int creditCardNumber;
  // other fields
  
  ServerOrder.fromApi(Order order) {
    customerName = getCustomerName(order.userId);
    creditCardNumber = getCreditCardNumber(order.first4digits);
  }
}

// Client code:
var clientOrder = ClientOrder(...);
celest.apis.order.buy(clientOrder.toApi());

// Server code:
Future<void> order(
  FunctionContext context, 
  Order order,
) async {
     var serverOrder = ServerOrder.fromApi(order);
    ...
}

Disadvantages: It increases complexity by adding an extra layer of abstraction, requiring developers to maintain additional classes and mappings between the API-specific class and the full client and server classes. This raises the potential for errors and inconsistencies, particularly during updates or changes to the data structure.


Anyway, maybe I'm overcomplicating things, and there is a simpler way to think about all this. But I believe there is going to be a learning curve and some experimentation before we learn the best practices. Once the community finds out the best practices I think Celest can publish those as recommendations.

And while all the alternatives above have disadvantages, I think all of them are still much better than the alternatives for Flutter apps that don't use Celest. When a Flutter client connects to a TypeScript or Java server, we are just forced to use
two different classes, written in two different languages, and we have to convert between them somehow and keep the conversion in sync when things change. So even if we don't find the perfect solution, I think Celest is still an order of magnitude improvement over the alternatives.

Anyway, the reason I started thinking about all this, is because I wanted to better understand how I'd be creating the APIs and defining the classes before I could think about how to migrate those APIs and classes. Looking at all the alternatives above, I think it doesn't make much of a difference in terms of solving the "version migration problem". Seems to be the same for all of them.

Proposal

Typically, the client app will be equal or older in relation to the server, as the server is usually updated first, before we publish a new client version to the app stores. If we assume this, it simplifies things. This means it's only the server that needs to deal with older app versions, not the other way around.

Let's say there are 5 app versions:

  • App versions 1 and 2 are now unsupported
  • App versions 3 and 4 are deprecated but still accepted
  • And the current app is version 5

The server needs to:

  • Throw an unsupported exception when communicating with app versions 1 and 2.
  • Accept app versions 3 and 4, but let the client know they are deprecated.
  • Accept app version 5.

When receiving an unsupported exception, the client app should lock the app and display a modal dialog telling the user to upgrade.

When receiving a deprecation warning, the client app may optionally let the user know there is a newer version, but things should work otherwise. This cannot be done via throwing an exception, but some cookie may be set in the response, that Celest could read and make available to the app (for example, by calling some special annotated function when that happens).

I'll consider an API where we save a Student:

var result = celest.apis.students.save(student);

The class name for the current version is simply Student, it has no "index". So Student is the class name for the current version 5. For versions 3 and 4 we could have classes Student3 and Student4.

The server needs to be able to receive any of Student, Student3 and Student4 parameters, and return Value, Value3 and Value4, respectively.

clas Student {
    String firstName, lastName;
    int yearOfBirth;
}

clas Student4 {
    String firstName, lastName;
    int age;
}

clas Student3 {
    String fistAndLastName;
    int age;
}

My proposal is that we annotate deprecated functions:

Future<Value> save(FunctionContext context, Student student) async { 
   // Do stuff.
}

@deprecatedFunc('save', version: 4)
Future<Value4> save4(FunctionContext context, Student4 student4) async {
    var student5 = student4.toStudent5();
    Value value5 = await save(context, student5);
    return Value4.fromValue5(value5); 
 }

@deprecatedFunc('save', version: 3)
Future<Value1> save3(FunctionContext context, Student3 student3) async {
    var student4 = student3.toStudent4();
    Value value4 = await save4(context, student4);
    return Value3.fromValue4(value4); 
 } 

Choosing the right function

When Celest gets a request for save, it does the following:

  • If the JSON fits the current (non-annotated) save function, call it.
  • Otherwise, try all the other functions annotated with @deprecatedFunc('save') util one fits and then call it. In this case, set the "deprecation warning cookie" in the response.
  • If no function fits, throw an UnsupportedException.

When I say the "JSON fits" I mean that the JSON has the same fields as the function's parameter list.

For example, this request would fit Student and the save function:

{
    "firstName": "John",
    "lastName": "Doe",
    "yearOfBirth": 1980
}

While this would fit Student4 and the save4 function:

{
    "firstName": "John",
    "lastName": "Doe",
    "age": 40
}

And this would fit Student3 and the save3 function:

{
    "fistAndLastName": "John Doe",
    "age": 40
}

Advantages of the proposal

  1. There is no need for the developer to create/parse Json to choose the correct function to call. It's still all done with classes.

  2. For clean code reasons, I think it's important that the current version of the functions and classes have no index. This way, the developer doesn't need to worry about the index when writing the current code. It's just Student and Value.

  3. In the above code I'm converting student3 and student4 to their newer format by using student3.toStudent4() and student4.toStudent5(), and calling the newer version functions. In other words, I'm "chaining" older versions into newer ones. That can work in a lot of cases, and makes it very easy to migrate code. But when something more complex is needed, we are free to do whatever we want, in the respective function body.

  4. It's likely that deprecated classes like Student3 and Student4 are just simple classes with not a lot of logic, especially if we can convert them to the current version and let the current code process it. In those cases, these old version classes are very simple, and they can be moved to the same directory as the API functions. They become implementation details of the API, and not part of the domain.

  5. Since the supported older class versions remain in the code, it's possible to create tests for the compatibility of those older versions.

  6. It's very easy to remove deprecated code when it becomes unsupported. Whenever we don't want to support version 3 anymore, we just remove the save3 function, Student3 and Value3 classes.

  7. The deprecation warning cookie and the unsupported exceptions are issued/thrown automatically by Celest. The developer doesn't need to worry about it.

  8. It allows evolving the API:

    • Complete removal of functions: The developer just needs to add a @deprecatedFunc annotation to some function, without creating a non-deprecated version.
    • Changing the names of functions: The developer just needs to add a @deprecatedFunc annotation to some function, and create the non-deprecated version with the new name.

Disadvantages

There are probably more disadvantages to this proposal, but the only one I can see at the moment is that the new version of the function needs to result in a different request JSON than the old version.

I don't think this is a big deal, but it's something to keep in mind. An alternative would be for the client to explicitly send the version number in the JSON. Another alternative is that the developer sets a version globally in the client, and Celest send this as a cookie to the server.

Diffing the migration

Celest can automatically check if the code currently being developed is indeed backwards compatible with the older app versions.

Suppose the developer presses some button in the IDE, and the Celest IDE plugin sends a getSchema request that gets the JSON definitions of all the currently published functions.

The plugin then compares it with the JSON created from the code in the IDE. If they are indeed compatible, it shows a message saying so; if they are not, it shows the differences, which will help the developer to correct it.

Let me show how to do this with the example above. We want to make sure the save function is compatible with the older versions.

Suppose:

  • We are currently developing the app version 5 in the IDE.
  • The server is running version 4, which supports versions 2, 3 and 4 of the app.

The IDE plugin sends a getSchema request to the server, and gets the following JSON:

{
    "save": {
        "4": {
             "firstName": "string",
             "lastName": "string",
             "age": "number"
             },            
        "3": {
             "fistAndLastName": "string",
             "age": "number"
             },            
        "2": {
             "fistAndLastName": "string",
             },                 
    }
}

It then gets the JSON from the code in the IDE:

{
    "save": {
        "5": {
             "firstName": "string",
             "lastName": "string",
             "yearOfBirth": "number"
             },            
        "4": {
             "firstName": "string",
             "lastName": "string",                                
             "age": "number"
             },            
        "3": {
             "name": "string",
             "age": "number"
             },            
    }
}

Diffing these two JSONs, the plugin shows the following message:

Function `save`:
 
* Version 5 is being developed.
 
* Version 4 is deprecated and supported.

* Version 3 is deprecated and incorrectly supported:

   Your code contains:
   
     "3": {
          "name": "string",
          "age": "number"
          },            
   
   But it should be:
   
     "3": {
          "fistAndLastName": "string",
          "age": "number"
          },            

* Version 2 is unsupported. If you wish to support it, it should contain:

     "2": {
          "fistAndLastName": "string",
          },               

Final Thoughts

This proposal allows the new version of our server functions work with older versions of the app.

However, it doesn't let old app versions continue to use old function versions, as proposed by @caseycrogers :

I make a breaking change and ship v2. v2 clients will crash trying to talk to v1 server functions,
and vice versa
How do I ensure that residual v1 clients continue to hit server v1 and new v2 clients hit server
v2?

I haven't experienced a situation where I needed an old app version to communicate with an old function version. This might be uncommon, or maybe it's just something I haven't encountered.

But I see other uses for that anyway: Having different function versions available at the same time could let us set up a test environment for new app versions using TestFlight, before making them live.

So, there's value in keeping different function versions available simultaneously. However, this is a separate feature from what I've discussed here.

@abdallahshaban557
Copy link
Contributor

Hi @marcglasberg - thank you so much for the well-written and thought-out scenarios here! We are going to discuss these points and then share our thoughts. This is amazing, PLEASE don't apologize for making it detailed and clear, we really appreciate it!

@caseycrogers
Copy link

caseycrogers commented Dec 31, 2023

I haven't experienced a situation where I needed an old app version to communicate with an old function version. This might be uncommon, or maybe it's just something I haven't encountered.

For me, I like this idea because it means an old version of my app continues to work exactly as it did without requiring any effort on my part to make the latest version of the server backwards compatible. I don't have to keep old version tagged methods around-which feels like could get messy pretty fast.

The downside, of course, is this isn't really coherent with different server versions sharing a DB-if the function changes correspond to a DB change I need to either rely on the changes being backwards compatible or write a translation layer as you've outlined. In practice, my current app has no backend-just SQL tables+views and I ensure any schema changes are backwards compatible (eg if a field is added it's nullable until the clients before it was added are no longer supported).

But on some level I'm not sure there is a very coherent way around some messiness like this-no matter what. Your example with translation layers doesn't work for scenarios where a brand new field is added:

{
    "firstName": "John",
    "lastName": "Doe",
    "yearOfBirth": 1980,
    "favoriteColor": "#A73340"
}

The client would have had to prompt the user for their favorite color in order to get this information-so there's no way for the old client to provide the new field. So we end up falling back to my existing ad hoc system: new fields are optional until the pre-field client is dead.

I guess I'd like to see a versioning migration system to be broken down by how it handles the following three scenarios:

  1. Field removed
  2. Field added
  3. Field modified (including renames in this category)

Not to complicate things even more, but how are DB schema and server functions kept in sync? Is there a deploy system that guarantees both are migrated in a single transaction?

@marcglasberg
Copy link
Author

marcglasberg commented Jan 2, 2024

@caseycrogers

Your example with translation layers doesn't work for scenarios where a brand new field is added

I think it works. It must, because my proposal basically lets you change the API in any desired way, without any limitation, for both the current version and any of the older versions, separately.

But, of course, you need to change it in a way that makes logical sense to you app. If you are adding a favorite color to the new version 5 of the app, the function v5 depends on the UI of this v5 app:

Option 1: If the UI of the app v5 allows the user to select a favorite color, but that's optional.

  • The function v5 returns favoriteColor as optional (string or null).
  • The deprecated function v4 simply doesn't return favoriteColor.

Option 2: If the UI of the app v5 expects a favorite color to be always present.

  • The function v5 returns favoriteColor as a string. If a color was not yet chosen (is not present in the database), it simply returns a default color, like black.
  • The deprecated function v4 simply doesn't return favoriteColor.

For me, I like this idea because it means an old version of my app continues to work exactly as it did without requiring any effort on my part to make the latest version of the server backwards compatible.

As I said in my Final Thoughts section above, I'm not opposed to also allow an app version to communicate with an old function version, as it may work sometimes, depending on the desired changes and the specifics of the app. And yes, when it works it will be easier that way.

But this won't ALWAYS work. Most changes to databases, business rules, connection to third-party services etc, are usually not backwards compatible enough so that you can still keep the old app version working with the old function, working with the new database, new business rules, and new third-party interactions.

A simple example: Suppose your app calculates some fee that users must pay. You want to change the way the fee is calculated, at the same time for all users. If your new function version 5 changes the way the fee is calculated, you cannot continue to allow users of the previous version 4 to use the old calculation. You must now create a deprecated function compatible with the API of version 4, but with the new calculation.

In other words, I think we need to be able to change the API in any desired way, without any limitation, which is the goal of my proposal.

@dickermoshe
Copy link

dickermoshe commented Jan 7, 2024

I haven't experienced a situation where I needed an old app version to communicate with an old function version. This might be uncommon, or maybe it's just something I haven't encountered.

This is very common. If I'm changing params for an existing endpoint, I want the old apps to still use the old endpoint and the new apps to use the new one.
If I refactor an endpoint to collect emails from new users, I would want this to be required. This will be helpful later when using this endpoint, I will get a type-error if I don't include it. However I still want the old apps to work.

@dickermoshe
Copy link

dickermoshe commented Jan 7, 2024

The benefit to having a FooRequest and FooResponse be separate classes is that id won't be an optional param.
It will be int on FooResponse and wont exists at all on FooRequest

There are many more like this beyond id:

  • Date Created
  • Date Modified
  • Total Tweet Counts
  • ... anything generated on the server.

Having them extend from one base class will help for any typing issues.

class BaseFoo{
  final String username;
  const BaseFoo({required this.username});
}
class RequestFoo extends  BaseFoo{
  const RequestFoo({required super.username});
}
class ResponseFoo extends  BaseFoo{
  final int id;
  const ResponseFoo({required super.username, required this.id});
}

I've been using OpenAPI codegen for a while now. It's truly a breath of fresh air.

No more if (class.username!= null) Text(class.username), if something is required, it not nullable.

Celest being written in dart itself has the potential to give this superpowers.
However, 1 class to rule them all is a regression in my own dumb opinion.

@caseycrogers
Copy link

caseycrogers commented Jan 7, 2024

Iirc Google/gRPC regularly manages breaking changes in end points this way (though I could be wrong, it's been forever since I've touched the stuff).

In my mind ideal default behavior is that old clients hit old versions of the endpoints because the majority of the time that will yield functional behavior with no extra effort on my part. It also feels the most intuitive-by default the client code always talks to it's own version of the server code as if the latter has literally been shipped with the former.

When I need a translation layer and/or need to force a change across older versions (eg hot fixing a bug), then I'd find the system you've outlined highly useful. But in my mind it's better if that system is opt in so that the typical path is low effort.

To continue with the favorite color example (which isn't entirely contrived, this is a scenario that comes up repeatedly with my app), let's say favorite color is required in v5, and I want favorite color to be null in my DB for old clients. I suppose there are three potential approaches:
A. v4 clients hit v5 functions, but the v5 function has favorite color as an optional arg
B. v4 clients hit v5 functions, but I've used your proposed system to write a translation layer, the v5 function has favorite color as a required but nullable arg
C. v4 clients hit v4 functions

C is trivial for me as a developer. B is fairly annoying as it's a fair amount extra work and noise in my code base. A seems okay, but is not ideal as it makes it easy for me to accidentally forget to specify favorite color in my v5 client.

This is why I like C as default behavior and B as opt in behavior. No matter what I do, my DB is going to have to be backwards compatible with v4 clients so I might as well have my server backwards compatible by default by just retaining the v4 functions. A server translation layer can't help me here because the value has to come from the client.

@dickermoshe
Copy link

dickermoshe commented Jan 7, 2024

My only additional proposal is that this is placed in a one decorator on the save function and not all over the place:

@fallbacks({
  Student3:save3,
  Student4:save4,
})
Future<Value> save(FunctionContext context, Student student) async { 
   // Do stuff.
}

Future<Value4> save4(FunctionContext context, Student4 student4) async {
    var student5 = student4.toStudent5();
    Value value5 = await save(context, student5);
    return Value4.fromValue5(value5); 
 }

Future<Value1> save3(FunctionContext context, Student3 student3) async {
    var student4 = student3.toStudent4();
    Value value4 = await save4(context, student4);
    return Value3.fromValue4(value4); 
 } 

@dickermoshe
Copy link

@marcglasberg
I'm trying to find all the issues with it, because it seems too good to be true!
But besides for these few issues, this seems awesome!

My only opinion is that typing should be super strict. If we know something is null, That should be exposed to the user. Even if it requires 2 classes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants