> 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 > >
