You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rave.apache.org by Daniel Gornstein <dg...@mitre.org> on 2013/07/29 16:45:22 UTC
Review Request 13017: RAVE 998 - Update models and repositories layers to
support CRUD operations from REST api
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13017/
-----------------------------------------------------------
Review request for rave.
Repository: rave
Description
-------
RAVE 998 - Update models and repositories layers to support CRUD operations from REST api
Diffs
-----
/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/PortalPreferenceRepository.java 1508083
/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/RegionRepository.java 1508083
/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/RegionWidgetRepository.java 1508083
/trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/UserRepository.java 1508083
/trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/model/JpaPortalPreference.java 1508083
/trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/model/JpaRegion.java 1508083
/trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/model/JpaRegionWidget.java 1508083
/trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/repository/impl/JpaPortalPreferenceRepository.java 1508083
/trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/repository/impl/JpaRegionRepository.java 1508083
/trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/repository/impl/JpaRegionWidgetRepository.java 1508083
/trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/repository/impl/JpaUserRepository.java 1508083
/trunk/rave-components/rave-jpa/src/test/java/org/apache/rave/portal/repository/impl/JpaPortalPreferenceRepositoryTest.java 1508083
/trunk/rave-components/rave-jpa/src/test/java/org/apache/rave/portal/repository/impl/JpaRegionRepositoryTest.java 1508083
/trunk/rave-components/rave-jpa/src/test/java/org/apache/rave/portal/repository/impl/JpaRegionWidgetRepositoryTest.java 1508083
/trunk/rave-components/rave-jpa/src/test/java/org/apache/rave/portal/repository/impl/JpaUserRepositoryTest.java 1508083
/trunk/rave-components/rave-mongodb/src/main/java/org/apache/rave/portal/repository/impl/MongoDbPortalPreferenceRepository.java 1508083
/trunk/rave-components/rave-mongodb/src/main/java/org/apache/rave/portal/repository/impl/MongoDbRegionRepository.java 1508083
/trunk/rave-components/rave-mongodb/src/main/java/org/apache/rave/portal/repository/impl/MongoDbRegionWidgetRepository.java 1508083
/trunk/rave-components/rave-mongodb/src/main/java/org/apache/rave/portal/repository/impl/MongoDbUserRepository.java 1508083
/trunk/rave-components/rave-mongodb/src/test/java/org/apache/rave/portal/repository/impl/MongoDbPortalPreferenceRepositoryTest.java 1508083
/trunk/rave-components/rave-mongodb/src/test/java/org/apache/rave/portal/repository/impl/MongoDbRegionRepositoryTest.java 1508083
/trunk/rave-components/rave-mongodb/src/test/java/org/apache/rave/portal/repository/impl/MongoDbRegionWidgetRepositoryTest.java 1508083
/trunk/rave-components/rave-mongodb/src/test/java/org/apache/rave/portal/repository/impl/MongoDbUserRepositoryTest.java 1508083
Diff: https://reviews.apache.org/r/13017/diff/
Testing
-------
Thanks,
Daniel Gornstein
Re: Review Request 13017: RAVE 998 - Update models and repositories layers
to support CRUD operations from REST api
Posted by Erin Noe-Payne <er...@mitre.org>.
> On July 29, 2013, 2:51 p.m., Chris Geer wrote:
> > I know the ResourceRepository interface wasn't added as part of this patch but it's the first time I've been reviewing it. I don't understand the need for a second repository. I looked in the ResourceRepository code and there wasn't really a hint why it was separate, in fact, the class level comment was identical to the Repository interface.
Chris, the ResourceRepository is meant to add the get all and paginated get all requests, which is needed for supporting the rest api. My thought was that looking at all of the repository interfaces many of them don't actually need to support those functions - OauthToken, ActivityStreams, Authority, etc. So I broke it into a separate interface.
If you disagree I can merge the two interfaces, or at least update the comments to actually say something meaningful.
- Erin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13017/#review24133
-----------------------------------------------------------
On July 29, 2013, 2:45 p.m., Daniel Gornstein wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13017/
> -----------------------------------------------------------
>
> (Updated July 29, 2013, 2:45 p.m.)
>
>
> Review request for rave.
>
>
> Repository: rave
>
>
> Description
> -------
>
> RAVE 998 - Update models and repositories layers to support CRUD operations from REST api
>
>
> Diffs
> -----
>
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/PortalPreferenceRepository.java 1508083
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/RegionRepository.java 1508083
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/RegionWidgetRepository.java 1508083
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/UserRepository.java 1508083
> /trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/model/JpaPortalPreference.java 1508083
> /trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/model/JpaRegion.java 1508083
> /trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/model/JpaRegionWidget.java 1508083
> /trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/repository/impl/JpaPortalPreferenceRepository.java 1508083
> /trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/repository/impl/JpaRegionRepository.java 1508083
> /trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/repository/impl/JpaRegionWidgetRepository.java 1508083
> /trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/repository/impl/JpaUserRepository.java 1508083
> /trunk/rave-components/rave-jpa/src/test/java/org/apache/rave/portal/repository/impl/JpaPortalPreferenceRepositoryTest.java 1508083
> /trunk/rave-components/rave-jpa/src/test/java/org/apache/rave/portal/repository/impl/JpaRegionRepositoryTest.java 1508083
> /trunk/rave-components/rave-jpa/src/test/java/org/apache/rave/portal/repository/impl/JpaRegionWidgetRepositoryTest.java 1508083
> /trunk/rave-components/rave-jpa/src/test/java/org/apache/rave/portal/repository/impl/JpaUserRepositoryTest.java 1508083
> /trunk/rave-components/rave-mongodb/src/main/java/org/apache/rave/portal/repository/impl/MongoDbPortalPreferenceRepository.java 1508083
> /trunk/rave-components/rave-mongodb/src/main/java/org/apache/rave/portal/repository/impl/MongoDbRegionRepository.java 1508083
> /trunk/rave-components/rave-mongodb/src/main/java/org/apache/rave/portal/repository/impl/MongoDbRegionWidgetRepository.java 1508083
> /trunk/rave-components/rave-mongodb/src/main/java/org/apache/rave/portal/repository/impl/MongoDbUserRepository.java 1508083
> /trunk/rave-components/rave-mongodb/src/test/java/org/apache/rave/portal/repository/impl/MongoDbPortalPreferenceRepositoryTest.java 1508083
> /trunk/rave-components/rave-mongodb/src/test/java/org/apache/rave/portal/repository/impl/MongoDbRegionRepositoryTest.java 1508083
> /trunk/rave-components/rave-mongodb/src/test/java/org/apache/rave/portal/repository/impl/MongoDbRegionWidgetRepositoryTest.java 1508083
> /trunk/rave-components/rave-mongodb/src/test/java/org/apache/rave/portal/repository/impl/MongoDbUserRepositoryTest.java 1508083
>
> Diff: https://reviews.apache.org/r/13017/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Daniel Gornstein
>
>
Re: Review Request 13017: RAVE 998 - Update models and repositories layers
to support CRUD operations from REST api
Posted by Erin Noe-Payne <er...@mitre.org>.
> On July 29, 2013, 2:51 p.m., Chris Geer wrote:
> > I know the ResourceRepository interface wasn't added as part of this patch but it's the first time I've been reviewing it. I don't understand the need for a second repository. I looked in the ResourceRepository code and there wasn't really a hint why it was separate, in fact, the class level comment was identical to the Repository interface.
>
> Erin Noe-Payne wrote:
> Chris, the ResourceRepository is meant to add the get all and paginated get all requests, which is needed for supporting the rest api. My thought was that looking at all of the repository interfaces many of them don't actually need to support those functions - OauthToken, ActivityStreams, Authority, etc. So I broke it into a separate interface.
>
> If you disagree I can merge the two interfaces, or at least update the comments to actually say something meaningful.
>
>
>
> Chris Geer wrote:
> Updating the comments would be good. Overall I think I could go either way on the interface. I realize not everything needs it but since almost all but a few select repositories should be accessible via web services, I'm ok saying that the default Repository interface has these methods to keep it simple. If we want to keep them separate I'll throw out a couple thoughts:
>
> 1) Does it make sense for the new interface to just extend the existing one so that there is only one interface to implement?
> 2) I'd like to see a clearer name for the interface. Maybe PagenatableRepository? (bad name, but something more descriptive than Resource)
Ok, I will merge into the Repository interface and fill in with not implemented.
- Erin
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13017/#review24133
-----------------------------------------------------------
On July 29, 2013, 2:45 p.m., Daniel Gornstein wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13017/
> -----------------------------------------------------------
>
> (Updated July 29, 2013, 2:45 p.m.)
>
>
> Review request for rave.
>
>
> Repository: rave
>
>
> Description
> -------
>
> RAVE 998 - Update models and repositories layers to support CRUD operations from REST api
>
>
> Diffs
> -----
>
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/PortalPreferenceRepository.java 1508083
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/RegionRepository.java 1508083
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/RegionWidgetRepository.java 1508083
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/UserRepository.java 1508083
> /trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/model/JpaPortalPreference.java 1508083
> /trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/model/JpaRegion.java 1508083
> /trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/model/JpaRegionWidget.java 1508083
> /trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/repository/impl/JpaPortalPreferenceRepository.java 1508083
> /trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/repository/impl/JpaRegionRepository.java 1508083
> /trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/repository/impl/JpaRegionWidgetRepository.java 1508083
> /trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/repository/impl/JpaUserRepository.java 1508083
> /trunk/rave-components/rave-jpa/src/test/java/org/apache/rave/portal/repository/impl/JpaPortalPreferenceRepositoryTest.java 1508083
> /trunk/rave-components/rave-jpa/src/test/java/org/apache/rave/portal/repository/impl/JpaRegionRepositoryTest.java 1508083
> /trunk/rave-components/rave-jpa/src/test/java/org/apache/rave/portal/repository/impl/JpaRegionWidgetRepositoryTest.java 1508083
> /trunk/rave-components/rave-jpa/src/test/java/org/apache/rave/portal/repository/impl/JpaUserRepositoryTest.java 1508083
> /trunk/rave-components/rave-mongodb/src/main/java/org/apache/rave/portal/repository/impl/MongoDbPortalPreferenceRepository.java 1508083
> /trunk/rave-components/rave-mongodb/src/main/java/org/apache/rave/portal/repository/impl/MongoDbRegionRepository.java 1508083
> /trunk/rave-components/rave-mongodb/src/main/java/org/apache/rave/portal/repository/impl/MongoDbRegionWidgetRepository.java 1508083
> /trunk/rave-components/rave-mongodb/src/main/java/org/apache/rave/portal/repository/impl/MongoDbUserRepository.java 1508083
> /trunk/rave-components/rave-mongodb/src/test/java/org/apache/rave/portal/repository/impl/MongoDbPortalPreferenceRepositoryTest.java 1508083
> /trunk/rave-components/rave-mongodb/src/test/java/org/apache/rave/portal/repository/impl/MongoDbRegionRepositoryTest.java 1508083
> /trunk/rave-components/rave-mongodb/src/test/java/org/apache/rave/portal/repository/impl/MongoDbRegionWidgetRepositoryTest.java 1508083
> /trunk/rave-components/rave-mongodb/src/test/java/org/apache/rave/portal/repository/impl/MongoDbUserRepositoryTest.java 1508083
>
> Diff: https://reviews.apache.org/r/13017/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Daniel Gornstein
>
>
Re: Review Request 13017: RAVE 998 - Update models and repositories layers
to support CRUD operations from REST api
Posted by Chris Geer <ch...@cxtsoftware.com>.
> On July 29, 2013, 2:51 p.m., Chris Geer wrote:
> > I know the ResourceRepository interface wasn't added as part of this patch but it's the first time I've been reviewing it. I don't understand the need for a second repository. I looked in the ResourceRepository code and there wasn't really a hint why it was separate, in fact, the class level comment was identical to the Repository interface.
>
> Erin Noe-Payne wrote:
> Chris, the ResourceRepository is meant to add the get all and paginated get all requests, which is needed for supporting the rest api. My thought was that looking at all of the repository interfaces many of them don't actually need to support those functions - OauthToken, ActivityStreams, Authority, etc. So I broke it into a separate interface.
>
> If you disagree I can merge the two interfaces, or at least update the comments to actually say something meaningful.
>
>
Updating the comments would be good. Overall I think I could go either way on the interface. I realize not everything needs it but since almost all but a few select repositories should be accessible via web services, I'm ok saying that the default Repository interface has these methods to keep it simple. If we want to keep them separate I'll throw out a couple thoughts:
1) Does it make sense for the new interface to just extend the existing one so that there is only one interface to implement?
2) I'd like to see a clearer name for the interface. Maybe PagenatableRepository? (bad name, but something more descriptive than Resource)
- Chris
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13017/#review24133
-----------------------------------------------------------
On July 29, 2013, 2:45 p.m., Daniel Gornstein wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13017/
> -----------------------------------------------------------
>
> (Updated July 29, 2013, 2:45 p.m.)
>
>
> Review request for rave.
>
>
> Repository: rave
>
>
> Description
> -------
>
> RAVE 998 - Update models and repositories layers to support CRUD operations from REST api
>
>
> Diffs
> -----
>
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/PortalPreferenceRepository.java 1508083
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/RegionRepository.java 1508083
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/RegionWidgetRepository.java 1508083
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/UserRepository.java 1508083
> /trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/model/JpaPortalPreference.java 1508083
> /trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/model/JpaRegion.java 1508083
> /trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/model/JpaRegionWidget.java 1508083
> /trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/repository/impl/JpaPortalPreferenceRepository.java 1508083
> /trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/repository/impl/JpaRegionRepository.java 1508083
> /trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/repository/impl/JpaRegionWidgetRepository.java 1508083
> /trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/repository/impl/JpaUserRepository.java 1508083
> /trunk/rave-components/rave-jpa/src/test/java/org/apache/rave/portal/repository/impl/JpaPortalPreferenceRepositoryTest.java 1508083
> /trunk/rave-components/rave-jpa/src/test/java/org/apache/rave/portal/repository/impl/JpaRegionRepositoryTest.java 1508083
> /trunk/rave-components/rave-jpa/src/test/java/org/apache/rave/portal/repository/impl/JpaRegionWidgetRepositoryTest.java 1508083
> /trunk/rave-components/rave-jpa/src/test/java/org/apache/rave/portal/repository/impl/JpaUserRepositoryTest.java 1508083
> /trunk/rave-components/rave-mongodb/src/main/java/org/apache/rave/portal/repository/impl/MongoDbPortalPreferenceRepository.java 1508083
> /trunk/rave-components/rave-mongodb/src/main/java/org/apache/rave/portal/repository/impl/MongoDbRegionRepository.java 1508083
> /trunk/rave-components/rave-mongodb/src/main/java/org/apache/rave/portal/repository/impl/MongoDbRegionWidgetRepository.java 1508083
> /trunk/rave-components/rave-mongodb/src/main/java/org/apache/rave/portal/repository/impl/MongoDbUserRepository.java 1508083
> /trunk/rave-components/rave-mongodb/src/test/java/org/apache/rave/portal/repository/impl/MongoDbPortalPreferenceRepositoryTest.java 1508083
> /trunk/rave-components/rave-mongodb/src/test/java/org/apache/rave/portal/repository/impl/MongoDbRegionRepositoryTest.java 1508083
> /trunk/rave-components/rave-mongodb/src/test/java/org/apache/rave/portal/repository/impl/MongoDbRegionWidgetRepositoryTest.java 1508083
> /trunk/rave-components/rave-mongodb/src/test/java/org/apache/rave/portal/repository/impl/MongoDbUserRepositoryTest.java 1508083
>
> Diff: https://reviews.apache.org/r/13017/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Daniel Gornstein
>
>
Re: Review Request 13017: RAVE 998 - Update models and repositories layers
to support CRUD operations from REST api
Posted by Chris Geer <ch...@cxtsoftware.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13017/#review24133
-----------------------------------------------------------
I know the ResourceRepository interface wasn't added as part of this patch but it's the first time I've been reviewing it. I don't understand the need for a second repository. I looked in the ResourceRepository code and there wasn't really a hint why it was separate, in fact, the class level comment was identical to the Repository interface.
- Chris Geer
On July 29, 2013, 2:45 p.m., Daniel Gornstein wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13017/
> -----------------------------------------------------------
>
> (Updated July 29, 2013, 2:45 p.m.)
>
>
> Review request for rave.
>
>
> Repository: rave
>
>
> Description
> -------
>
> RAVE 998 - Update models and repositories layers to support CRUD operations from REST api
>
>
> Diffs
> -----
>
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/PortalPreferenceRepository.java 1508083
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/RegionRepository.java 1508083
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/RegionWidgetRepository.java 1508083
> /trunk/rave-components/rave-core/src/main/java/org/apache/rave/portal/repository/UserRepository.java 1508083
> /trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/model/JpaPortalPreference.java 1508083
> /trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/model/JpaRegion.java 1508083
> /trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/model/JpaRegionWidget.java 1508083
> /trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/repository/impl/JpaPortalPreferenceRepository.java 1508083
> /trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/repository/impl/JpaRegionRepository.java 1508083
> /trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/repository/impl/JpaRegionWidgetRepository.java 1508083
> /trunk/rave-components/rave-jpa/src/main/java/org/apache/rave/portal/repository/impl/JpaUserRepository.java 1508083
> /trunk/rave-components/rave-jpa/src/test/java/org/apache/rave/portal/repository/impl/JpaPortalPreferenceRepositoryTest.java 1508083
> /trunk/rave-components/rave-jpa/src/test/java/org/apache/rave/portal/repository/impl/JpaRegionRepositoryTest.java 1508083
> /trunk/rave-components/rave-jpa/src/test/java/org/apache/rave/portal/repository/impl/JpaRegionWidgetRepositoryTest.java 1508083
> /trunk/rave-components/rave-jpa/src/test/java/org/apache/rave/portal/repository/impl/JpaUserRepositoryTest.java 1508083
> /trunk/rave-components/rave-mongodb/src/main/java/org/apache/rave/portal/repository/impl/MongoDbPortalPreferenceRepository.java 1508083
> /trunk/rave-components/rave-mongodb/src/main/java/org/apache/rave/portal/repository/impl/MongoDbRegionRepository.java 1508083
> /trunk/rave-components/rave-mongodb/src/main/java/org/apache/rave/portal/repository/impl/MongoDbRegionWidgetRepository.java 1508083
> /trunk/rave-components/rave-mongodb/src/main/java/org/apache/rave/portal/repository/impl/MongoDbUserRepository.java 1508083
> /trunk/rave-components/rave-mongodb/src/test/java/org/apache/rave/portal/repository/impl/MongoDbPortalPreferenceRepositoryTest.java 1508083
> /trunk/rave-components/rave-mongodb/src/test/java/org/apache/rave/portal/repository/impl/MongoDbRegionRepositoryTest.java 1508083
> /trunk/rave-components/rave-mongodb/src/test/java/org/apache/rave/portal/repository/impl/MongoDbRegionWidgetRepositoryTest.java 1508083
> /trunk/rave-components/rave-mongodb/src/test/java/org/apache/rave/portal/repository/impl/MongoDbUserRepositoryTest.java 1508083
>
> Diff: https://reviews.apache.org/r/13017/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Daniel Gornstein
>
>