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