Skip to content

Commit 254ef20

Browse files
committed
initNews NPE fix
Adds some null checking and modifies the handling of newly created newsSets.
1 parent 8c01e15 commit 254ef20

File tree

3 files changed

+41
-9
lines changed

3 files changed

+41
-9
lines changed

src/main/java/org/jasig/portlet/newsreader/dao/HibernateNewsStore.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,11 +173,15 @@ public List<PredefinedNewsDefinition> getHiddenPredefinedNewsDefinitions(Long se
173173
/** {@inheritDoc} */
174174
public void initNews(NewsSet set, Set<String> roles) {
175175
try {
176+
if (set == null) {
177+
logger.warn("Null NewsSet passed to initNews method");
178+
return;
179+
}
176180

177181
// if the user doesn't have any roles, we don't have any
178182
// chance of getting predefined news, so just go ahead
179183
// and return
180-
if (roles.isEmpty())
184+
if (roles == null || roles.isEmpty())
181185
return;
182186

183187
String query = "FROM PredefinedNewsDefinition def "
@@ -356,6 +360,10 @@ public List<NewsSet> getNewsSetsForUser(String userId) {
356360
})
357361
public void storeNewsSet(NewsSet set) {
358362
try {
363+
if (set == null) {
364+
logger.warn("Null NewsSet passed to storeNewsSet method");
365+
return;
366+
}
359367

360368
getHibernateTemplate().saveOrUpdate(set);
361369
getHibernateTemplate().flush();

src/main/java/org/jasig/portlet/newsreader/mvc/portlet/reader/AjaxNewsController.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,11 @@ public ModelAndView getJSONFeeds(ResourceRequest request, ResourceResponse respo
124124

125125
String setName = request.getPreferences().getValue("newsSetName", "default");
126126
NewsSet set = setCreationService.getNewsSet(setName, request);
127+
if (set == null) {
128+
log.warn("Unable to retrieve NewsSet for setName={}", setName);
129+
model.put("message", "Unable to load news feeds. Please try again later.");
130+
return new ModelAndView("json", model);
131+
}
127132
final List<NewsConfiguration> feeds = AbstractNewsController.filterNonWhitelistedConfigurations(request, set.getNewsConfigurations());
128133
Collections.sort(feeds);
129134

src/main/java/org/jasig/portlet/newsreader/service/SharedNewsSetServiceImpl.java

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -68,12 +68,20 @@ public void setNewsStore(NewsStore newsStore) {
6868
*/
6969
/** {@inheritDoc} */
7070
public NewsSet getNewsSet(String fname, PortletRequest request) {
71+
if (request == null) {
72+
logger.warn("Null PortletRequest passed to getNewsSet method");
73+
return null;
74+
}
7175

7276
final PortletSession session = request.getPortletSession();
7377

7478
// get the user id associated with the current user, or use the configured
7579
// guest username if no user is authenticated
7680
final String userId = userIdService.getUserId(request);
81+
if (userId == null) {
82+
logger.warn("Unable to determine userId from request");
83+
return null;
84+
}
7785

7886
NewsSet set;
7987

@@ -88,19 +96,30 @@ public NewsSet getNewsSet(String fname, PortletRequest request) {
8896
set = new NewsSet();
8997
set.setUserId(userId);
9098
set.setName(fname);
91-
newsStore.storeNewsSet(set);
92-
//TODO: the persisted set (line above) isn't always available to the line below. Hibernate being lazy?
93-
set = newsStore.getNewsSet(userId, fname); // get set_id
99+
100+
try {
101+
newsStore.storeNewsSet(set);
102+
103+
if (set.getId() == null) {
104+
logger.warn("NewsSet was saved but no ID was generated for userId={}, fname={}", userId, fname);
105+
NewsSet retrievedSet = newsStore.getNewsSet(userId, fname);
106+
if (retrievedSet != null) {
107+
set = retrievedSet;
108+
} else {
109+
logger.error("Failed to create or retrieve NewsSet for userId={}, fname={}", userId, fname);
110+
return null;
111+
}
112+
}
113+
} catch (Exception e) {
114+
logger.error("Error creating NewsSet for userId={}, fname={}: {}", userId, fname, e.getMessage());
115+
return null;
116+
}
94117
}
95118

96119
// Persistent set is now loaded but may still need re-initalising since last use.
97120
// by adding setId to session, we signal that initialisation has taken place.
98121
if (session.getAttribute("setId", PortletSession.PORTLET_SCOPE) == null) {
99-
if (set != null) {
100-
logger.debug("re-initalising loaded newsSet " + set.getName());
101-
} else {
102-
logger.debug("attempting to re-initialize loaded newsSet, but it is null");
103-
}
122+
logger.debug("re-initalising loaded newsSet " + set.getName());
104123

105124
@SuppressWarnings("unchecked")
106125
final Set<String> roles = rolesService.getUserRoles(request);

0 commit comments

Comments
 (0)