[jbossseam-issues] [JBoss JIRA] Commented: (JBSEAM-2775) package-info.java should be optional:"convention over configuration"

[Eric Jung (JIRA)]

    [ http://jira.jboss.com/jira/browse/JBSEAM-2775?page=comments#action_12409385 ] 
            
Eric Jung commented on JBSEAM-2775:
-----------------------------------

Hi Norman,

Nice work. Here are my comments:

0. You didn't take my updates to the seampay example, demonstrating how this new feature works. Did you miss those changes or was that intentional? I think it's a good idea to showcase this in an example if we want people to use it :) I probably should have added a comment to components.xml explaining that the "pay" namespace is automatically resolved, instead of just changing the namespace.

1. You didn't update my class-level javadoc in NamespacePackageResolver.java, so it's out-of-date with what you checked into the trunk. For example, it still references the seam: namespace instead of java:. Let me know if you'd like me to submit a patch with just javadoc updates. I'd be happy to do it.

2. If you don't like the support for hierarchical URIs such as http://www.company.com/dept/product, I think you should remove it entirely. Since this is a feature upgrade to seam, people will be happy with the new feature even if it just supports opaque URIs. In 2008, java developers with more than just a basic understanding of XML comprehend that URI != URL, and won't be confused or have the need to specify only hierarchical URIs as the namespace. Furthermore, dropping support for hierarchical URLs will reduce the code significantly.

3. You removed a few things from my code which have me wondering why:
  a. You removed isValidJavaPackageName() but kept makeSafeForJava(). Why not drop both or keep both? Partial support for package name checking and fixing seems odd to me: why not go all the way or drop the whole idea? If you drop the idea, the class-level javadoc needs updating.
  b. For hierarchical URI parsing, you removed the part which ignores any leading www subdomain. If you're going to keep support for hierarchical URIs, I think this is important to keep. It makes the parsing a little smarter, and you know people are going to use their company's URL as their namespace.... corporate URLs generally begin with www but, by convention, no one includes www in their package structures.

4. Finally, I think makeSafeForJava() should implement all 3 bullet points at http://java.sun.com/docs/books/jls/third_edition/html/packages.html#7.7, not just the first bullet point. Sorry, I forget to implement the 2nd two.

I'd be happy to submit a patch for any/all of the above changes against trunk. Let me know how I can help.

Thanks,
Eric

> package-info.java should be optional:"convention over configuration"
> --------------------------------------------------------------------
>
>                 Key: JBSEAM-2775
>                 URL: http://jira.jboss.com/jira/browse/JBSEAM-2775
>             Project: Seam
>          Issue Type: Feature Request
>            Reporter: Eric Jung
>         Assigned To: Norman Richards
>             Fix For: 2.1.0.BETA1
>
>         Attachments: JBSEAM-2775-v3.diff, JBSEAM-2775.diff, JBSEAM-2775.diff
>
>
> Currently, package-info.java is required when referring to custom Seam components in components.xml. However, to further align Seam with the "convention over configuration" philosophy, this file can be optional by making some assumptions:
> 1. In the absence of package-info.java , the XML namespace in components.xml for custom components has a well-defined mapping to a Java package
> 2. This mapping can be defined as:
>   a. The XML namespace must be parsable by java.net.URI otherwise automatic mapping is aborted
>   a. Protocol (scheme), the www/ subdomain, port, query parameters, anchors (references), and userInfo are ignored, if present.
>   b. The top-level domain becomes the root Java package
>   c. Subdomains become Java packages under the root package, applied in right-to-left order as specified in the URI.
>   d. The path, as returned by URI.getPath(), is mapped to further Java packages such that each path element becomes another Java package appended in left-to-right order
> Examples:
> http://www.company.com/department/product ==> com.company.department.product
> https://company.com/department/product ==> com.company.department.product
> ftp://www.company.org/department/product ==> org.company.department.product
> abc://company.org/department/product ==> org.company.department.product
> company.net/foo/bar/baz ==> net.company.foo.bar.baz
> JIRA isn't displaying my last example correctly. See http://pastebin.mozilla.org/375878 for the last example.

-- 
This message is automatically generated by JIRA.
-
If you think it was sent incorrectly contact one of the administrators: http://jira.jboss.com/jira/secure/Administrators.jspa
-
For more information on JIRA, see: http://www.atlassian.com/software/jira