DEV Community: Piotr Nieweglowski

The problem with HTTP PUT method and possible ways to resolve it

Piotr Nieweglowski — Thu, 17 Dec 2020 21:45:03 +0000

Introduction

In this post I'm going to focus on the problem related to HTTP PUT method. I will show by example Web API HTTP PUT method which can lead to bugs in the code and show you how to handle such scenarios to make your code reliable. At the bottom of the post, there is a github repository with all codebase used here.

How should we handle updating of the data in API?

Probably the most common way to update the data in API is usage of PUT HTTP method.

Let's take a look at the code:

public class Customer
{
    public int Id { get; set; }
    public string Name { get; set; }
    public string City { get; set; }
    public bool IsPremiumMember { get; set; }
}

Let's assume that we develop a web-shop application. When user spends some particular amount of money, she or he will become a premium member and has the opportunity to purchase some goods with the extra discount. For sake of simplicity we put some good practices like domain-centric logic aside. In our example the shop is very young startup with only critical functions in their system and every update on the data is done manually by stuff. To update the customer's data, we need to trigger a Put method of the controller class:

[ApiController]
[Route("[controller]")]
public class CustomerController : ControllerBase
{
    private static readonly IEnumerable<Customer> _customers = new List<Customer>()
    {
        new Customer { Id = 1, Name = "John", City = "Cracow", IsPremiumMember = false }
    };

    [HttpGet("{id}")]
    public IActionResult Get(int id)
    {
        return Ok(_customers.FirstOrDefault(x => x.Id == id));
    }

    [HttpPut]
    public IActionResult Put(Customer customer)
    {
        var toUpdate = _customers.First(x => x.Id == customer.Id);
        toUpdate.Name = customer.Name;
        toUpdate.City = customer.City;
        toUpdate.IsPremiumMember = customer.IsPremiumMember;
        return NoContent();
    }
}

John has moved recently to Warsaw. He's calling to the shop and tells that fact. Bob, who is a worker of the shop is picking up the phone and is going to update John's address as requested. He opens a web browser with the application. The frontend layer calls backend get method and gets the data:

{
    "id": 1,
    "name": "John",
    "city": "Cracow",
    "isPremiumMember": false
}

Bob is just going to update the data but before that he wants to make a coffee.

At the same time Alice who works in the same team as Bob needs to perform an update operation on Johns account as well. We know that John spends a lot of money and his account needs to be promoted to premium level. Alice opens the browser and gets the same data. She clicks 'Promote account to premium' button and updates the data. After refreshing a browser she gets the data:

{
    "id": 1,
    "name": "John",
    "city": "Cracow",
    "isPremiumMember": true
}

Bob goes back to his PC with a mug of coffee and he's going to finish his task. He amends the city to Warsaw and clicks update button. What will be the result of such scenario? Let's check that and call Get method. The result is:

{
    "id": 1,
    "name": "John",
    "city": "Warsaw",
    "isPremiumMember": false
}

What?! John is not a premium user? You may ask: how it is possible? It can happen because PUT performs update on all fields. We send to backend a set of data and we update every single field except the id. The newly updated field can be overwritten with the old value like in Alice and Bob case. We don't want to have such vulnerability in our code. Let's check what we can do in order to mitigate this issue.

Versioning

The first method to solve the issue is versioning. We need to assign initial version to John's data and increment that after every update. Let's change the code and use this mechanism:

public abstract class VersionedEntity
{
    public long Version { get; set; }
}

public class Customer : VersionedEntity
{
    public int Id { get; set; }
    public string Name { get; set; }
    public string City { get; set; }
    public bool IsPremiumMember { get; set; }
}

We created VersionedEntity abstract class because if we create any new entity it will be easier to maintain solution but it is not required. We could add Version property directly to Customer class.

OK, we have a version stored in the system, let's try to reproduce Alice & Bob scenario and check if the issue still exists.

Bob makes GET call to fetch the data, a result is:

{
    "id": 1,
    "name": "John",
    "city": "Cracow",
    "isPremiumMember": false,
    "version": 1
}

Bob goes to make a coffee
Alice opens a browser, gets the same data from GET method.
Alice updates the data, she sets 'isPremiumMember' to 'true', refreshes the browser to get the data:

