[ 
https://issues.apache.org/jira/browse/OFBIZ-5312?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Jacques Le Roux updated OFBIZ-5312:
-----------------------------------

    Attachment: OFBIZ-5312 - ofbiz-ecommerce-seo.patch
                SeoContextFilter.java.patch

Here is a new completed patch
*Notes:*
* I have added the files urlregexp.xml and urlregexp.xsd in their OFBiz 
conventional places
* I placed the 4 transform classes in 
specialpurpose\ecommerce\src\org\ofbiz\ecommerce\webapp\control\ftl. Because by 
OFBiz conventions this is where they are supposed to be, so I moved 
SeoTransform.java there. Else I keep the Syracus directories structure. . The 
new ones are
CatalogAltUrlSeoTransform.java
UrlRegexpTransform.java
CatalogUrlSeoTransform.java
* I have also added those files in 
specialpurpose\ecommerce\src\org\ofbiz\ecommerce\webapp
CatalogUrlSeoFilter.java 
CategoryUrlServlet.java 
ControlServlet.java 
ProductUrlServlet.java 
UrlRegexpConfigUtil.java 
UrlRegexpContextFilter.java 
UrlUtil.java
* I commented out the authorization part in UrlRegexpContextFilter.java (no 
longer exists in OFBiz trunk), I mean getAuthz() and 
AbstractAuthorization.clearThreadLocal()
* tests pass without problems

*Questions to Jinghai:*
# Why do we need?
{code}
Index: specialpurpose/ecommerce/build.xml
===================================================================
--- specialpurpose/ecommerce/build.xml  (revision 1532805)
+++ specialpurpose/ecommerce/build.xml  (working copy)
@@ -48,5 +48,6 @@
         <fileset dir="../../applications/order/build/lib" includes="*.jar"/>
         <fileset dir="../../applications/accounting/build/lib" 
includes="*.jar"/>
         <fileset dir="../../applications/securityext/build/lib" 
includes="*.jar"/>
+        <fileset dir="../../framework/base/lib/scripting" includes="*.jar"/>
     </path>
 </project>
{code}
# I found several differences between your patch and Syracus for 
SeoContextFilter.java (see attached SeoContextFilter.java.patch for diff). I 
saw that in your patch you also removed getAuthz() and 
AbstractAuthorization.clearThreadLocal() which reassured me about that (I was 
not anxious ;o). The main point is your moving of the block which begins by // 
Debug.logInfo("In ContextFilter.doFilter, FORWARDED_FROM_SERVLET is NOT set", 
module); I suppose your patch is newer, because I checked in my working copy of 
Syracus and found that you did not touch SeoContextFilter.java since Februrary 
2013. Am'I right?


> Proposal: URL-Generation Changes
> --------------------------------
>
>                 Key: OFBIZ-5312
>                 URL: https://issues.apache.org/jira/browse/OFBIZ-5312
>             Project: OFBiz
>          Issue Type: New Feature
>          Components: specialpurpose/ecommerce
>    Affects Versions: SVN trunk
>            Reporter: Jacques Le Roux
>            Assignee: Jacques Le Roux
>            Priority: Minor
>              Labels: changes, ecommerce, friendly, seo, url
>             Fix For: SVN trunk
>
>         Attachments: OFBIZ-5312 - ofbiz-ecommerce-seo.patch, OFBIZ-5312 - 
> ofbiz-ecommerce-seo.patch, OFBIZ-5312 - ofbiz-ecommerce-seo.patch, 
> SeoContextFilter.java.patch
>
>
> [This was proposed by Paul Piper in Nabble 7 months 
> ago|http://ofbiz.135035.n4.nabble.com/Proposal-URL-Generation-Changes-td4639289.html].
>  Here is quoted Paul's proposal
> {quote}
> Hey Everyone,
> over at ilscipio (www.ilscipio.com) we developed a set of functional OFBiz 
> changes that we believe the entire community could benefit from. The changes 
> have been implemented in parts in Syracus (www.syracus.net) for a while now, 
> but we figured that some of which are too crucial for ofbiz' success in the 
> long run, so we are considering the contribution (as we did with the SOLR 
> component).
> As you are probably aware, OFBiz has a pretty uncommon way of generating 
> URLs. Most of this has to do with the fact that OFBiz uses a servlet 
> (ControlServlet)  to handle all requests. The servlet is mounted at /control, 
> so that it won't interfere with other servlets. Though functionally valid, 
> this has the sideeffect that all urls are actually created on /control, which 
> is neither pretty, nor good by any measures of SEO. It also means that a few 
> 302 redirects are necessary to forward the user from / to /control/main. It 
> also makes requests more complicated, since many forwards are necessary 
> whenever somebody wants to move away from this implementation.
> Since this is hurtful to many of the implementers, I wanted to discuss 
> whether or not you guys would be interested in the changes we have made. The 
> functional changes contain:
> * Removal of /control out of all the urls
> * SEO-friendly URLS
> * Configurable product/category and other URLs
> * Frontpage mapping from /main to /
> It was tested on our end and contains all necessary improvements (Transforms, 
> Sample Configuration, Servlets & Filters) for it to be applicable.
> If interested, I would create a new JIRA ticket for this and after a few 
> minor internal discussions, we will gladly provide the rest of you with it.
> Regards,
> Paul 
> {quote}
> There is even a patch, mostly done by Jinghai Shi, that I attach here. Even 
> if it has been already used in [Syracus|http://syracus.net/] since early this 
> year, some help would be needed to test it thoroughly in OFBiz.
> Then we should discuss if it's the way to go. I believe it is. Who needs a 
> /control/ or /main by default in ecommerce urls? Would you not prefer 
> http://localhost:8080/ecommerce/ over 
> http://localhost:8080/ecommerce/control/main ?



--
This message was sent by Atlassian JIRA
(v6.1#6144)

Reply via email to