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

ResourceProcessor not used for nested Projection [DATAREST-925] #1288

Closed
spring-projects-issues opened this issue Oct 19, 2016 · 9 comments
Closed
Assignees
Labels
Milestone

Comments

@spring-projects-issues
Copy link

@spring-projects-issues spring-projects-issues commented Oct 19, 2016

Marc Zampetti opened DATAREST-925 and commented

There seems to be an issue with how ResourceProcessors are being applied to a Projection in certain case. I've linked to a repo that demonstrates the issue.

There are two entities, Person and Address. The Person is the parent in a @ManyToOne relationship to the Address. There is also a PersonProjection that shows a subset of the Person, most importantly it does not include the Set<Address> of the child Address entities. There is also a AddressProjection that includes thePersonProjection of the parent. The goal is to be able to retrieve the list of Address objects and their associated key Person information in a single call.

Now, I also have ResourceProcessor implementations that add some links to each of the entities. That includes ResourceProcessor implementations on the projections. Everything works as described and expected when using the normal SDR endpoints to get the Person and Address collections. The custom links are correctly added to each entity. Also, when I query for the Person collection with the projection, that too adds the appropriate link to the Person representation. However, when I query the Address collection with the projection, the embedded PersonProjection has the normal SDR-generated links for self and the object itself person in this case, it does not have the custom links.

The listing of the Person collection. Notice the google link, which is the custom link for this demonstration:

curl http://localhost:8080/persons; echo
{
  "_embedded" : {
    "persons" : [ {
      "firstName" : "Bob",
      "lastName" : "Bobby",
      "quote" : "Likes to Bob",
      "_links" : {
        "self" : {
          "href" : "http://localhost:8080/persons/1"
        },
        "person" : {
          "href" : "http://localhost:8080/persons/1{?projection}",
          "templated" : true
        },
        "google" : {
          "href" : "http://www.google.com"
        },
        "addresses" : {
          "href" : "http://localhost:8080/persons/1/addresses"
        }
      }
    } ]
  },
  "_links" : {
    "self" : {
      "href" : "http://localhost:8080/persons"
    },
    "profile" : {
      "href" : "http://localhost:8080/profile/persons"
    }
  },
  "page" : {
    "size" : 20,
    "totalElements" : 1,
    "totalPages" : 1,
    "number" : 0
  }
}

Now the listing of the Person collection with the projection enabled. Notice that the quote field is not returned, but the google link is still there.

