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