{
    "id": 1,
    "name": "John",
    "city": "Cracow",
    "isPremiumMember": true,
    "version": 2 // note that version was incremented
}

Bob is back. He wants to amend the city from 'Cracow' to 'Warsaw'. After clicking 'Update' (his browser has still data with version: 1) button he gets the message:

The data you're trying to update has been changed. Please reload data and try once again.

All right! The issue does not exist! How did we achieved that? Apart from adding the version to the Customer class, we needed to change a controller method as well:

[HttpPut]
public IActionResult Put(Customer customer)
{
    var toUpdate = _customers.First(x => x.Id == customer.Id);
    if (toUpdate.Version != customer.Version) 
    {
        return BadRequest(
            "The data you're trying to update has been changed. " +
            "Please reload data and try once again.");
    }
    toUpdate.Name = customer.Name;
    toUpdate.City = customer.City;
    toUpdate.IsPremiumMember = customer.IsPremiumMember;
    toUpdate.Version++;
    return NoContent();
}

Before updating we check if passed version of the data is the latest. If not, a bad request with a proper message is returned. That's it! Let's go to the next way of handing the 'put issue'.

PATCH instead of PUT

The problem related to PUT is that it updated all fields. Not only changed but all of them. PATCH has different concept it applies only changes without touching unchanged parts. To get more information about patch please take a look at the site: http://jsonpatch.com/. To replace PUT with PATCH in our example we need to add two nuget packages to our solution:

<PackageReference Include="Microsoft.AspNetCore.JsonPatch" Version="5.0.1" />
<PackageReference Include="Microsoft.AspNetCore.Mvc.NewtonsoftJson" Version="5.0.1" />

The next step is to add Newtonsoft Json to our services in Startup class:

public void ConfigureServices(IServiceCollection services)
{

    services.AddControllers()
            .AddNewtonsoftJson();

    services.AddSwaggerGen(c =>
    {
        c.SwaggerDoc("v1", new OpenApiInfo { Title = "Blog", Version = "v1" });
    });
}

The final step is replacing Put with Patch method in controller class:

[HttpPatch("{id}")]
public IActionResult Patch(int id, JsonPatchDocument<Customer> patchDoc)
{
    var toUpdate = _customers.First(x => x.Id == id);
    patchDoc.ApplyTo(toUpdate);
    return NoContent();
}

To generate PATCH operation you can use additional libraries for your frontend application (you can find them on linked website). For this article purpose I used online generator https://extendsclass.com/json-patch.html.

Since PATCH does not touch unchanged data we are good to perform our two updates at the same time. The request body to update the city is:

[
 {
  "op": "replace",
  "path": "/city",
  "value": "Warsaw"
 }
]

And promoting to premium account:

[
 {
  "op": "replace",
  "path": "/isPremiumMember",
  "value": true
 }
]

As you can see it is maybe not so simple as PUT method but it provides better user experience - there is no need of refresh before updating if the data has been updated in the meantime.

A summary:

HTTP PUT performs update of all fields, not only on changed ones. That may lead to case that the newly updated field is overwritten with the old value,
To mitigate the issue you can apply versioning of the data. If user tries to update newer data with older data, an error is returned,
To resolve problem you can use PATCH instead of PUT. It is a bit more complicated and requires more code. It is worth doing that if we can have multiple concurrent updates on our data.

Thank you for reading to the end! I hope that my tips will help you to create more reliable code.

Github repository is available here (a postman collection with all http requests is attached to the repo):

piotrnieweglowski / the-put-problem

The problem with HTTP PUT method and possible ways to resolve it

Demo code for article on dev.to website: https://dev.to/pnieweglowski/the-problem-with-http-put-method-and-possible-ways-to-resolve-it-3p5h

Repository contains code used in the blog post as well as postman collection with request to fetch / modify data.

View on GitHub

The benefits of Advent of Code 2020

Piotr Nieweglowski — Sun, 06 Dec 2020 14:05:38 +0000

Introduction

In the article I'm going to describe what is 'Advent of code', what is my view on this. At the end of this short post I'll share other interesting coding challenges and AoC relates links which are worth taking a look.

What is Advent of code?

