Spring Data REST
  1. Spring Data REST
  2. DATAREST-117

@JsonIgnore and other Jackson annotations are ignored

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.0 GA (Codd)
    • Component/s: None
    • Labels:
      None

      Description

      Github Author: speedyg
      Github Last-Updated: 2013-08-21T10:01:31Z
      This issue was automatically imported from github

      In my application and in the example application the @JsonIgnore annotation is ignored.

      Reproduce:
      Add an @JsonIgnore annotation to the field status in Order.java. The status field should not be serialized.
      Call http://localhost:8080/restbucks/orders/1

      The status property is (still) exported.

      Also tried @JsonIgnoreProperties and @XmlTransient, no effect.

      Additionally, i am not able to find another way to exclude a property.

        Activity

        Hide
        Oliver Gierke added a comment -

        No worries, Nick. I just wanted to let you now that we addressed your concerns .

        Show
        Oliver Gierke added a comment - No worries, Nick. I just wanted to let you now that we addressed your concerns .
        Hide
        Nick Weedon added a comment -

        I had a look at the changes and ran them against my unit tests from pull request 133. I really like the way that you implemented this fix, I think it is much more elegant than my fix. I didn't know about the (de)serialization modifiers, that is a much better way to do it.

        I had a NPE exception when i ran my test at first and was about to upload a pull request with a suggested fix but then when I rebased my changes I saw that you already fixed it, just an hour after I fetched from the upstream

        I did notice however that you only applied the PersistentProperty null check to serialization. My unit tests indicated that this needs to be added to deserialization also, since the PersistentPropery will be null during serialization if @JsonProperty is used (i.e. if a field 'alias' is being supplied) since the name of the JSON field won't match the name the actual class property name.

        One other rather obvious thing to note is that there would of course need to be more work done to cover the scenario where the @JsonProperty annotation is applied to an association property or an ID property. I would hope that this is more of an edge case but it is of course still a possible scenario.

        I was thinking about how to solve this and I thought that one possible solution might be to build an 'alias' map during construction of the (de)serialization modifiers by scanning the domain classes for @JsonProperty annotations. This map could then be checked at run time in the case that a PersistentProperty cannot be found.

        It also briefly occurred to me that a these fields could be added to the PersistentEntity property cache in the org.springframework.data.mapping.context.AbstractMappingContext class. I quickly discarded this shit idea however since I think it would be far too invasive since this class lives in spring data commons. The other bad thing about this shit idea is that this would mean that the class would then have knowledge of Jackson annotations, introducing tight coupling.

        You probably have a better idea but I thought I would offer up my suggestions anyhow.

        For what it's worth I went ahead and created pull request #136 since I thought that the unit tests in this pull request might still be useful to help with reproducing the issue that occurs during deserialization (related to the use of the @JsonProperty annotation).

        Show
        Nick Weedon added a comment - I had a look at the changes and ran them against my unit tests from pull request 133. I really like the way that you implemented this fix, I think it is much more elegant than my fix. I didn't know about the (de)serialization modifiers, that is a much better way to do it. I had a NPE exception when i ran my test at first and was about to upload a pull request with a suggested fix but then when I rebased my changes I saw that you already fixed it, just an hour after I fetched from the upstream I did notice however that you only applied the PersistentProperty null check to serialization. My unit tests indicated that this needs to be added to deserialization also, since the PersistentPropery will be null during serialization if @JsonProperty is used (i.e. if a field 'alias' is being supplied) since the name of the JSON field won't match the name the actual class property name. One other rather obvious thing to note is that there would of course need to be more work done to cover the scenario where the @JsonProperty annotation is applied to an association property or an ID property. I would hope that this is more of an edge case but it is of course still a possible scenario. I was thinking about how to solve this and I thought that one possible solution might be to build an 'alias' map during construction of the (de)serialization modifiers by scanning the domain classes for @JsonProperty annotations. This map could then be checked at run time in the case that a PersistentProperty cannot be found. It also briefly occurred to me that a these fields could be added to the PersistentEntity property cache in the org.springframework.data.mapping.context.AbstractMappingContext class. I quickly discarded this shit idea however since I think it would be far too invasive since this class lives in spring data commons. The other bad thing about this shit idea is that this would mean that the class would then have knowledge of Jackson annotations, introducing tight coupling. You probably have a better idea but I thought I would offer up my suggestions anyhow. For what it's worth I went ahead and created pull request #136 since I thought that the unit tests in this pull request might still be useful to help with reproducing the issue that occurs during deserialization (related to the use of the @JsonProperty annotation).
        Hide
        Nick Weedon added a comment -

        By the way, I just wanted to say thanks for readdressing this issue. I have been battling with a rather inflexible REST client and these changes have allowed me to properly integrate with SDR. It saddened me to think that I might have to abandon SDR and go back to manually marshaling JSON data. Thanks again, really appreciate it

        Show
        Nick Weedon added a comment - By the way, I just wanted to say thanks for readdressing this issue. I have been battling with a rather inflexible REST client and these changes have allowed me to properly integrate with SDR. It saddened me to think that I might have to abandon SDR and go back to manually marshaling JSON data. Thanks again, really appreciate it
        Hide
        Greg Turnquist added a comment -

        @Nick I'm glad we put SDR back onto a solid foundation and it's serving your needs.

        If you have test cases that expose problems, feel free to post them possibly as a pull request on Github. You can flag stuff as "DON'T MERGE" at the top, and then provide more details. We can chat back and forth, and if it pans out as a real bug, we can then fashion a real JIRA issue and work towards fixing stuff. This let's us use github's "review" model handily.

        In general, we need test cases to expose these problems, and as you've seen recently, this project is moving heavily towards a test-then-fix direction, compared to where it was perhaps a year ago. Narrow, focused test cases help us knock out issues and your involvement is great at moving things along.

        As a side note, I'm working on the first of what may be many getting started guides (http://spring.io/guides) for SDR. I hope we can lower the bar and make it easier for others to start using it as well.

        Show
        Greg Turnquist added a comment - @Nick I'm glad we put SDR back onto a solid foundation and it's serving your needs. If you have test cases that expose problems, feel free to post them possibly as a pull request on Github. You can flag stuff as "DON'T MERGE" at the top, and then provide more details. We can chat back and forth, and if it pans out as a real bug, we can then fashion a real JIRA issue and work towards fixing stuff. This let's us use github's "review" model handily. In general, we need test cases to expose these problems, and as you've seen recently, this project is moving heavily towards a test-then-fix direction, compared to where it was perhaps a year ago. Narrow, focused test cases help us knock out issues and your involvement is great at moving things along. As a side note, I'm working on the first of what may be many getting started guides ( http://spring.io/guides ) for SDR. I hope we can lower the bar and make it easier for others to start using it as well.
        Hide
        Nick Weedon added a comment -

        @Greg I just flagged my #136 pull request with "DON'T MERGE" but i'm not sure if I have done this correctly. I simply suffixed the title with "DON'T MERGE", is this what you mean? I couldn't seem to find any other way to do this. This pull request includes the unit tests that reproduce the problem by the way.

        I like the test driven approach. It seems to work particularly well with developer to developer interaction. On that note, this has been an interesting experience for me since my usual means of collaboration include work meetings or chatting to someone over a cubicle divider

        Thanks for letting me know about the getting started guides too. I'll definitely take a look some time.

        If you like, I'd be happy to contribute to the guide in the area of integrating spring security since I found that I had to do a fair bit of web searching and general tinkering in this area to get this to work with my web application. There is still one area of this subject that I still need to find a solution to however (relates to discussion in DATAREST-236, Sri's problem with the findOne method).

        Show
        Nick Weedon added a comment - @Greg I just flagged my #136 pull request with "DON'T MERGE" but i'm not sure if I have done this correctly. I simply suffixed the title with "DON'T MERGE", is this what you mean? I couldn't seem to find any other way to do this. This pull request includes the unit tests that reproduce the problem by the way. I like the test driven approach. It seems to work particularly well with developer to developer interaction. On that note, this has been an interesting experience for me since my usual means of collaboration include work meetings or chatting to someone over a cubicle divider Thanks for letting me know about the getting started guides too. I'll definitely take a look some time. If you like, I'd be happy to contribute to the guide in the area of integrating spring security since I found that I had to do a fair bit of web searching and general tinkering in this area to get this to work with my web application. There is still one area of this subject that I still need to find a solution to however (relates to discussion in DATAREST-236 , Sri's problem with the findOne method).

          People

          • Assignee:
            Oliver Gierke
            Reporter:
            Thomas Darimont
          • Votes:
            2 Vote for this issue
            Watchers:
            7 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Agile