cxf-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Glen Mazza <glen.ma...@verizon.net>
Subject Re: svn commit: r589531 - in /incubator/cxf/trunk: common/common/src/main/java/org/apache/cxf/common/annotation/ common/common/src/main/java/org/apache/cxf/resource/ rt/ rt/core/src/main/java/org/apache/cxf/bus/spring/ rt/transports/http/src/main/java/org/...
Date Mon, 29 Oct 2007 10:49:24 GMT
Am Montag, den 29.10.2007, 07:23 +0000 schrieb ningjiang@apache.org:

> Author: ningjiang
> Date: Mon Oct 29 00:23:26 2007
> New Revision: 589531
> 
> URL: http://svn.apache.org/viewvc?rev=589531&view=rev
> Log:
> CXF-1143 added a generate-service-list flage, also fixed some NPEs of the resource resolver
> 

Nice work!  I wonder though it would be better to have this flag called
"hide-service-list-page" with a default value of "false".

I think if you have an option that is infrequently needed (say less than
20% of the time), it is more common to have that option be explicitly
activated with a value of "true".

If you call it "generate-service-list", I'm concerned (1) people will
incorrectly think that they need to specify that in order to show the
service list page (i.e., by default, CXF does not show the page, while
it actually does), and (2) conversely, people may incorrectly think that
page will *not* appear if they do not specify that flag, and think that
it is a CXF bug when it does.

Using the "-page" suffix also helps clarify that they will still be able
to see the WSDL's, just not the list of them, as well as the fact that
this is just a HTML feature (turning off viewing of the page)--not
something affecting internal processing.

Regards,
Glen


> Modified: incubator/cxf/trunk/rt/transports/http/src/main/java/org/apache/cxf/transport/servlet/ServletController.java
> URL: http://svn.apache.org/viewvc/incubator/cxf/trunk/rt/transports/http/src/main/java/org/apache/cxf/transport/servlet/ServletController.java?rev=589531&r1=589530&r2=589531&view=diff
> ==============================================================================
> --- incubator/cxf/trunk/rt/transports/http/src/main/java/org/apache/cxf/transport/servlet/ServletController.java
(original)
> +++ incubator/cxf/trunk/rt/transports/http/src/main/java/org/apache/cxf/transport/servlet/ServletController.java
Mon Oct 29 00:23:26 2007
> @@ -54,10 +54,15 @@
>      private ServletTransportFactory transport;
>      private CXFServlet cxfServlet;
>      private String lastBase = "";
> +    private boolean isGenerateServiceList = true;
>   
>      public ServletController(ServletTransportFactory df, CXFServlet servlet) {
>          this.transport = df;
> -        this.cxfServlet = servlet;
> +        this.cxfServlet = servlet;       
> +    }
> +    
> +    public void setGenerateServiceList(boolean generate) {
> +        isGenerateServiceList = generate;
>      }
>      
>      private synchronized void updateDests(HttpServletRequest request) {
> @@ -165,18 +170,19 @@
>          Collection<ServletDestination> destinations = transport.getDestinations();
>          response.setContentType("text/html");        
>          response.getWriter().write("<html><body>");
> -        
> -        if (destinations.size() > 0) {  
> -            for (ServletDestination sd : destinations) {
> -                if (null != sd.getEndpointInfo().getName()) {
> -                    String address = sd.getEndpointInfo().getAddress();
> -                    response.getWriter().write("<p> <a href=\"" + address +
"?wsdl\">");
> -                    response.getWriter().write(sd.getEndpointInfo().getName() + "</a>
</p>");
> -                }    
> +        if (isGenerateServiceList) {
> +            if (destinations.size() > 0) {  
> +                for (ServletDestination sd : destinations) {
> +                    if (null != sd.getEndpointInfo().getName()) {
> +                        String address = sd.getEndpointInfo().getAddress();
> +                        response.getWriter().write("<p> <a href=\"" + address
+ "?wsdl\">");
> +                        response.getWriter().write(sd.getEndpointInfo().getName() +
"</a> </p>");
> +                    }    
> +                }
> +            } else {
> +                response.getWriter().write("No service was found.");
>              }
> -        } else {
> -            response.getWriter().write("No service was found.");
> -        }
> +        }    
>          response.getWriter().write("</body></html>");
>      }
>  
> 
> Modified: incubator/cxf/trunk/systests/src/test/java/org/apache/cxf/systest/servlet/SpringServletTest.java
> URL: http://svn.apache.org/viewvc/incubator/cxf/trunk/systests/src/test/java/org/apache/cxf/systest/servlet/SpringServletTest.java?rev=589531&r1=589530&r2=589531&view=diff
> ==============================================================================
> --- incubator/cxf/trunk/systests/src/test/java/org/apache/cxf/systest/servlet/SpringServletTest.java
(original)
> +++ incubator/cxf/trunk/systests/src/test/java/org/apache/cxf/systest/servlet/SpringServletTest.java
Mon Oct 29 00:23:26 2007
> @@ -118,5 +118,18 @@
>          
>      }
>      
> +    @Test
> +    public void testGetServiceList() throws Exception {
> +        ServletUnitClient client = newClient();
> +        client.setExceptionsThrownOnErrorStatus(true);
> +        
> +        WebRequest req = 
> +            new GetMethodQueryWebRequest(CONTEXT_URL + "/services/");
> +        WebResponse res = client.getResponse(req);
> +        assertEquals(200, res.getResponseCode());
> +        assertEquals("text/html", res.getContentType());
> +        assertEquals("Here should have no services links ", 0, res.getLinks().length);
> +                
> +    }
>      
>  }
> 
> Modified: incubator/cxf/trunk/systests/src/test/java/org/apache/cxf/systest/servlet/web-spring.xml
> URL: http://svn.apache.org/viewvc/incubator/cxf/trunk/systests/src/test/java/org/apache/cxf/systest/servlet/web-spring.xml?rev=589531&r1=589530&r2=589531&view=diff
> ==============================================================================
> --- incubator/cxf/trunk/systests/src/test/java/org/apache/cxf/systest/servlet/web-spring.xml
(original)
> +++ incubator/cxf/trunk/systests/src/test/java/org/apache/cxf/systest/servlet/web-spring.xml
Mon Oct 29 00:23:26 2007
> @@ -39,10 +39,14 @@
>  	
>  	<servlet>
>  		<servlet-name>CXFServlet</servlet-name>
> -		<display-name>CXF Servlet</display-name>
> +		<display-name>CXF Servlet</display-name>		
>  		<servlet-class>
>  			org.apache.cxf.transport.servlet.CXFServlet
>  		</servlet-class>
> +		<init-param> 
> +            <param-name>generate-service-list</param-name> 
> +            <param-value>false</param-value> 
> +       </init-param> 
>  		<load-on-startup>1</load-on-startup>
>  	</servlet>
>  
> 
> 


Mime
View raw message