Advent of code is a coding challenge with concept similar to advent calendar. Instead of taking a chocolate every single day for 24 days (starting on 1st of December), we have a coding puzzle to resolve on the site. After resolving the exercise we got a star and a link to the part 2 of the exercise (another star to get). The complexity of exercises is between easy and medium (the best results on leaderboard are between 1 and 2 minutes for both parts of the task what is insane!)

What benefits can give you participation in Advent of Code?

The idea behind such coding activities like Advent of Code gets inspiration from kata in terms of combat sports. Let's have a look at the definition of kata from wikipedia:

kata is a Japanese word (型 or 形) meaning literally "form" referring to a detailed choreographed pattern of martial arts movements made to be practised alone, and can also be reviewed within groups and in unison when training. It is practised in Japanese martial arts as a way to memorize and perfect the movements being executed.

Coding kata is the coding activity a bit more different than just creating a production code of the application. The programmer who practices kata is very similar to a sportsman doing his training. If you think a bit more about this, athlete doesn't finish his job just after playing a match. He practices on daily basis. As software engineers we can't afford to perform full training after our working hours because it would take too much of our time. We have families, hobbies and many other important things to do so a full training is not an option. But we can perform smaller training units just to practice not very common daily tasks like bit operations, creating simple search algorithms from scratch or building our own data structures. And that's the coding kata! Same as in sport word, there is a big difference between developers who do training activities and the developers who don't care about that.

What are other advantages of Advent of Code