curl http://localhost:8080/persons?projection=simple; echo
{
  "_embedded" : {
    "persons" : [ {
      "firstName" : "Bob",
      "lastName" : "Bobby",
      "_links" : {
        "self" : {
          "href" : "http://localhost:8080/persons/1"
        },
        "person" : {
          "href" : "http://localhost:8080/persons/1{?projection}",
          "templated" : true
        },
        "google" : {
          "href" : "http://www.google.com"
        },
        "addresses" : {
          "href" : "http://localhost:8080/persons/1/addresses"
        }
      }
    } ]
  },
  "_links" : {
    "self" : {
      "href" : "http://localhost:8080/persons"
    },
    "profile" : {
      "href" : "http://localhost:8080/profile/persons"
    }
  },
  "page" : {
    "size" : 20,
    "totalElements" : 1,
    "totalPages" : 1,
    "number" : 0
  }
}
{

The listing of the Address collection. Notice the microsoft link which is the custom link for this demonstration:

curl http://localhost:8080/addresses; echo
{
  "_embedded" : {
    "addresses" : [ {
      "line1" : "1234 Line 1 Street",
      "line2" : "Apt Line 2",
      "city" : "Somewhere",
      "state" : "NY",
      "country" : "USA",
      "zipcode" : "12345",
      "_links" : {
        "self" : {
          "href" : "http://localhost:8080/addresses/1"
        },
        "address" : {
          "href" : "http://localhost:8080/addresses/1{?projection}",
          "templated" : true
        },
        "microsoft" : {
          "href" : "http://www.microsoft.com"
        },
        "person" : {
          "href" : "http://localhost:8080/addresses/1/person"
        }
      }
    } ]
  },
  "_links" : {
    "self" : {
      "href" : "http://localhost:8080/addresses"
    },
    "profile" : {
      "href" : "http://localhost:8080/profile/addresses"
    }
  },
  "page" : {
    "size" : 20,
    "totalElements" : 1,
    "totalPages" : 1,
    "number" : 0
  }
}

And finally the listing of the Address projection, without the expected google link for the embedded Person. However, the Person is missing the quote field, so it is processing the rest of the projection.

curl http://localhost:8080/addresses?projection=simple; echo
{
  "_embedded" : {
    "addresses" : [ {
      "state" : "NY",
      "line" : "1234 Line 1 Street Apt Line 2",
      "city" : "Somewhere",
      "person" : {
        "firstName" : "Bob",
        "lastName" : "Bobby",
        "_links" : {
          "self" : {
            "href" : "http://localhost:8080/persons/1{?projection}",
            "templated" : true
          },
          "addresses" : {
            "href" : "http://localhost:8080/persons/1/addresses"
          }
        }
      },
      "zipcode" : "12345",
      "_links" : {
        "self" : {
          "href" : "http://localhost:8080/addresses/1"
        },
        "address" : {
          "href" : "http://localhost:8080/addresses/1{?projection}",
          "templated" : true
        },
        "microsoft" : {
          "href" : "http://www.microsoft.com"
        },
        "person" : {
          "href" : "http://localhost:8080/addresses/1/person"
        }
      }
    } ]
  },
  "_links" : {
    "self" : {
      "href" : "http://localhost:8080/addresses"
    },
    "profile" : {
      "href" : "http://localhost:8080/profile/addresses"
    }
  },
  "page" : {
    "size" : 20,
    "totalElements" : 1,
    "totalPages" : 1,
    "number" : 0
  }
}

It is not clear why the additional links are not being added. For all I can tell, it should be. There are no errors or warnings or any other indication of an issue that I can see.


Affects: 2.5.4 (Hopper SR4)

Reference URL: https://github.com/zampettim/datarest925

Referenced from: pull request #238

Backported to: 2.6 RC1 (Ingalls)

2 votes, 4 watchers

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Oct 20, 2016

Marc Zampetti commented

Updated with a more clear description and linked to the demonstration repo

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Nov 3, 2016

Oliver Drotbohm commented

What I am kind of wondering is: is the fact that you have a ResourceProcessor for both the entity and the projection instance an attempt to solve the problem or really intended? I can imagine people argue three things: they want the ResourceProcessor instances invoked for the projection target and have the links potentially generated added to the projection (i.e. links created for an entity are always added, no matter if a projection is applied or not). Or they could argue they only want to get ResourceProcessor instances invoked that are explicitly typed to the projection (i.e. projections always get a dedicated treatment and don't "inherit" links produced by the entity specific processor). A third option could be that it's intended that both are combined, which then raises the question of which of the two is called first

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Nov 3, 2016

Craig commented

I believe the best solution is to get ResourceProcessor instances invoked that are explicitly typed to the projection.

If someone wants a ResourceProcessor to apply to both the entity and the projection, they can create an interface that both the entity and the projection extend and create a ResourceProcessor for that

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Nov 3, 2016

Craig commented

Here's a pull request - I'd love some review (am I heading in the right direction?).

#238

Thanks!

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Nov 3, 2016

Craig commented

I ran the test case https://github.com/zampettim/datarest925 against my pull request at #238 by adding this to the pom.xml of test case:

<dependency>
        <groupId>org.springframework.data</groupId>
        <artifactId>spring-data-rest-webmvc</artifactId>
        <version>2.5.5.BUILD-SNAPSHOT</version>
</dependency>
<dependency>
        <groupId>org.springframework.data</groupId>
        <artifactId>spring-data-rest-core</artifactId>
        <version>2.5.5.BUILD-SNAPSHOT</version>
</dependency>
<dependency>
        <groupId>org.springframework.data</groupId>
        <artifactId>spring-data-rest-hal-browser</artifactId>
        <version>2.5.5.BUILD-SNAPSHOT</version>
</dependency>

Here's the last curl execution's output from the issue's description:

curl http://localhost:8080/addresses?projection=simple; echo
{
   "_embedded":{
      "addresses":[
         {
            "state":"NY",
            "line":"1234 Line 1 Street Apt Line 2",
            "city":"Somewhere",
            "person":{
               "firstName":"Bob",
               "lastName":"Bobby",
               "_links":{
                  "self":{
                     "href":"http://localhost:8080/persons/1{?projection}",
                     "templated":true
                  },
                  "addresses":{
                     "href":"http://localhost:8080/persons/1/addresses"
                  },
                  "google":{
                     "href":"http://www.google.com"
                  }
               }
            },
            "zipcode":"12345",
            "_links":{
               "self":{
                  "href":"http://localhost:8080/addresses/1"
               },
               "address":{
                  "href":"http://localhost:8080/addresses/1{?projection}",
                  "templated":true
               },
               "microsoft":{
                  "href":"http://www.microsoft.com"
               },
               "person":{
                  "href":"http://localhost:8080/addresses/1/person"
               }
            }
         }
      ]
   },
   "_links":{
      "self":{
         "href":"http://localhost:8080/addresses"
      },
      "profile":{
         "href":"http://localhost:8080/profile/addresses"
      }
   },
   "page":{
      "size":20,
      "totalElements":1,
      "totalPages":1,
      "number":0
   }
}

Notice that the embedded Person has the "google" links as it should

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Nov 3, 2016

Oliver Drotbohm commented

That's fine. We still need an integration test for our codebase

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Nov 4, 2016

Johannes Hiemer commented

This is something I asked Oliver a couple of weeks ago as well. Would be nice to see this feature in the implementation!

@spring-projects-issues
Copy link
Author

@spring-projects-issues spring-projects-issues commented Dec 2, 2016

Oliver Drotbohm commented

That's in place now. I've merged Craig's PR so that ResourceProcessor implementations bound to the projection type should be invoked now

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

No branches or pull requests

2 participants