rave-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Chris Geer" <ch...@cxtsoftware.com>
Subject Re: Review Request 12076: People REST Service using JSON Views and Jackson 2.1
Date Tue, 25 Jun 2013 15:57:12 GMT


> On June 25, 2013, 2:16 p.m., Matt Franklin wrote:
> > Most things should be fine.  I think we probably should have done the upgrade of
Jackson then done the view stuff; but that is not a big deal.
> > 
> > The big issue I have is that it is pretty inflexible.  Better than nothing, but
would really be nice if it was configurable.

I am going to try and split out the jackson upgrade stuff and just commit it as it's about
time. I agree this isn't as flexible as I'd like but it is better than what we have now.


> On June 25, 2013, 2:16 p.m., Matt Franklin wrote:
> > /trunk/rave-components/rave-core-api/src/main/java/org/apache/rave/rest/PeopleResource.java,
line 34
> > <https://reviews.apache.org/r/12076/diff/1/?file=310728#file310728line34>
> >
> >     Using a class for the view seems like it could get clumsy.  
> >     
> >     It would be really nice to be able to dynamically restrict the response to an
arbitrary set of fields.
> >     
> >     Could you do this with @JsonFilter?

Yes we could but it would require a lot more work on every web service call. Maybe it's the
right approach but I'm not sure. Worth discussing though. I think the JsonView is going to
be fine in 99% of the cases but won't solve the issue if there are fields that can only be
seen based on permissions (i.e. admins). We could solve that by moving to JsonFilter but it
will get really complicated with nested objects. The other alternative is to have a different
web method for each "view" of the data. We could differentiate via accept-type mapping so
admins could request "application/json+admin" or something like that.

No perfect solution here. What do you think?


> On June 25, 2013, 2:16 p.m., Matt Franklin wrote:
> > /trunk/rave-portal-resources/src/main/webapp/WEB-INF/applicationContext-security.xml,
line 64
> > <https://reviews.apache.org/r/12076/diff/1/?file=310770#file310770line64>
> >
> >     Is this just for the test?

Yes, this is to ensure we can run the integration tests.


- Chris


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/12076/#review22354
-----------------------------------------------------------