You can try to code using language which is not you first choice (like GoLang in my case, I use C# at work),
You can share your solution and discuss that with your coding friends,
There is a good reason to create your first github repo if you haven't created it so far

A summary:

Advent of Code is a site with daily coding puzzles,
Solving small programming tasks on daily basis make you better developer,
It's not too late to start resolving AoC 2020. It's a good opportunity to create a github repo and share it with dev community!

Thank you for reading to the end! I hope that my tips will help you to create more reliable code.

Why using DateTime.Now directly is a bad idea

Piotr Nieweglowski — Sun, 29 Nov 2020 11:08:10 +0000

Introduction

In this post I'm explaining why using DateTime.Now directly in the code is not the best idea and what we can do in order to ensure testability of dates / time in the code.

Our code is often dependent on time

That is the reality - I worked in many projects and in every single project we used dates / time to implement some logic. It is hard to imagine a business domain free of date / time dependency.

On the other hand I've seen a lot of bad practices regarding to using dates / time in the code. Let's focus now on the most obvious code smell in my opinion. That thing is using DateTime.Now static property directly in the code. Let's get our hands dirty and write some code:

public class Greeting
{
    public string SayHello()
    {
        var hour = DateTime.Now.Hour;
        if (hour < 12)
            return "Good Morning!";
        if (hour < 18)
            return "Good Afternoon!";
        return "Good Evening";
    }
}

The logic in our class is very simple. The method SayHello returns appropriate greeting depending on the current time. At the moment when I'm writing this post, it's 10:40 am in Poland so I'm expecting that the method is going to return a string "Good Morning!". I don't want to rely on my guesses, I'm going to create an unit test and cover the logic with a test.

[Fact]
public void SayHello_Test()
{
    // Arrange
    var greeting = new Greeting();
    // Act
    var result = greeting.SayHello();
    // Assert
    Assert.Equal("Good Morning!", result);
}

After running the test I got confirmation - my method works as expected. So far so good! Unfortunately there is one big issue in our code which leads to a serious problem in the test. Our test is not repeatable. It works now but if I wait for two hours, the test will be red. It will expect 'Good Morning!' to be returned but the result will be 'Good Afternoon!'. We've got that issue because we don't have control over date and time in our code. Let's fix that. What can we do to take control over date and time? We have many ways to resolve the problem:

Create a fake assembly using Microsoft Fakes
Wrap DateTime.Now with custom class and expose Now using Func delegate
Wrap DateTime.Now with an interface and inject implementation during object creation.

Let's describe shortly all three approaches.
(1) has one only big disadvantage for me. Microsoft Fakes is available only with Visual Studio Ultimate. That was always the biggest issue for me, so I haven't used that approach in my solutions.

However there are some open source solutions and maybe it is worth taking a look at them:
Pose, Prig

(2) This is the simplest way to take control over the time dependency I must say. The only one thing you need to have testable solution is creating a service class with the implementation:

public static class SystemTime
{
    public static Func<DateTime> Now = () => DateTime.Now;
}

And use that in your code instead of using DateTime.Now directly. For testing purposes you can set whatever fits your needs by calling:

SystemTime.Now = () => new DateTime(2020, 11, 11);

It's so simple, isn't it? My view on this is - that's almost perfect solution. Why almost perfect? I can imagine that a developer in production code sets some date which is not the current date not intentionally and it'll be hard to find. After such change the code is buggy and all tests are green! I don't like it but I understand that someone can prefer this solution because of simplicity.

Number (3) is a best choice for me. It requires a bit more code but it's worth doing that in my opinion. Let's introduce additional interface as that was mentioned.

public interface ISystemTime
{
    DateTime Now { get; }
}

and the implementing class:

public class SystemTime : ISystemTime
{
    public DateTime Now => DateTime.Now;
}

We need to modify our Greeting class:

public class Greeting
{
    private ISystemTime _time;
    public Greeting(ISystemTime time)
    {
        _time = time;
    }
    public string SayHello()
    {
        var hour = _time.Now.Hour;
        if (hour < 12)
            return "Good Morning!";
        if (hour < 18)
            return "Good Afternoon!";
        return "Good Evening";
    }
}

As you can see it doesn't use DateTime.Now directly, we have a proxy class and keep a reference to it using an interface. It is the critical point from unit testing perspective. It gives us the ability of replacing the implementation for testing purposes.

Let's continue our work and update the test to get rid of unrepeatable behaviour.

First step is to create our FakeSystemTime class for testing purposes:

public class FakeSystemTime : ISystemTime
{
    private DateTime _now;
    public FakeSystemTime(DateTime now)
    {
        _now = now;
    }
    public DateTime Now => _now;
}

And update the test:

[Fact]
public void SayHello_Test()
{
    // Arrange
    ISystemTime time = new FakeSystemTime(new DateTime(2020, 11, 11, 10, 00, 00));
    var greeting = new Greeting(time);
    // Act
    var result = greeting.SayHello();
    // Assert
    Assert.Equal("Good Morning!", result);
}

Now I can ensure you that this test is repeatable and it will be green even if you run it at 9:00 pm. What is more, we don't have the ability of setting custom value in our production code, that was the biggest disadvantage for me in approach (2). Happy days!

What we could do more?

We could add tests to cover two other scenarios (Good Afternoon, Good Evening) or use xUnit Theory in order to parametrise the test.
We could configure IoC containers for our production code to use SystemTime class and use FakeSystemTime for tests instead of creating these objects manually. I skipped that part for the sake of simplicity,
We could use mocking framework like NSubstitute or Moq instead of creating FakeSystemTime manually.

A summary:

We shouldn't use external dependencies like date / time directly in our code. It makes testing harder and sometimes even impossible,
There are many ways to resolve the issue of external dependencies in the code from testing point of view,
Using the interface should give us more control over the external dependency, however we'll need to write some additional code.

Thank you for reading to the end! I hope that my tips will help you to create more reliable code.

Don't make setters of properties in your class public!

Piotr Nieweglowski — Sun, 22 Nov 2020 21:04:28 +0000

Introduction

In this post I'm explaining why public setters of properties can introduce bugs to our code and how to ensure better control over the data integrity in C# class.

OK, so what is wrong with public setters?

In my entire live I have seen a lot of classes similar to this one:

public class Car
{
    public string Color { get; set; }
    public string Brand { get; set; }
    public List<Wheel> Wheels { get; set; }

    public Car(string color, string brand)
    {
        Color = color;
        Brand = brand;
        Wheels = CreateFourWheels();
    }
}

At first glance you may think that everything is fine with such class. I'll try to explain what in my opinion is broken - that thing is public setters. With such constructed class, nothing stops a client of that class from doing such operation:

var car = new Car("White", "BMW");
// some logic here
car.Brand = "Toyota";

What happened here? Someone did operation which was not allowed. A client of our class performed a change which makes no sens regarding to the business domain. A brand of the car cannot be changed during the whole life of the car. It was possible because of the public setter.
Conclusion numer 1: in most cases with public setter we lose the control of the data integrity of our model.
We can fix that by changing setter's accessibility to private. After such change a client of our class is not able to change the value of the property, it will end with a compilation error.

OK, we have fixed the setter accessibility. Is everything fine right now and are we done? Not really. I'd like to emphasise two things more which are critical from the data integrity point of view. One of these two things can be quite tricky and not obvious.

Let's assume that if a Ford car is being constructed it can have only black color. Any other color is not allowed. Let's change our constructor to this:

public Car(string color, string brand)
{
  if (brand == "Ford" && color != "Black")
  {
    throw new Exception("Ford is allowed only in black color");
  }

  Color = color;
  Brand = brand;
  Wheels = CreateFourWheels();
}

We added a simple check and if the condition is not met, an exception is thrown. It is not the ideal solution but I did it to point out that every change of the data should be validated. If we had the method which makes repainting possible, we'd add a similar check to the method as well.

public void Repaint(string color)
{
  if (Brand == "Ford" && color != "Black")
  {
    throw new Exception("Ford is allowed only in black color");
  }

  Color = color;
}

The example of validation is simplified in the code above, I'm planning to create another post and cover this topic so I don't want to focus on this topic too much in this article.

Let's take a look at our class once again:

public class Car
{
   public string Color { get; private set; }
   public string Brand { get; private set; }
   public List<Wheel> Wheels { get; private set; }

   public Car(string color, string brand)
   {
       // the content of constructor
   }
}

Let's focus on Wheels property. It doesn't have a public setter. Is everything set properly then? Not really, there is one small gotcha. It is still possible to change the content of the collection, a client of our class can perform operations like these:

var car = new Car("White", "BMW");
car.Wheels.Add(new Wheel()); // (a)
car.Wheels.Clear();          // (b)

In the line (a) we added new additional wheel despite the fact that our wheel collection does not have public setter but a private one. If our logic assumes that every car has 4 wheels, operation of adding 5th wheel should not be allowed. In the line (b) we clear the collection and it becomes an empty collection. That should be forbidden too.

Why it is possible you may ask? It is possible because changing the setter of collection to a private one blocks only assigning a new value to the collection. The property is accessible and client code can invoke methods on it.

car.Wheels = new List<Wheel>(); // compilation error
car.Wheels.Clear();             // still possible

How we can improve that? We have two options:

public class Car
{
  private List<Wheel> _wheels;
  public string Color { get; private set; }
  public string Brand { get; private set; }
  public List<Wheel> Wheels { get { return _wheels.ToList(); } }

  public Car(string color, string brand)
  {
     _wheels = CreateFourWheels();
  }
}

In first option we changed the logic behind Wheels property. We use private field _wheels internally and client of the class can reach only a copy of the collection (by using .ToList() method). Even if the copy is changed, the original field remains the same.

public class Car
{
  private List<Wheel> _wheels;
  public string Color { get; private set; }
  public string Brand { get; private set; }
  public ReadOnlyCollection<Wheel> Wheels
  { 
    get { return new ReadOnlyCollection<Wheel>(_wheels); } 
  }

  public Car(string color, string brand)
  {
     _wheels = CreateFourWheels();
  }
}

The second way of providing the data integrity is using ReadOnlyCollection type. After such change client code is not able to call any method which changes the content of the collection.

A summary:

In most cases with public setter we lose the control of the data integrity in our model. We can resolve that by using private setters,
Every change of the data should be validated in constructor or in the method / a setter,
Collections need special treatment. We should expose a copy of the collection calling .ToList() method or use ReadOnlyCollection.

Thank you for reading to the end! I hope that my tips will help you to create more reliable code.

DEV Community: Piotr Nieweglowski

The problem with HTTP PUT method and possible ways to resolve it

Introduction

How should we handle updating of the data in API?

Versioning

PATCH instead of PUT

A summary:

piotrnieweglowski / the-put-problem

The problem with HTTP PUT method and possible ways to resolve it

The benefits of Advent of Code 2020

Introduction

What is Advent of code?

What benefits can give you participation in Advent of Code?

What are other advantages of Advent of Code

Other coding challenges and AoC related links

A summary:

Why using DateTime.Now directly is a bad idea

Introduction

Our code is often dependent on time

A summary:

Don't make setters of properties in your class public!

Introduction

OK, so what is wrong with public setters?

A summary: