[JENKINS-20509] Move doCheckJobName to View

[jglick]

JENKINS-20509

Useless on its own because we are still awaiting a fix of JENKINS-33755 that would actually call this endpoint, but at least sets up the right backend.

@reviewbybees

[ghost]

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

[recena]

I have to include a similar validation in the client-side part, specifically, here.

[jglick]

You cannot do all this validation on the client side, and there is no reason to. We already have an endpoint to do all the validation you need.

[recena]

client-side validations do not exclude to server-side validations.

[jglick]

Well certainly you can do trivial client-side validation like that the field is not empty. But the detailed logic (illegal character names, Item.CREATE permission, ProjectNamingStrategy, existing child) would be hard to replicate on the client side, and we would wind up with a lot of duplicated code since the server already checks all this stuff before actually creating the new item.

[recena]

Exactly, I'm thinking about a client-side validation like: required, illegal-characters, etc... Really, I'm trying to avoid that the users reach this no pretty view:

[jglick]

But that is accomplished simply by calling checkJobName and displaying an error message if it returns one (perhaps also disabling the submit button).

[recena]

🐝 and 👍

[kzantow]

Should this actually be done at the Item / TopLevelItem level?

[jglick]

Should this actually be done at the Item / TopLevelItem level?

Not sure what you are asking.

This PR is just taking an endpoint used by View/newJob, /checkJobName, and making it possible to call it on a folder context rather than at root level, fixing the problems mentioned in JENKINS-20509: that the Item.CREATE check and existing child check were both being done against Jenkins rather than the Folder you are trying to create an item in.

Now it is also possible to keep this logic in Jenkins and add it to Folder, as in fact it was, but then you have two copies of the code, and they will diverge, as noted in jenkinsci/cloudbees-folder-plugin#51. So it is easier to maintain just one copy in View, which is the StaplerFallback of both.

[kzantow]

Let's say, for example, there's an item type for running an assembler build, it requires an old tool that breaks if spaces are in the path, in the event the Item type itself was aware of this limitation, it could prevent names being used with spaces. Obviously this wouldn't catch all scenarios. Anyway, this is probably a fairly irrelevant case.

🐝

[jglick]

in the event the Item type itself was aware of this limitation, it could prevent names being used with spaces

No it could not, because this method is not sensitive to item type. (It could be, but that would need to be a new API in TopLevelItemDescriptor.)

ProjectNamingStrategys can forbid such names at a system level.

[recena]

Where is being used?

[jglick]

Javadoc