On June 25, 2013, 6:36 a.m., Chris Geer wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/12076/
> -----------------------------------------------------------
> 
> (Updated June 25, 2013, 6:36 a.m.)
> 
> 
> Review request for rave.
> 
> 
> Bugs: RAVE-978
>     https://issues.apache.org/jira/browse/RAVE-978
> 
> 
> Repository: rave
> 
> 
> Description
> -------
> 
> I've upgraded Rave to use Jackson 2.1 and implemented the people service to use the Jackson
JSONView technology that allows us to return selective data. This will allow us to return
subsets of data when appropriate. Looking for a high level review to see if we want to progress
with this approach.
> 
> Can be tested with the following two URLs. They return the same Person object, just with
different subsets of data.
> 
> http://localhost:8080/portal/api/rest/people/
> http://localhost:8080/portal/api/rest/people/1
> 
> Standardizing on JSON will allow us to accept partial updates which would be very beneficial
on complex objects.
> 
> I probably should have isolated the Jackson upgrade but it's pretty entwined by now.
I've also added some integration tests to test the new CXF web services. You can check out
the basics by heading into rave-integration-tests/rave-webservice-tests and running "mvn -Pintegration-tests"
> 
> 
> Diffs
> -----
> 
>   /trunk/pom.xml 1496346 
>   /trunk/rave-components/rave-commons/pom.xml 1496346 
>   /trunk/rave-components/rave-commons/src/main/java/org/apache/rave/util/JsonUtils.java
1496346 
>   /trunk/rave-components/rave-core-api/pom.xml 1496346 
>   /trunk/rave-components/rave-core-api/src/main/java/org/apache/rave/model/RegionWidget.java
1496346 
>   /trunk/rave-components/rave-core-api/src/main/java/org/apache/rave/rest/JSONViews.java
PRE-CREATION 
>   /trunk/rave-components/rave-core-api/src/main/java/org/apache/rave/rest/PeopleResource.java
1496346 
>   /trunk/rave-components/rave-core-api/src/main/java/org/apache/rave/rest/model/Person.java
1496346 
>   /trunk/rave-components/rave-core/pom.xml 1496346 
>   /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/model/impl/PageUserImpl.java
1496346 
>   /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/model/impl/RegionImpl.java
1496346 
>   /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/model/impl/RegionWidgetImpl.java
1496346 
>   /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/model/util/WidgetMarketplaceSearchResult.java
1496346 
>   /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/model/util/WidgetMarketplaceWidgetResult.java
1496346 
>   /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/PersonRepository.java
1496346 
>   /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/UserService.java
1496346 
>   /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/impl/DefaultUserService.java
1496346 
>   /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/service/impl/DefaultWidgetMarketplaceService.java
1496346 
>   /trunk/rave-components/rave-core/src/main/java/org/apache/rave/rest/impl/DefaultPeopleResource.java
1496346 
>   /trunk/rave-components/rave-core/src/test/java/org/apache/rave/portal/service/impl/DefaultUserServiceTest.java
1496346 
>   /trunk/rave-components/rave-jpa/pom.xml 1496346 
>   /trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/model/JpaCategory.java
1496346 
>   /trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/model/JpaPage.java
1496346 
>   /trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/model/JpaPageUser.java
1496346 
>   /trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/model/JpaPerson.java
1496346 
>   /trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/model/JpaRegion.java
1496346 
>   /trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/model/JpaRegionWidget.java
1496346 
>   /trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/repository/impl/JpaPersonRepository.java
1496346 
>   /trunk/rave-components/rave-jpa/src/test/java/org/apache/rave/portal/repository/impl/JpaPersonRepositoryTest.java
1496346 
>   /trunk/rave-components/rave-mongodb/pom.xml 1496346 
>   /trunk/rave-components/rave-mongodb/src/main/java/org/apache/rave/portal/model/MongoDbCategory.java
1496346 
>   /trunk/rave-components/rave-mongodb/src/main/java/org/apache/rave/portal/model/MongoDbGroup.java
1496346 
>   /trunk/rave-components/rave-mongodb/src/main/java/org/apache/rave/portal/model/MongoDbPage.java
1496346 
>   /trunk/rave-components/rave-mongodb/src/main/java/org/apache/rave/portal/model/MongoDbPageTemplate.java
1496346 
>   /trunk/rave-components/rave-mongodb/src/main/java/org/apache/rave/portal/model/MongoDbUser.java
1496346 
>   /trunk/rave-components/rave-mongodb/src/main/java/org/apache/rave/portal/model/MongoDbWidget.java
1496346 
>   /trunk/rave-components/rave-mongodb/src/main/java/org/apache/rave/portal/repository/impl/MongoDbPersonRepository.java
1496346 
>   /trunk/rave-components/rave-web/pom.xml 1496346 
>   /trunk/rave-components/rave-web/src/main/java/org/apache/rave/portal/web/model/MaterializedBeanObjectMapperFactory.java
1496346 
>   /trunk/rave-integration-tests/pom.xml 1496346 
>   /trunk/rave-integration-tests/rave-webservice-tests/pom.xml PRE-CREATION 
>   /trunk/rave-integration-tests/rave-webservice-tests/src/main/java/org/apache/rave/integrationtests/webservice/StateManager.java
PRE-CREATION 
>   /trunk/rave-integration-tests/rave-webservice-tests/src/main/java/org/apache/rave/integrationtests/webservice/Stories.java
PRE-CREATION 
>   /trunk/rave-integration-tests/rave-webservice-tests/src/main/java/org/apache/rave/integrationtests/webservice/steps/CommonSteps.java
PRE-CREATION 
>   /trunk/rave-integration-tests/rave-webservice-tests/src/main/java/org/apache/rave/integrationtests/webservice/steps/PeopleSteps.java
PRE-CREATION 
>   /trunk/rave-integration-tests/rave-webservice-tests/src/main/resources/org/apache/rave/integrationtests/webservice/people.story
PRE-CREATION 
>   /trunk/rave-portal-dependencies/pom.xml 1496346 
>   /trunk/rave-portal-resources/pom.xml 1496346 
>   /trunk/rave-portal-resources/src/main/webapp/WEB-INF/applicationContext-security.xml
1496346 
>   /trunk/rave-portal-resources/src/main/webapp/WEB-INF/cxf-applicationContext.xml 1496346

>   /trunk/rave-portal-resources/src/main/webapp/WEB-INF/dispatcher-servlet.xml 1496346

>   /trunk/rave-providers/rave-opensocial-provider/rave-opensocial-core/src/main/java/org/apache/rave/opensocial/repository/impl/DecoratingOpenSocialPersonRepository.java
1496346 
> 
> Diff: https://reviews.apache.org/r/12076/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Chris Geer
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message