grails-profiles / web-plugin

0 stars 1 forks source link

Create UrlMappings file with proper file name. #1

Open rpalcolea opened 8 years ago

rpalcolea commented 8 years ago

This is an enhancement ticket.

Context

grails/grails-core#9657 https://grails.slack.com/archives/plugins/p1471423052000063

People has issues with the naming convention for the UrlMappings file in plugins.

Proposal

Prepend the plugin name to the UrlMappings filename to prevent unpredictable results.

Solution

I was thinking on adding the following to the profile.yml but is already present

excludes:
  - grails-app/controllers/UrlMappings.groovy

I would like to know how to exclude the default UrlMappings.groovy that comes from the web profile

After that, create the UrlMappings artifact as follows:

@grails.codegen.projectClassName@UrlMappings.groovy

package @grails.codegen.defaultPackage@

class @grails.codegen.projectClassName@UrlMappings {

    static mappings = {
        "/$controller/$action?/$id?(.$format)?"{
            constraints {
                // apply constraints here
            }
        }

        "/"(view:"/index")
        "500"(view:'/error')
        "404"(view:'/notFound')
    }
}

Is this something that could generate value this profile?

What do you think? Is this something that you would like to have?

Thanks

Roberto

rpalcolea commented 8 years ago

Hi @graemerocher,

Is this something you would like to have in the framework or should I just closed the issue?

cheers

graemerocher commented 8 years ago

Maybe if you explained the motivation for this it would be more clear why you want to alter the name of UrlMappings

robertoschwald commented 8 years ago

If you have "UrlMappings.groovy" in a plugin, and create a new project, you get naming conflicts, as both in the plugin and in the project you have a duplicate class.

graemerocher commented 8 years ago

no you don't because the UrlMappings class is created in a package specific to the plugin or app

robertoschwald commented 8 years ago

Has that changed in a newer version? With Grails 3.0.10 I got it in the default package, which causes the collision. Other plugins are affected as well, e.g. https://github.com/bmuschko/grails-google-visualization/tree/master/grails-app/controllers

robertoschwald commented 8 years ago

Even the documentation for 3.1.1 states "grails-app/controllers/UrlMappings.groovy" http://docs.grails.org/3.1.1/guide/theWebLayer.html#urlmappings

And the plugin section states:

"In addition, the default UrlMappings.groovy file is excluded to avoid naming conflicts, however you are free to add a UrlMappings definition under a different name which will be included. For example a file called grails-app/controllers/BlogUrlMappings.groovy is fine."

Therefore, I think it would be best practice if Grails web-plugin would create the UrlMappings file prepended with the plugin name to avoid such collisions.

graemerocher commented 8 years ago

The documentation is out of date, try create an app or plugin with 3.1.9+... the UrlMappings.groovy file goes into a package to avoid conflicts. I see no need to include a unique name either.

rpalcolea commented 8 years ago

Hi guys,

Sorry, I didn't get notifications because I forgot to watch the repo, my fault

Yes, the documentation states that: http://docs.grails.org/latest/guide/plugins.html#providingBasicArtefacts

That was the motivation behind this ticket

I did what you suggested @graemerocher I created a plugin with 3.1.10 and got the following:

├── grails-app
│   ├── controllers
│   │   └── test
│   │       └── plugin
│   │           └── UrlMappings.groovy

So you're right, the file goes into a package to avoid conflicts.

I think it's an issue with older versions, am I right? but going forward it won't be necessary since Grails is already placing the file in a proper package.

Perhaps it would be better to clarify the docs for older versions of 3.x with this detail? Instead of changing the profile.