spring-projects / spring-data-rest

Simplifies building hypermedia-driven REST web services on top of Spring Data repositories
https://spring.io/projects/spring-data-rest
Apache License 2.0
916 stars 562 forks source link

PUT creates new DynamoDB entity when hash/range key are specified in body [DATAREST-1025] #1387

Open spring-projects-issues opened 7 years ago

spring-projects-issues commented 7 years ago

Ryon Day opened DATAREST-1025 and commented

I use Spring-Data-DynamoDB and have been updating dependencies.

I have an org.springframework.core.convert.converter.Converter registered to be able to reference my DynamoDB entities from Spring-Data-Rest using their hash/range keys, so: http://service/foo/hash-range will get the DynamoDB entity with hash and range keys hash and range. This integrates well with Spring-data-rest and the AWS API which uses composite key objects.

@Component
public class StringToFooIdConverter implements Converter<String, FooId> {
    // Conversion
}

Let's say that bar and baz fields are the hash and range key properties for the Foo object.

This means that we create a new entity by

{
    "bar": "qux", 
    "baz": "garple", 
    "some_other_thing": "quux" 
}

And then we get the same entity with

{
    "bar": "qux", 
    "baz": "garple", 
    "some_other_thing": "quux" 
}

Cool! We also need to be able to PUT to replace. This is where the problem comes in.

Spring-Data-REST 2.4.4.RELEASE

Request

{
    "bar": "qux2", 
    "baz": "garple2", 
    "some_other_thing": "quux2" 
}

Response

{
    "bar": "qux", 
    "baz": "garple", 
    "some_other_thing": "quux2" 
}

Cool! It recognized that since this is a key/value store, you'd actually be changing the id, so it did not do it!

Request

{
    "some_other_thing": "quux3" 
}

Response

{
    "bar": "qux", 
    "baz": "garple", 
    "some_other_thing": "quux2" 
}

Cool! since the key was specified on the URL, it got it, updated the record, and returned the whole thing! Works like a champ.

Spring-Data-REST 2.6.1.RELEASE

Request

{
    "bar": "qux2", 
    "baz": "garple2", 
    "some_other_thing": "quux2" 
}

Response

{
    "bar": "qux2", 
    "baz": "garple2", 
    "some_other_thing": "quux2" 
}

Oh dear! It created a NEW record, and the old one still exists too! It did not respect that the attributes were keys!

Request

{
    "some_other_thing": "quux3" 
}

Response

{
  "error": "invalid_request",
  "error_description": "Message not readable"
}

Oh dear! It would seem that there is some weirdness here, because it tries to create a new object (or something) and cannot do so.

Desired State

This functionality should be the same as Spring-Data-Rest 2.4.4.RELEASE


Affects: 2.6.1 (Ingalls SR1), 2.5.8 (Hopper SR8)

spring-projects-issues commented 7 years ago

Ryon Day commented

I am working on a sample project to illustrate this issue

spring-projects-issues commented 7 years ago

Ryon Day commented

I have issued a sample project for this issue:

https://github.com/ryonday/DATAREST-1025

There are three separate tags in the repository: spring-boot-1.5.2 spring-boot-1.4.5 spring-boot-1.3.8

Each tag is built off that version of Spring Boot. 1.3.8 is the last "good" version that does not have this problem.

You will need a running DynamoDB-local (very easy) to exercise this issue.

I have also included a Postman collection that has all of the rest calls needed to demonstrate this issue.

This issue is actually a huge blocker towards upgrading our REST app to 1.5.2 from 1.3.8.

The root of this problem is that we "flatten" our DynamoDB composite key so that instead of something like this:

{
  id: {
    "hash": "foo",
    "range": "bar"
  }
  "fieldName": "value"
}

We do this:

{
  "hash": "foo",
  "range": bar",
  "fieldName": "value"
}

We accomplish this by annotating the getter for the key with ignore annotations and supplying setters and getters for the hash and range keys that access the key's setters and getters.

The problem seems to be that previous Spring-Data-Commons versions seem to have somehow "detected" that attributes were part of a key, or that the serialized key was different than the actual key of the item.

spring-projects-issues commented 7 years ago

Ryon Day commented

The cause seems to be some changes made to DomainObjectReader::readPut:

With Spring-Data-REST 2.4.4, the SimplePropertyHandler seems to treat @Id-annotated fields differently (DomainObjectReader:124) from source to destination (the original). It looks as though doMerge (DomainObjectReader:160) filters out the properties somehow.

spring-projects-issues commented 7 years ago

Oliver Drotbohm commented

Suspiciously sounds like DATAREST-1050. Would you mind giving the latest releases a try?

spring-projects-issues commented 7 years ago

Ryon Day commented

The description does sound very similar, but the mechanism of action here is a little different. I haven't looked at the code changes that happened in SDR versions included in the Spring-Boot starters between 1.3.8 and 1.5.2, but before, SDR somehow recognized that a primary key change shouldn't be written to the database.

I tested this with Spring-Boot 1.5.2.RELEASE using the Spring dependency management plugin to test with the newest SDR:

dependencyManagement {

    overriddenByDependencies = false

    dependencies {
        dependency 'org.springframework.data:spring-data-rest-webmvc:2.6.2.BUILD-SNAPSHOT'
        dependency 'org.springframework.data:spring-data-rest-core:2.6.2.BUILD-SNAPSHOT'
    }
}

And the problem still happens.

spring-projects-issues commented 7 years ago

Ryon Day commented

Looks like the SDR version included with the Spring-Boot 1.3.8 starter is 2.4.5. The use case works correctly on this version