You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rave.apache.org by mf...@apache.org on 2011/08/26 17:52:03 UTC
svn commit: r1162146 - in
/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal:
repository/impl/ service/impl/ web/controller/
Author: mfranklin
Date: Fri Aug 26 15:52:03 2011
New Revision: 1162146
URL: http://svn.apache.org/viewvc?rev=1162146&view=rev
Log:
Added TODO comments and fixed minor inconsistencies
Modified:
incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/repository/impl/H2OpenJpaDialect.java
incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/repository/impl/JpaWidgetRepository.java
incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/repository/impl/TranslatedH2Exception.java
incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/service/impl/DefaultPageService.java
incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/web/controller/PageController.java
incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/web/controller/WidgetStoreController.java
Modified: incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/repository/impl/H2OpenJpaDialect.java
URL: http://svn.apache.org/viewvc/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/repository/impl/H2OpenJpaDialect.java?rev=1162146&r1=1162145&r2=1162146&view=diff
==============================================================================
--- incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/repository/impl/H2OpenJpaDialect.java (original)
+++ incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/repository/impl/H2OpenJpaDialect.java Fri Aug 26 15:52:03 2011
@@ -32,7 +32,10 @@ import org.springframework.orm.jpa.vendo
*
* @author CARLUCCI
*/
+
+//TODO: Move this class to commons
public class H2OpenJpaDialect extends OpenJpaDialect {
+ private static final long serialVersionUID = 1L;
/**
* Translates an H2 database error into a Rave application Exception
*
Modified: incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/repository/impl/JpaWidgetRepository.java
URL: http://svn.apache.org/viewvc/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/repository/impl/JpaWidgetRepository.java?rev=1162146&r1=1162145&r2=1162146&view=diff
==============================================================================
--- incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/repository/impl/JpaWidgetRepository.java (original)
+++ incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/repository/impl/JpaWidgetRepository.java Fri Aug 26 15:52:03 2011
@@ -34,6 +34,8 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.stereotype.Repository;
+import static org.apache.rave.persistence.jpa.util.JpaUtil.getSingleResult;
+
@Repository
public class JpaWidgetRepository extends AbstractJpaRepository<Widget> implements WidgetRepository {
@@ -126,10 +128,7 @@ public class JpaWidgetRepository extends
// url is a unique field, so no paging needed
query.setParameter(Widget.PARAM_URL, widgetUrl);
final List<Widget> resultList = query.getResultList();
- if (resultList.isEmpty()) {
- return null;
- }
- return resultList.get(0);
+ return getSingleResult(resultList);
}
/**
@@ -140,6 +139,7 @@ public class JpaWidgetRepository extends
* @param pageSize maximum number of items to be returned
* @return valid list of widgets, can be empty
*/
+ //TODO: Make this generic and move to JpaUtil in commons
protected List<Widget> getPagedResult(TypedQuery<Widget> query, int offset, int pageSize) {
if (pageSize >= LARGE_PAGESIZE) {
log.warn("Requesting potentially large resultset of Widgets. Pagesize is {}", pageSize);
Modified: incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/repository/impl/TranslatedH2Exception.java
URL: http://svn.apache.org/viewvc/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/repository/impl/TranslatedH2Exception.java?rev=1162146&r1=1162145&r2=1162146&view=diff
==============================================================================
--- incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/repository/impl/TranslatedH2Exception.java (original)
+++ incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/repository/impl/TranslatedH2Exception.java Fri Aug 26 15:52:03 2011
@@ -22,6 +22,7 @@ import org.springframework.dao.DataAcces
*
* @author ACARLUCCI
*/
+//TODO: Move this class to commons
public class TranslatedH2Exception extends DataAccessException {
private int errorCode;
private String error;
Modified: incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/service/impl/DefaultPageService.java
URL: http://svn.apache.org/viewvc/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/service/impl/DefaultPageService.java?rev=1162146&r1=1162145&r2=1162146&view=diff
==============================================================================
--- incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/service/impl/DefaultPageService.java (original)
+++ incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/service/impl/DefaultPageService.java Fri Aug 26 15:52:03 2011
@@ -190,31 +190,6 @@ public class DefaultPageService implemen
target.getRegionWidgets().add(newPosition, widget);
}
- private static <T> T getFromRepository(long id, Repository<T> repo) {
- T object = repo.get(id);
- if (object == null) {
- throw new IllegalArgumentException("Could not find object of given id in " + repo.getClass().getSimpleName());
- }
- return object;
- }
-
- private static void updateRenderSequences(List<RegionWidget> regionWidgets) {
- int count = 0;
- for (RegionWidget widget : regionWidgets) {
- widget.setRenderOrder(count);
- count++;
- }
- }
-
- private static RegionWidget findRegionWidgetById(Long id, List<RegionWidget> regionWidgets) {
- for (RegionWidget widget : regionWidgets) {
- if (widget.getId().equals(id)) {
- return widget;
- }
- }
- throw new IllegalArgumentException("Invalid RegionWidget ID");
- }
-
private Page addNewPage(User user, String pageName, String pageLayoutCode) {
PageLayout pageLayout = pageLayoutRepository.getByPageLayoutCode(pageLayoutCode);
@@ -238,7 +213,8 @@ public class DefaultPageService implemen
return page;
}
-
+
+ //TODO: If there is a reason why this is annotated @Transactional when the calling public method is @Transactional, note it in comments
@Transactional(readOnly = false)
private void updatePageRenderSequences(List<Page> pages) {
if (pages != null && !pages.isEmpty()) {
@@ -287,5 +263,30 @@ public class DefaultPageService implemen
updatePageRenderSequences(pages);
return movingPage;
- }
+ }
+
+ private static <T> T getFromRepository(long id, Repository<T> repo) {
+ T object = repo.get(id);
+ if (object == null) {
+ throw new IllegalArgumentException("Could not find object of given id in " + repo.getClass().getSimpleName());
+ }
+ return object;
+ }
+
+ private static void updateRenderSequences(List<RegionWidget> regionWidgets) {
+ int count = 0;
+ for (RegionWidget widget : regionWidgets) {
+ widget.setRenderOrder(count);
+ count++;
+ }
+ }
+
+ private static RegionWidget findRegionWidgetById(Long id, List<RegionWidget> regionWidgets) {
+ for (RegionWidget widget : regionWidgets) {
+ if (widget.getId().equals(id)) {
+ return widget;
+ }
+ }
+ throw new IllegalArgumentException("Invalid RegionWidget ID");
+ }
}
\ No newline at end of file
Modified: incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/web/controller/PageController.java
URL: http://svn.apache.org/viewvc/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/web/controller/PageController.java?rev=1162146&r1=1162145&r2=1162146&view=diff
==============================================================================
--- incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/web/controller/PageController.java (original)
+++ incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/web/controller/PageController.java Fri Aug 26 15:52:03 2011
@@ -49,6 +49,7 @@ public class PageController {
private PageService pageService;
private UserService userService;
+ //TODO (RAVE-99) Figure out a better way to register script blocks so that we don't have to have this cross package dep
@Autowired
private OpenSocialEnvironment openSocialEnvironment;
Modified: incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/web/controller/WidgetStoreController.java
URL: http://svn.apache.org/viewvc/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/web/controller/WidgetStoreController.java?rev=1162146&r1=1162145&r2=1162146&view=diff
==============================================================================
--- incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/web/controller/WidgetStoreController.java (original)
+++ incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal/web/controller/WidgetStoreController.java Fri Aug 26 15:52:03 2011
@@ -112,6 +112,7 @@ public class WidgetStoreController {
* @param model {@link Model}
* @return the view name of the Add new Widget form
*/
+ //TODO: Change the value of this request mapping so that it follows the pattern /store/widget/add
@RequestMapping(method = RequestMethod.GET, value = "addwidget")
public String viewAddWidgetForm(Model model) {
final Widget widget = new Widget();
@@ -127,6 +128,9 @@ public class WidgetStoreController {
* @param model {@link Model}
* @return if successful the view name of the widget, otherwise the form
*/
+ /*TODO: Change the value of this request mapping so that it follows the pattern /store/widget/add
+ TODO: The value can be the same as the GET action as you are mapping based on method & name
+ */
@RequestMapping(method = RequestMethod.POST, value = "doaddwidget")
public String viewAddWidgetResult(@ModelAttribute Widget widget, BindingResult results,
Model model) {
Re: svn commit: r1162146 - in /incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal:
repository/impl/ service/impl/ web/controller/
Posted by Marlon Pierce <mp...@cs.indiana.edu>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
#2 is fine with me.
Marlon
On 8/26/11 12:03 PM, Franklin, Matthew B. wrote:
> I looked through the rave-portal java code and for the most part, it looks like everyone is starting to keep consistent coding practices. I think this is a good sign that the community is beginning to mature.
>
> There are few things I noticed and commented on in the code, but there was one thing I thought needed to be brought to the list. Some of the Model objects define the names of the queries as constant Strings that are then used in the annotations and the repositories. Others just use inline strings. I think we need to pick one of the following approaches and make the necessary changes to keep consistency:
>
> 1) Use inline Strings
> 2) Define constant Strings in the model classes
> 3) Create a Constants class that defines all strings used in named queries
>
> If no one cares which one we use, I will implement option 2.
>
> -Matt
>
>
>> -----Original Message-----
>> From: mfranklin@apache.org [mailto:mfranklin@apache.org]
>> Sent: Friday, August 26, 2011 11:52 AM
>> To: rave-commits@incubator.apache.org
>> Subject: svn commit: r1162146 - in /incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal: repository/impl/ service/impl/
>> web/controller/
>>
>> Author: mfranklin
>> Date: Fri Aug 26 15:52:03 2011
>> New Revision: 1162146
>>
>> URL: http://svn.apache.org/viewvc?rev=1162146&view=rev
>> Log:
>> Added TODO comments and fixed minor inconsistencies
>>
>> Modified:
>> incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/repository/impl/H2OpenJpaDial
>> ect.java
>> incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/repository/impl/JpaWidgetRep
>> ository.java
>> incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/repository/impl/TranslatedH2E
>> xception.java
>> incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/service/impl/DefaultPageServic
>> e.java
>> incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/web/controller/PageController.
>> java
>> incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/web/controller/WidgetStoreCo
>> ntroller.java
>>
>> Modified: incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/repository/impl/H2OpenJpaDial
>> ect.java
>> URL: http://svn.apache.org/viewvc/incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/repository/impl/H2OpenJpaDial
>> ect.java?rev=1162146&r1=1162145&r2=1162146&view=diff
>> ===========================================================
>> ===================
>> --- incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/repository/impl/H2OpenJpaDial
>> ect.java (original)
>> +++ incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/repository/impl/H2OpenJpaDial
>> ect.java Fri Aug 26 15:52:03 2011
>> @@ -32,7 +32,10 @@ import org.springframework.orm.jpa.vendo
>> *
>> * @author CARLUCCI
>> */
>> +
>> +//TODO: Move this class to commons
>> public class H2OpenJpaDialect extends OpenJpaDialect {
>> + private static final long serialVersionUID = 1L;
>> /**
>> * Translates an H2 database error into a Rave application Exception
>> *
>>
>> Modified: incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/repository/impl/JpaWidgetRep
>> ository.java
>> URL: http://svn.apache.org/viewvc/incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/repository/impl/JpaWidgetRep
>> ository.java?rev=1162146&r1=1162145&r2=1162146&view=diff
>> ===========================================================
>> ===================
>> --- incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/repository/impl/JpaWidgetRep
>> ository.java (original)
>> +++ incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/repository/impl/JpaWidgetRep
>> ository.java Fri Aug 26 15:52:03 2011
>> @@ -34,6 +34,8 @@ import org.slf4j.Logger;
>> import org.slf4j.LoggerFactory;
>> import org.springframework.stereotype.Repository;
>>
>> +import static org.apache.rave.persistence.jpa.util.JpaUtil.getSingleResult;
>> +
>> @Repository
>> public class JpaWidgetRepository extends AbstractJpaRepository<Widget>
>> implements WidgetRepository {
>>
>> @@ -126,10 +128,7 @@ public class JpaWidgetRepository extends
>> // url is a unique field, so no paging needed
>> query.setParameter(Widget.PARAM_URL, widgetUrl);
>> final List<Widget> resultList = query.getResultList();
>> - if (resultList.isEmpty()) {
>> - return null;
>> - }
>> - return resultList.get(0);
>> + return getSingleResult(resultList);
>> }
>>
>> /**
>> @@ -140,6 +139,7 @@ public class JpaWidgetRepository extends
>> * @param pageSize maximum number of items to be returned
>> * @return valid list of widgets, can be empty
>> */
>> + //TODO: Make this generic and move to JpaUtil in commons
>> protected List<Widget> getPagedResult(TypedQuery<Widget> query, int
>> offset, int pageSize) {
>> if (pageSize >= LARGE_PAGESIZE) {
>> log.warn("Requesting potentially large resultset of Widgets. Pagesize is
>> {}", pageSize);
>>
>> Modified: incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/repository/impl/TranslatedH2E
>> xception.java
>> URL: http://svn.apache.org/viewvc/incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/repository/impl/TranslatedH2E
>> xception.java?rev=1162146&r1=1162145&r2=1162146&view=diff
>> ===========================================================
>> ===================
>> --- incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/repository/impl/TranslatedH2E
>> xception.java (original)
>> +++ incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/repository/impl/TranslatedH2E
>> xception.java Fri Aug 26 15:52:03 2011
>> @@ -22,6 +22,7 @@ import org.springframework.dao.DataAcces
>> *
>> * @author ACARLUCCI
>> */
>> +//TODO: Move this class to commons
>> public class TranslatedH2Exception extends DataAccessException {
>> private int errorCode;
>> private String error;
>>
>> Modified: incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/service/impl/DefaultPageServic
>> e.java
>> URL: http://svn.apache.org/viewvc/incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/service/impl/DefaultPageServic
>> e.java?rev=1162146&r1=1162145&r2=1162146&view=diff
>> ===========================================================
>> ===================
>> --- incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/service/impl/DefaultPageServic
>> e.java (original)
>> +++ incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/service/impl/DefaultPageServic
>> e.java Fri Aug 26 15:52:03 2011
>> @@ -190,31 +190,6 @@ public class DefaultPageService implemen
>> target.getRegionWidgets().add(newPosition, widget);
>> }
>>
>> - private static <T> T getFromRepository(long id, Repository<T> repo) {
>> - T object = repo.get(id);
>> - if (object == null) {
>> - throw new IllegalArgumentException("Could not find object of given id
>> in " + repo.getClass().getSimpleName());
>> - }
>> - return object;
>> - }
>> -
>> - private static void updateRenderSequences(List<RegionWidget>
>> regionWidgets) {
>> - int count = 0;
>> - for (RegionWidget widget : regionWidgets) {
>> - widget.setRenderOrder(count);
>> - count++;
>> - }
>> - }
>> -
>> - private static RegionWidget findRegionWidgetById(Long id,
>> List<RegionWidget> regionWidgets) {
>> - for (RegionWidget widget : regionWidgets) {
>> - if (widget.getId().equals(id)) {
>> - return widget;
>> - }
>> - }
>> - throw new IllegalArgumentException("Invalid RegionWidget ID");
>> - }
>> -
>> private Page addNewPage(User user, String pageName, String
>> pageLayoutCode) {
>> PageLayout pageLayout =
>> pageLayoutRepository.getByPageLayoutCode(pageLayoutCode);
>>
>> @@ -238,7 +213,8 @@ public class DefaultPageService implemen
>>
>> return page;
>> }
>> -
>> +
>> + //TODO: If there is a reason why this is annotated @Transactional when
>> the calling public method is @Transactional, note it in comments
>> @Transactional(readOnly = false)
>> private void updatePageRenderSequences(List<Page> pages) {
>> if (pages != null && !pages.isEmpty()) {
>> @@ -287,5 +263,30 @@ public class DefaultPageService implemen
>> updatePageRenderSequences(pages);
>>
>> return movingPage;
>> - }
>> + }
>> +
>> + private static <T> T getFromRepository(long id, Repository<T> repo) {
>> + T object = repo.get(id);
>> + if (object == null) {
>> + throw new IllegalArgumentException("Could not find object of given id
>> in " + repo.getClass().getSimpleName());
>> + }
>> + return object;
>> + }
>> +
>> + private static void updateRenderSequences(List<RegionWidget>
>> regionWidgets) {
>> + int count = 0;
>> + for (RegionWidget widget : regionWidgets) {
>> + widget.setRenderOrder(count);
>> + count++;
>> + }
>> + }
>> +
>> + private static RegionWidget findRegionWidgetById(Long id,
>> List<RegionWidget> regionWidgets) {
>> + for (RegionWidget widget : regionWidgets) {
>> + if (widget.getId().equals(id)) {
>> + return widget;
>> + }
>> + }
>> + throw new IllegalArgumentException("Invalid RegionWidget ID");
>> + }
>> }
>> \ No newline at end of file
>>
>> Modified: incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/web/controller/PageController.
>> java
>> URL: http://svn.apache.org/viewvc/incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/web/controller/PageController.
>> java?rev=1162146&r1=1162145&r2=1162146&view=diff
>> ===========================================================
>> ===================
>> --- incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/web/controller/PageController.
>> java (original)
>> +++ incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/web/controller/PageController.
>> java Fri Aug 26 15:52:03 2011
>> @@ -49,6 +49,7 @@ public class PageController {
>> private PageService pageService;
>> private UserService userService;
>>
>> + //TODO (RAVE-99) Figure out a better way to register script blocks so that
>> we don't have to have this cross package dep
>> @Autowired
>> private OpenSocialEnvironment openSocialEnvironment;
>>
>>
>> Modified: incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/web/controller/WidgetStoreCo
>> ntroller.java
>> URL: http://svn.apache.org/viewvc/incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/web/controller/WidgetStoreCo
>> ntroller.java?rev=1162146&r1=1162145&r2=1162146&view=diff
>> ===========================================================
>> ===================
>> --- incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/web/controller/WidgetStoreCo
>> ntroller.java (original)
>> +++ incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/web/controller/WidgetStoreCo
>> ntroller.java Fri Aug 26 15:52:03 2011
>> @@ -112,6 +112,7 @@ public class WidgetStoreController {
>> * @param model {@link Model}
>> * @return the view name of the Add new Widget form
>> */
>> + //TODO: Change the value of this request mapping so that it follows the
>> pattern /store/widget/add
>> @RequestMapping(method = RequestMethod.GET, value = "addwidget")
>> public String viewAddWidgetForm(Model model) {
>> final Widget widget = new Widget();
>> @@ -127,6 +128,9 @@ public class WidgetStoreController {
>> * @param model {@link Model}
>> * @return if successful the view name of the widget, otherwise the form
>> */
>> + /*TODO: Change the value of this request mapping so that it follows the
>> pattern /store/widget/add
>> + TODO: The value can be the same as the GET action as you are mapping
>> based on method & name
>> + */
>> @RequestMapping(method = RequestMethod.POST, value =
>> "doaddwidget")
>> public String viewAddWidgetResult(@ModelAttribute Widget widget,
>> BindingResult results,
>> Model model) {
>>
>
-----BEGIN PGP SIGNATURE-----
Version: GnuPG/MacGPG2 v2.0.16 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/
iQEcBAEBAgAGBQJOV8dvAAoJEEfVXEODPFIDRDQIAJZJDj4bCEVsbopuhBc1y4qt
swvu1JScuy+mUY++xAxaaGmKAo/Ikdy9DQ9HIirpE1Aus4mm3SR8jPAuOECyXPWQ
EXt8YiNd2v+erp5x0mblAWwdOc79cJ02tchcJgJFjEoyL5/CVKxD2ZepxJcfdmO/
6ofUNdDDH3b769+PF71YQ4OWmnBJPqUL6sKG1A3FBmDZWkplRjh1zWrLDYcsegE7
QGL2WUgk0SCxEovF7rXqutvtb2ShvslpDUyv20sSTj6jASUXytcfu02nyraDYgl1
anbxuFMnxQzsRUeh3woiMe1iM1cU8tqS2uL2vdv8mJP1ImOACPvOjkdF4aC1WpA=
=rGsN
-----END PGP SIGNATURE-----
Re: svn commit: r1162146 - in /incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal:
repository/impl/ service/impl/ web/controller/
Posted by Jasha Joachimsthal <j....@onehippo.com>.
On 26 August 2011 18:03, Franklin, Matthew B. <mf...@mitre.org> wrote:
> I looked through the rave-portal java code and for the most part, it looks
> like everyone is starting to keep consistent coding practices. I think this
> is a good sign that the community is beginning to mature.
>
> There are few things I noticed and commented on in the code, but there was
> one thing I thought needed to be brought to the list. Some of the Model
> objects define the names of the queries as constant Strings that are then
> used in the annotations and the repositories. Others just use inline
> strings. I think we need to pick one of the following approaches and make
> the necessary changes to keep consistency:
>
> 1) Use inline Strings
> 2) Define constant Strings in the model classes
> 3) Create a Constants class that defines all strings used in named queries
>
> If no one cares which one we use, I will implement option 2.
>
Option 1 is too error prone, option 3 can grow to a very large class, so I'd
say option 2 is the best.
Still don't like the idea of defining the queries in the model when their
usage is in the Repository.
Jasha
Re: svn commit: r1162146 - in /incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal: repository/impl/ service/impl/ web/controller/
Posted by Raminderjeet Singh <ra...@gmail.com>.
I will also go with option 2.
Raminder
On Aug 26, 2011, at 12:03 PM, Franklin, Matthew B. wrote:
> I looked through the rave-portal java code and for the most part, it looks like everyone is starting to keep consistent coding practices. I think this is a good sign that the community is beginning to mature.
>
> There are few things I noticed and commented on in the code, but there was one thing I thought needed to be brought to the list. Some of the Model objects define the names of the queries as constant Strings that are then used in the annotations and the repositories. Others just use inline strings. I think we need to pick one of the following approaches and make the necessary changes to keep consistency:
>
> 1) Use inline Strings
> 2) Define constant Strings in the model classes
> 3) Create a Constants class that defines all strings used in named queries
>
> If no one cares which one we use, I will implement option 2.
>
> -Matt
>
>
>> -----Original Message-----
>> From: mfranklin@apache.org [mailto:mfranklin@apache.org]
>> Sent: Friday, August 26, 2011 11:52 AM
>> To: rave-commits@incubator.apache.org
>> Subject: svn commit: r1162146 - in /incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal: repository/impl/ service/impl/
>> web/controller/
>>
>> Author: mfranklin
>> Date: Fri Aug 26 15:52:03 2011
>> New Revision: 1162146
>>
>> URL: http://svn.apache.org/viewvc?rev=1162146&view=rev
>> Log:
>> Added TODO comments and fixed minor inconsistencies
>>
>> Modified:
>> incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/repository/impl/H2OpenJpaDial
>> ect.java
>> incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/repository/impl/JpaWidgetRep
>> ository.java
>> incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/repository/impl/TranslatedH2E
>> xception.java
>> incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/service/impl/DefaultPageServic
>> e.java
>> incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/web/controller/PageController.
>> java
>> incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/web/controller/WidgetStoreCo
>> ntroller.java
>>
>> Modified: incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/repository/impl/H2OpenJpaDial
>> ect.java
>> URL: http://svn.apache.org/viewvc/incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/repository/impl/H2OpenJpaDial
>> ect.java?rev=1162146&r1=1162145&r2=1162146&view=diff
>> ===========================================================
>> ===================
>> --- incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/repository/impl/H2OpenJpaDial
>> ect.java (original)
>> +++ incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/repository/impl/H2OpenJpaDial
>> ect.java Fri Aug 26 15:52:03 2011
>> @@ -32,7 +32,10 @@ import org.springframework.orm.jpa.vendo
>> *
>> * @author CARLUCCI
>> */
>> +
>> +//TODO: Move this class to commons
>> public class H2OpenJpaDialect extends OpenJpaDialect {
>> + private static final long serialVersionUID = 1L;
>> /**
>> * Translates an H2 database error into a Rave application Exception
>> *
>>
>> Modified: incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/repository/impl/JpaWidgetRep
>> ository.java
>> URL: http://svn.apache.org/viewvc/incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/repository/impl/JpaWidgetRep
>> ository.java?rev=1162146&r1=1162145&r2=1162146&view=diff
>> ===========================================================
>> ===================
>> --- incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/repository/impl/JpaWidgetRep
>> ository.java (original)
>> +++ incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/repository/impl/JpaWidgetRep
>> ository.java Fri Aug 26 15:52:03 2011
>> @@ -34,6 +34,8 @@ import org.slf4j.Logger;
>> import org.slf4j.LoggerFactory;
>> import org.springframework.stereotype.Repository;
>>
>> +import static org.apache.rave.persistence.jpa.util.JpaUtil.getSingleResult;
>> +
>> @Repository
>> public class JpaWidgetRepository extends AbstractJpaRepository<Widget>
>> implements WidgetRepository {
>>
>> @@ -126,10 +128,7 @@ public class JpaWidgetRepository extends
>> // url is a unique field, so no paging needed
>> query.setParameter(Widget.PARAM_URL, widgetUrl);
>> final List<Widget> resultList = query.getResultList();
>> - if (resultList.isEmpty()) {
>> - return null;
>> - }
>> - return resultList.get(0);
>> + return getSingleResult(resultList);
>> }
>>
>> /**
>> @@ -140,6 +139,7 @@ public class JpaWidgetRepository extends
>> * @param pageSize maximum number of items to be returned
>> * @return valid list of widgets, can be empty
>> */
>> + //TODO: Make this generic and move to JpaUtil in commons
>> protected List<Widget> getPagedResult(TypedQuery<Widget> query, int
>> offset, int pageSize) {
>> if (pageSize >= LARGE_PAGESIZE) {
>> log.warn("Requesting potentially large resultset of Widgets. Pagesize is
>> {}", pageSize);
>>
>> Modified: incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/repository/impl/TranslatedH2E
>> xception.java
>> URL: http://svn.apache.org/viewvc/incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/repository/impl/TranslatedH2E
>> xception.java?rev=1162146&r1=1162145&r2=1162146&view=diff
>> ===========================================================
>> ===================
>> --- incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/repository/impl/TranslatedH2E
>> xception.java (original)
>> +++ incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/repository/impl/TranslatedH2E
>> xception.java Fri Aug 26 15:52:03 2011
>> @@ -22,6 +22,7 @@ import org.springframework.dao.DataAcces
>> *
>> * @author ACARLUCCI
>> */
>> +//TODO: Move this class to commons
>> public class TranslatedH2Exception extends DataAccessException {
>> private int errorCode;
>> private String error;
>>
>> Modified: incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/service/impl/DefaultPageServic
>> e.java
>> URL: http://svn.apache.org/viewvc/incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/service/impl/DefaultPageServic
>> e.java?rev=1162146&r1=1162145&r2=1162146&view=diff
>> ===========================================================
>> ===================
>> --- incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/service/impl/DefaultPageServic
>> e.java (original)
>> +++ incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/service/impl/DefaultPageServic
>> e.java Fri Aug 26 15:52:03 2011
>> @@ -190,31 +190,6 @@ public class DefaultPageService implemen
>> target.getRegionWidgets().add(newPosition, widget);
>> }
>>
>> - private static <T> T getFromRepository(long id, Repository<T> repo) {
>> - T object = repo.get(id);
>> - if (object == null) {
>> - throw new IllegalArgumentException("Could not find object of given id
>> in " + repo.getClass().getSimpleName());
>> - }
>> - return object;
>> - }
>> -
>> - private static void updateRenderSequences(List<RegionWidget>
>> regionWidgets) {
>> - int count = 0;
>> - for (RegionWidget widget : regionWidgets) {
>> - widget.setRenderOrder(count);
>> - count++;
>> - }
>> - }
>> -
>> - private static RegionWidget findRegionWidgetById(Long id,
>> List<RegionWidget> regionWidgets) {
>> - for (RegionWidget widget : regionWidgets) {
>> - if (widget.getId().equals(id)) {
>> - return widget;
>> - }
>> - }
>> - throw new IllegalArgumentException("Invalid RegionWidget ID");
>> - }
>> -
>> private Page addNewPage(User user, String pageName, String
>> pageLayoutCode) {
>> PageLayout pageLayout =
>> pageLayoutRepository.getByPageLayoutCode(pageLayoutCode);
>>
>> @@ -238,7 +213,8 @@ public class DefaultPageService implemen
>>
>> return page;
>> }
>> -
>> +
>> + //TODO: If there is a reason why this is annotated @Transactional when
>> the calling public method is @Transactional, note it in comments
>> @Transactional(readOnly = false)
>> private void updatePageRenderSequences(List<Page> pages) {
>> if (pages != null && !pages.isEmpty()) {
>> @@ -287,5 +263,30 @@ public class DefaultPageService implemen
>> updatePageRenderSequences(pages);
>>
>> return movingPage;
>> - }
>> + }
>> +
>> + private static <T> T getFromRepository(long id, Repository<T> repo) {
>> + T object = repo.get(id);
>> + if (object == null) {
>> + throw new IllegalArgumentException("Could not find object of given id
>> in " + repo.getClass().getSimpleName());
>> + }
>> + return object;
>> + }
>> +
>> + private static void updateRenderSequences(List<RegionWidget>
>> regionWidgets) {
>> + int count = 0;
>> + for (RegionWidget widget : regionWidgets) {
>> + widget.setRenderOrder(count);
>> + count++;
>> + }
>> + }
>> +
>> + private static RegionWidget findRegionWidgetById(Long id,
>> List<RegionWidget> regionWidgets) {
>> + for (RegionWidget widget : regionWidgets) {
>> + if (widget.getId().equals(id)) {
>> + return widget;
>> + }
>> + }
>> + throw new IllegalArgumentException("Invalid RegionWidget ID");
>> + }
>> }
>> \ No newline at end of file
>>
>> Modified: incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/web/controller/PageController.
>> java
>> URL: http://svn.apache.org/viewvc/incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/web/controller/PageController.
>> java?rev=1162146&r1=1162145&r2=1162146&view=diff
>> ===========================================================
>> ===================
>> --- incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/web/controller/PageController.
>> java (original)
>> +++ incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/web/controller/PageController.
>> java Fri Aug 26 15:52:03 2011
>> @@ -49,6 +49,7 @@ public class PageController {
>> private PageService pageService;
>> private UserService userService;
>>
>> + //TODO (RAVE-99) Figure out a better way to register script blocks so that
>> we don't have to have this cross package dep
>> @Autowired
>> private OpenSocialEnvironment openSocialEnvironment;
>>
>>
>> Modified: incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/web/controller/WidgetStoreCo
>> ntroller.java
>> URL: http://svn.apache.org/viewvc/incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/web/controller/WidgetStoreCo
>> ntroller.java?rev=1162146&r1=1162145&r2=1162146&view=diff
>> ===========================================================
>> ===================
>> --- incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/web/controller/WidgetStoreCo
>> ntroller.java (original)
>> +++ incubator/rave/trunk/rave-
>> portal/src/main/java/org/apache/rave/portal/web/controller/WidgetStoreCo
>> ntroller.java Fri Aug 26 15:52:03 2011
>> @@ -112,6 +112,7 @@ public class WidgetStoreController {
>> * @param model {@link Model}
>> * @return the view name of the Add new Widget form
>> */
>> + //TODO: Change the value of this request mapping so that it follows the
>> pattern /store/widget/add
>> @RequestMapping(method = RequestMethod.GET, value = "addwidget")
>> public String viewAddWidgetForm(Model model) {
>> final Widget widget = new Widget();
>> @@ -127,6 +128,9 @@ public class WidgetStoreController {
>> * @param model {@link Model}
>> * @return if successful the view name of the widget, otherwise the form
>> */
>> + /*TODO: Change the value of this request mapping so that it follows the
>> pattern /store/widget/add
>> + TODO: The value can be the same as the GET action as you are mapping
>> based on method & name
>> + */
>> @RequestMapping(method = RequestMethod.POST, value =
>> "doaddwidget")
>> public String viewAddWidgetResult(@ModelAttribute Widget widget,
>> BindingResult results,
>> Model model) {
>>
>
RE: svn commit: r1162146 - in
/incubator/rave/trunk/rave-portal/src/main/java/org/apache/rave/portal:
repository/impl/ service/impl/ web/controller/
Posted by "Franklin, Matthew B." <mf...@mitre.org>.
I looked through the rave-portal java code and for the most part, it looks like everyone is starting to keep consistent coding practices. I think this is a good sign that the community is beginning to mature.
There are few things I noticed and commented on in the code, but there was one thing I thought needed to be brought to the list. Some of the Model objects define the names of the queries as constant Strings that are then used in the annotations and the repositories. Others just use inline strings. I think we need to pick one of the following approaches and make the necessary changes to keep consistency:
1) Use inline Strings
2) Define constant Strings in the model classes
3) Create a Constants class that defines all strings used in named queries
If no one cares which one we use, I will implement option 2.
-Matt
>-----Original Message-----
>From: mfranklin@apache.org [mailto:mfranklin@apache.org]
>Sent: Friday, August 26, 2011 11:52 AM
>To: rave-commits@incubator.apache.org
>Subject: svn commit: r1162146 - in /incubator/rave/trunk/rave-
>portal/src/main/java/org/apache/rave/portal: repository/impl/ service/impl/
>web/controller/
>
>Author: mfranklin
>Date: Fri Aug 26 15:52:03 2011
>New Revision: 1162146
>
>URL: http://svn.apache.org/viewvc?rev=1162146&view=rev
>Log:
>Added TODO comments and fixed minor inconsistencies
>
>Modified:
> incubator/rave/trunk/rave-
>portal/src/main/java/org/apache/rave/portal/repository/impl/H2OpenJpaDial
>ect.java
> incubator/rave/trunk/rave-
>portal/src/main/java/org/apache/rave/portal/repository/impl/JpaWidgetRep
>ository.java
> incubator/rave/trunk/rave-
>portal/src/main/java/org/apache/rave/portal/repository/impl/TranslatedH2E
>xception.java
> incubator/rave/trunk/rave-
>portal/src/main/java/org/apache/rave/portal/service/impl/DefaultPageServic
>e.java
> incubator/rave/trunk/rave-
>portal/src/main/java/org/apache/rave/portal/web/controller/PageController.
>java
> incubator/rave/trunk/rave-
>portal/src/main/java/org/apache/rave/portal/web/controller/WidgetStoreCo
>ntroller.java
>
>Modified: incubator/rave/trunk/rave-
>portal/src/main/java/org/apache/rave/portal/repository/impl/H2OpenJpaDial
>ect.java
>URL: http://svn.apache.org/viewvc/incubator/rave/trunk/rave-
>portal/src/main/java/org/apache/rave/portal/repository/impl/H2OpenJpaDial
>ect.java?rev=1162146&r1=1162145&r2=1162146&view=diff
>===========================================================
>===================
>--- incubator/rave/trunk/rave-
>portal/src/main/java/org/apache/rave/portal/repository/impl/H2OpenJpaDial
>ect.java (original)
>+++ incubator/rave/trunk/rave-
>portal/src/main/java/org/apache/rave/portal/repository/impl/H2OpenJpaDial
>ect.java Fri Aug 26 15:52:03 2011
>@@ -32,7 +32,10 @@ import org.springframework.orm.jpa.vendo
> *
> * @author CARLUCCI
> */
>+
>+//TODO: Move this class to commons
> public class H2OpenJpaDialect extends OpenJpaDialect {
>+ private static final long serialVersionUID = 1L;
> /**
> * Translates an H2 database error into a Rave application Exception
> *
>
>Modified: incubator/rave/trunk/rave-
>portal/src/main/java/org/apache/rave/portal/repository/impl/JpaWidgetRep
>ository.java
>URL: http://svn.apache.org/viewvc/incubator/rave/trunk/rave-
>portal/src/main/java/org/apache/rave/portal/repository/impl/JpaWidgetRep
>ository.java?rev=1162146&r1=1162145&r2=1162146&view=diff
>===========================================================
>===================
>--- incubator/rave/trunk/rave-
>portal/src/main/java/org/apache/rave/portal/repository/impl/JpaWidgetRep
>ository.java (original)
>+++ incubator/rave/trunk/rave-
>portal/src/main/java/org/apache/rave/portal/repository/impl/JpaWidgetRep
>ository.java Fri Aug 26 15:52:03 2011
>@@ -34,6 +34,8 @@ import org.slf4j.Logger;
> import org.slf4j.LoggerFactory;
> import org.springframework.stereotype.Repository;
>
>+import static org.apache.rave.persistence.jpa.util.JpaUtil.getSingleResult;
>+
> @Repository
> public class JpaWidgetRepository extends AbstractJpaRepository<Widget>
>implements WidgetRepository {
>
>@@ -126,10 +128,7 @@ public class JpaWidgetRepository extends
> // url is a unique field, so no paging needed
> query.setParameter(Widget.PARAM_URL, widgetUrl);
> final List<Widget> resultList = query.getResultList();
>- if (resultList.isEmpty()) {
>- return null;
>- }
>- return resultList.get(0);
>+ return getSingleResult(resultList);
> }
>
> /**
>@@ -140,6 +139,7 @@ public class JpaWidgetRepository extends
> * @param pageSize maximum number of items to be returned
> * @return valid list of widgets, can be empty
> */
>+ //TODO: Make this generic and move to JpaUtil in commons
> protected List<Widget> getPagedResult(TypedQuery<Widget> query, int
>offset, int pageSize) {
> if (pageSize >= LARGE_PAGESIZE) {
> log.warn("Requesting potentially large resultset of Widgets. Pagesize is
>{}", pageSize);
>
>Modified: incubator/rave/trunk/rave-
>portal/src/main/java/org/apache/rave/portal/repository/impl/TranslatedH2E
>xception.java
>URL: http://svn.apache.org/viewvc/incubator/rave/trunk/rave-
>portal/src/main/java/org/apache/rave/portal/repository/impl/TranslatedH2E
>xception.java?rev=1162146&r1=1162145&r2=1162146&view=diff
>===========================================================
>===================
>--- incubator/rave/trunk/rave-
>portal/src/main/java/org/apache/rave/portal/repository/impl/TranslatedH2E
>xception.java (original)
>+++ incubator/rave/trunk/rave-
>portal/src/main/java/org/apache/rave/portal/repository/impl/TranslatedH2E
>xception.java Fri Aug 26 15:52:03 2011
>@@ -22,6 +22,7 @@ import org.springframework.dao.DataAcces
> *
> * @author ACARLUCCI
> */
>+//TODO: Move this class to commons
> public class TranslatedH2Exception extends DataAccessException {
> private int errorCode;
> private String error;
>
>Modified: incubator/rave/trunk/rave-
>portal/src/main/java/org/apache/rave/portal/service/impl/DefaultPageServic
>e.java
>URL: http://svn.apache.org/viewvc/incubator/rave/trunk/rave-
>portal/src/main/java/org/apache/rave/portal/service/impl/DefaultPageServic
>e.java?rev=1162146&r1=1162145&r2=1162146&view=diff
>===========================================================
>===================
>--- incubator/rave/trunk/rave-
>portal/src/main/java/org/apache/rave/portal/service/impl/DefaultPageServic
>e.java (original)
>+++ incubator/rave/trunk/rave-
>portal/src/main/java/org/apache/rave/portal/service/impl/DefaultPageServic
>e.java Fri Aug 26 15:52:03 2011
>@@ -190,31 +190,6 @@ public class DefaultPageService implemen
> target.getRegionWidgets().add(newPosition, widget);
> }
>
>- private static <T> T getFromRepository(long id, Repository<T> repo) {
>- T object = repo.get(id);
>- if (object == null) {
>- throw new IllegalArgumentException("Could not find object of given id
>in " + repo.getClass().getSimpleName());
>- }
>- return object;
>- }
>-
>- private static void updateRenderSequences(List<RegionWidget>
>regionWidgets) {
>- int count = 0;
>- for (RegionWidget widget : regionWidgets) {
>- widget.setRenderOrder(count);
>- count++;
>- }
>- }
>-
>- private static RegionWidget findRegionWidgetById(Long id,
>List<RegionWidget> regionWidgets) {
>- for (RegionWidget widget : regionWidgets) {
>- if (widget.getId().equals(id)) {
>- return widget;
>- }
>- }
>- throw new IllegalArgumentException("Invalid RegionWidget ID");
>- }
>-
> private Page addNewPage(User user, String pageName, String
>pageLayoutCode) {
> PageLayout pageLayout =
>pageLayoutRepository.getByPageLayoutCode(pageLayoutCode);
>
>@@ -238,7 +213,8 @@ public class DefaultPageService implemen
>
> return page;
> }
>-
>+
>+ //TODO: If there is a reason why this is annotated @Transactional when
>the calling public method is @Transactional, note it in comments
> @Transactional(readOnly = false)
> private void updatePageRenderSequences(List<Page> pages) {
> if (pages != null && !pages.isEmpty()) {
>@@ -287,5 +263,30 @@ public class DefaultPageService implemen
> updatePageRenderSequences(pages);
>
> return movingPage;
>- }
>+ }
>+
>+ private static <T> T getFromRepository(long id, Repository<T> repo) {
>+ T object = repo.get(id);
>+ if (object == null) {
>+ throw new IllegalArgumentException("Could not find object of given id
>in " + repo.getClass().getSimpleName());
>+ }
>+ return object;
>+ }
>+
>+ private static void updateRenderSequences(List<RegionWidget>
>regionWidgets) {
>+ int count = 0;
>+ for (RegionWidget widget : regionWidgets) {
>+ widget.setRenderOrder(count);
>+ count++;
>+ }
>+ }
>+
>+ private static RegionWidget findRegionWidgetById(Long id,
>List<RegionWidget> regionWidgets) {
>+ for (RegionWidget widget : regionWidgets) {
>+ if (widget.getId().equals(id)) {
>+ return widget;
>+ }
>+ }
>+ throw new IllegalArgumentException("Invalid RegionWidget ID");
>+ }
> }
>\ No newline at end of file
>
>Modified: incubator/rave/trunk/rave-
>portal/src/main/java/org/apache/rave/portal/web/controller/PageController.
>java
>URL: http://svn.apache.org/viewvc/incubator/rave/trunk/rave-
>portal/src/main/java/org/apache/rave/portal/web/controller/PageController.
>java?rev=1162146&r1=1162145&r2=1162146&view=diff
>===========================================================
>===================
>--- incubator/rave/trunk/rave-
>portal/src/main/java/org/apache/rave/portal/web/controller/PageController.
>java (original)
>+++ incubator/rave/trunk/rave-
>portal/src/main/java/org/apache/rave/portal/web/controller/PageController.
>java Fri Aug 26 15:52:03 2011
>@@ -49,6 +49,7 @@ public class PageController {
> private PageService pageService;
> private UserService userService;
>
>+ //TODO (RAVE-99) Figure out a better way to register script blocks so that
>we don't have to have this cross package dep
> @Autowired
> private OpenSocialEnvironment openSocialEnvironment;
>
>
>Modified: incubator/rave/trunk/rave-
>portal/src/main/java/org/apache/rave/portal/web/controller/WidgetStoreCo
>ntroller.java
>URL: http://svn.apache.org/viewvc/incubator/rave/trunk/rave-
>portal/src/main/java/org/apache/rave/portal/web/controller/WidgetStoreCo
>ntroller.java?rev=1162146&r1=1162145&r2=1162146&view=diff
>===========================================================
>===================
>--- incubator/rave/trunk/rave-
>portal/src/main/java/org/apache/rave/portal/web/controller/WidgetStoreCo
>ntroller.java (original)
>+++ incubator/rave/trunk/rave-
>portal/src/main/java/org/apache/rave/portal/web/controller/WidgetStoreCo
>ntroller.java Fri Aug 26 15:52:03 2011
>@@ -112,6 +112,7 @@ public class WidgetStoreController {
> * @param model {@link Model}
> * @return the view name of the Add new Widget form
> */
>+ //TODO: Change the value of this request mapping so that it follows the
>pattern /store/widget/add
> @RequestMapping(method = RequestMethod.GET, value = "addwidget")
> public String viewAddWidgetForm(Model model) {
> final Widget widget = new Widget();
>@@ -127,6 +128,9 @@ public class WidgetStoreController {
> * @param model {@link Model}
> * @return if successful the view name of the widget, otherwise the form
> */
>+ /*TODO: Change the value of this request mapping so that it follows the
>pattern /store/widget/add
>+ TODO: The value can be the same as the GET action as you are mapping
>based on method & name
>+ */
> @RequestMapping(method = RequestMethod.POST, value =
>"doaddwidget")
> public String viewAddWidgetResult(@ModelAttribute Widget widget,
>BindingResult results,
> Model model) {
>