Skip to content
This repository has been archived by the owner on May 9, 2020. It is now read-only.

Commit

Permalink
Improved: Refactor ContentWorkerInterface methods signatures
Browse files Browse the repository at this point in the history
(OFBIZ-9164)

While working on OFBIZ-6919 which was built on R13.07 I stumbled upon an issue 
due to r1652852 where Adrian improved the cacheKey in 
FormFactory.getFormFromLocation() by adding a delegator reference (Tenants). 

Actually I'm not even sure it was done at r1652852 because Adrian did not 
maintain the FormFactory svn history.

Anyway, to make a long story short I had to introduce a DispatchContext 
parameter when calling FormFactory.readFormDocument() when the code from 
R13.07 only passed a null.

This had an impact in the hierarchy tree because FormFactory.readFormDocument() 
was called in DataResourceWorker class, where the new code was called 
from renderDataResourceAsText(). 

So I instead of only passing a Delegator I decided to pass only a 
LocalDispatcher parameter in renderDataResourceAsText(), since we can get the 
Delegator  from the LocalDispatcher. Doing so it had an impact on the 
renderDataResourceAsText hierarchy tree ending in DataResourceWorkerInterface 
and all related.

I finally decided to apply the same "Change Method Signature" refactoring 
pattern (http://refactoring.com/catalog/addParameter.html) to all cases 
related to ContentWorkerInterface. No need to pass a delegator when you have 
LocalDispatcher!

Since last commit and revert I fixed 5 test failures and improved the code more,
 mostly formatting, comments, etc.

There are very ugly method signatures out there which still need refactoring,
but it's so much work...

git-svn-id: https://svn.apache.org/repos/asf/ofbiz/trunk@1777297 13f79535-47bb-0310-9956-ffa450edef68
  • Loading branch information
JacquesLeRoux committed Jan 4, 2017
1 parent 8df17d7 commit 13f998c
Show file tree
Hide file tree
Showing 40 changed files with 183 additions and 167 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ public static List<SyndEntry> generateEntryList(LocalDispatcher dispatcher, Dele
String sub = null;
try {
Map<String, Object> dummy = new HashMap<String, Object>();
sub = ContentWorker.renderSubContentAsText(dispatcher, delegator, v.getString("contentId"), mapKey, dummy, locale, mimeTypeId, true);
sub = ContentWorker.renderSubContentAsText(dispatcher, v.getString("contentId"), mapKey, dummy, locale, mimeTypeId, true);
} catch (GeneralException e) {
Debug.logError(e, module);
} catch (IOException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ public static String cms(HttpServletRequest request, HttpServletResponse respons
writer = response.getWriter();
GenericValue webSiteContent = EntityQuery.use(delegator).from("WebSiteContent").where("webSiteId", webSiteId, "webSiteContentTypeId", "MAINTENANCE_PAGE").filterByDate().queryFirst();
if (webSiteContent != null) {
ContentWorker.renderContentAsText(dispatcher, delegator, webSiteContent.getString("contentId"), writer, null, locale, "text/html", null, null, true);
ContentWorker.renderContentAsText(dispatcher, webSiteContent.getString("contentId"), writer, null, locale, "text/html", null, null, true);
return "success";
} else {
request.setAttribute("_ERROR_MESSAGE_", "Not able to display maintenance page for [" + webSiteId + "]");
Expand Down Expand Up @@ -295,11 +295,11 @@ public static String cms(HttpServletRequest request, HttpServletResponse respons
List<GenericValue> webAnalytics = EntityQuery.use(delegator).from("WebAnalyticsConfig").where("webSiteId", webSiteId).queryList();
// render
if (UtilValidate.isNotEmpty(webAnalytics) && hasErrorPage) {
ContentWorker.renderContentAsText(dispatcher, delegator, contentId, writer, templateMap, locale, "text/html", null, null, true, webAnalytics);
ContentWorker.renderContentAsText(dispatcher, contentId, writer, templateMap, locale, "text/html", null, null, true, webAnalytics);
} else if (UtilValidate.isEmpty(mapKey)) {
ContentWorker.renderContentAsText(dispatcher, delegator, contentId, writer, templateMap, locale, "text/html", null, null, true);
ContentWorker.renderContentAsText(dispatcher, contentId, writer, templateMap, locale, "text/html", null, null, true);
} else {
ContentWorker.renderSubContentAsText(dispatcher, delegator, contentId, writer, mapKey, templateMap, locale, "text/html", true);
ContentWorker.renderSubContentAsText(dispatcher, contentId, writer, mapKey, templateMap, locale, "text/html", true);
}

} catch (TemplateException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ public static void indexKeywords(GenericValue content, boolean doAll) throws Gen
public static void addWeightedDataResourceString(GenericValue drView, int weight, List<String> strings, Delegator delegator, GenericValue content) {
Map<String, Object> drContext = UtilMisc.<String, Object>toMap("content", content);
try {
String contentText = DataResourceWorker.renderDataResourceAsText(delegator, drView.getString("dataResourceId"), drContext, null, null, false);
String contentText = DataResourceWorker.renderDataResourceAsText(null, delegator, drView.getString("dataResourceId"), drContext, null, null, false);
for (int i = 0; i < weight; i++) {
strings.add(contentText);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,19 +99,19 @@ public ContentMapFacade(LocalDispatcher dispatcher, GenericValue content, Map<St
init();
}

private ContentMapFacade(LocalDispatcher dispatcher, Delegator delegator, String contentId, Map<String, Object> context, Locale locale, String mimeTypeId, boolean cache) {
private ContentMapFacade(LocalDispatcher dispatcher, String contentId, Map<String, Object> context, Locale locale, String mimeTypeId, boolean cache) {
this.dispatcher = dispatcher;
this.delegator = delegator;
this.delegator = dispatcher.getDelegator();
this.contentId = contentId;
this.context = context;
this.locale = locale;
this.mimeType = mimeTypeId;
this.cache = cache;
try {
if (cache) {
this.value = EntityQuery.use(delegator).from("Content").where("contentId", contentId).cache().queryOne();
this.value = EntityQuery.use(this.delegator).from("Content").where("contentId", contentId).cache().queryOne();
} else {
this.value = EntityQuery.use(delegator).from("Content").where("contentId", contentId).queryOne();
this.value = EntityQuery.use(this.delegator).from("Content").where("contentId", contentId).queryOne();
}
} catch (GenericEntityException e) {
Debug.logError(e, module);
Expand Down Expand Up @@ -254,13 +254,12 @@ public Object get(Object obj) {
// so we're only looking for a direct alias using contentId
if (webSiteId != null && delegator != null) {
try {

GenericValue webSitePathAlias = EntityQuery.use(delegator).from("WebSitePathAlias")
.where("mapKey", null,
"webSiteId", webSiteId,
"contentId", this.contentId)
.where("mapKey", null, "webSiteId", webSiteId,"contentId", this.contentId)
.orderBy("-fromDate")
.cache().filterByDate().queryFirst();
.cache()
.filterByDate()
.queryFirst();
if (webSitePathAlias != null) {
contentUri = webSitePathAlias.getString("pathAlias");
}
Expand Down Expand Up @@ -300,7 +299,7 @@ public Object get(Object obj) {
}
if (subs != null) {
for (GenericValue v: subs) {
subContent.add(new ContentMapFacade(dispatcher, delegator, v.getString("contentId"), context, locale, mimeType, cache));
subContent.add(new ContentMapFacade(dispatcher, v.getString("contentId"), context, locale, mimeType, cache));
}
}
return subContent;
Expand Down Expand Up @@ -339,7 +338,7 @@ protected String renderThis() {
}

try {
return ContentWorker.renderContentAsText(dispatcher, delegator, contentId, renderCtx, locale, mimeType, cache);
return ContentWorker.renderContentAsText(dispatcher, contentId, renderCtx, locale, mimeType, cache);
} catch (GeneralException e) {
Debug.logError(e, module);
return e.toString();
Expand Down Expand Up @@ -432,7 +431,7 @@ public Object get(Object key) {
Debug.logError(e, module);
}
if (content != null) {
return new ContentMapFacade(dispatcher, delegator, content.getString("contentId"), context, locale, mimeType, cache);
return new ContentMapFacade(dispatcher, content.getString("contentId"), context, locale, mimeType, cache);
}

return null;
Expand Down Expand Up @@ -471,7 +470,7 @@ public Object get(Object key) {
Debug.logError(e, module);
}
if (sub != null) {
return new ContentMapFacade(dispatcher, delegator, sub.getString("contentId"), context, locale, mimeType, cache);
return new ContentMapFacade(dispatcher, sub.getString("contentId"), context, locale, mimeType, cache);
}

return null;
Expand Down Expand Up @@ -533,7 +532,7 @@ public Object get(Object key) {
} else if ("render".equalsIgnoreCase(name)) {
// render just the dataresource
try {
return DataResourceWorker.renderDataResourceAsText(delegator, value.getString("dataResourceId"), context, locale, mimeType, cache);
return DataResourceWorker.renderDataResourceAsText(dispatcher, delegator, value.getString("dataResourceId"), context, locale, mimeType, cache);
} catch (GeneralException e) {
Debug.logError(e, module);
return e.toString();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -846,7 +846,7 @@ public static Map<String, Object> renderSubContentAsText(DispatchContext dctx, M
}

try {
ContentWorker.renderSubContentAsText(dispatcher, delegator, contentId, outWriter, mapKey, templateContext, locale, mimeTypeId, true);
ContentWorker.renderSubContentAsText(dispatcher, contentId, outWriter, mapKey, templateContext, locale, mimeTypeId, true);
out.write(outWriter.toString());
results.put("textData", outWriter.toString());
} catch (GeneralException e) {
Expand Down Expand Up @@ -896,7 +896,7 @@ public static Map<String, Object> renderContentAsText(DispatchContext dctx, Map<
}

try {
ContentWorker.renderContentAsText(dispatcher, delegator, contentId, outWriter, templateContext, locale, mimeTypeId, null, null, true);
ContentWorker.renderContentAsText(dispatcher, contentId, outWriter, templateContext, locale, mimeTypeId, null, null, true);
if (out != null) out.write(outWriter.toString());
results.put("textData", outWriter.toString());
} catch (GeneralException e) {
Expand Down
Loading

0 comments on commit 13f998c

Please sign in to comment.