Changing a plugin parameter type

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Changing a plugin parameter type

John Westcott
In the Jenkins Ansible Tower plugin I had a checkbox for importing logs. In order to satisfy a new requirement I would like to change this into a select menu. So I modified my code to switch from a true/false checkbox into a select menu which has values of true, false, full and vars as options.
My plugin supported both Freestyle and Pipeline Jenkins jobs.
After changing my code all existing freestyle jobs made the conversion seamlessly an old boolean true or false values mapped into the string values “true” and “false” without any issues.

However, groovy scripts using the pipeline portion of the plugin are throwing stack exceptions because the grooy scripts specify boolean values like :
importTowerLogs = true,

These values are not mapping to strings and Jenkins is putting out an exception when running the pipeline:
java.lang.ClassCastException: org.jenkinsci.plugins.ansible_tower.AnsibleTowerStep.importTowerLogs expects class java.lang.String but received class java.lang.Boolean
	at org.jenkinsci.plugins.structs.describable.DescribableModel.coerce(DescribableModel.java:492)
	at org.jenkinsci.plugins.structs.describable.DescribableModel.buildArguments(DescribableModel.java:409)
	at org.jenkinsci.plugins.structs.describable.DescribableModel.buildArguments(DescribableModel.java:409)
	     at org.jenkinsci.plugins.structs.describable.DescribableModel.coerce(DescribableModel.java:492)

My old @DataBoundConstructor looked like:
@DataBoundConstructor
public AnsibleTowerStep(@Nonnull String towerServer, Boolean importTowerLogs, Boolean param2, ...) {

My new constructor needs to be:
@DataBoundConstructor
public AnsibleTowerStep(
@Nonnull String towerServer, String importTowerLogs, Boolean param2, ...) {

I tried having two @DataBoundConstructors, one for the old Boolean and one for the String, but this caused an exception when compiling the plugin:
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:compile (default-compile) on project ansible-tower: Compilation failure
[ERROR] javax.annotation.processing.FilerException: Attempt to reopen a file for path /Users/jowestco/IdeaProjects/ansible-tower-plugin/target/classes/org/jenkinsci/plugins/ansible_tower/AnsibleTowerStep.stapler
[ERROR] at com.sun.tools.javac.processing.JavacFiler.checkFileReopening(JavacFiler.java:535)
[ERROR] at com.sun.tools.javac.processing.JavacFiler.createResource(JavacFiler.java:431)
[ERROR] at org.kohsuke.stapler.jsr269.AbstractProcessorImpl.createResource(AbstractProcessorImpl.java:81)


After that I tried to have a constructor (not decorated with @DataBoundConstructor) that would take a Boolean instead of a String and would call my DataBoundConstructor like:
public AnsibleTowerStep(@Nonnull String towerServer, Boolean importTowerLogs, Boolean param2, ...) { this(towerServer, importTowerLogs.toString(), param2…); }
But the pipelines were unable to find and use this constructor and threw the same stack trace as above.

As another solution, I also tried to change the constructor to just an Object like:
@DataBoundConstructor
public AnsibleTowerStep(@Nonnull String towerServer, Object importTowerLogs, Boolean param2, ...) {
With this the groovy scripts worked and I was able to use importTowerLogs.toString() to get a string for either a boolean or string object. Unfortunately, with this solution, the pipeline syntax generator would raise an exception that it couldn’t convert a string param into an object..

Does anyone have any other ideas on how I may be able to achieve what I am trying to do seamlessly (change a boolean field to a string)? Or do I need to tell my users that the new version of my plugin requires changing groovy scripts from 'importTowerLogs = true’ to ‘importTowerLogs = ‘true”’?
If making users update groovy scripts is my only solution, what are the recommend way(s) to warn users about this change? Obviously I will put it in the release nodes and spike it out somewhere in my README.md file as well; but is there anything else I can/should do? i.e. can I mark the new version of the plugin as using an incomparable syntax with the old version?

Thanks in advance,

-John

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/DDE53530-48EF-4910-A085-2A8F695BB525%40redhat.com.
Reply | Threaded
Open this post in threaded view
|

Re: Changing a plugin parameter type

Gavin Mogan
You should be able to make @DataBoundSetter and make a setter

I think I remember people saying that you need to at least leave the old constructor in place for legacy freestyle jobs. I know that's why I prefer setters.

On Thu., Jun. 4, 2020, 1:43 p.m. John Westcott, <[hidden email]> wrote:
In the Jenkins Ansible Tower plugin I had a checkbox for importing logs. In order to satisfy a new requirement I would like to change this into a select menu. So I modified my code to switch from a true/false checkbox into a select menu which has values of true, false, full and vars as options.
My plugin supported both Freestyle and Pipeline Jenkins jobs.
After changing my code all existing freestyle jobs made the conversion seamlessly an old boolean true or false values mapped into the string values “true” and “false” without any issues.

However, groovy scripts using the pipeline portion of the plugin are throwing stack exceptions because the grooy scripts specify boolean values like :
importTowerLogs = true,

These values are not mapping to strings and Jenkins is putting out an exception when running the pipeline:
java.lang.ClassCastException: org.jenkinsci.plugins.ansible_tower.AnsibleTowerStep.importTowerLogs expects class java.lang.String but received class java.lang.Boolean
	at org.jenkinsci.plugins.structs.describable.DescribableModel.coerce(DescribableModel.java:492)
	at org.jenkinsci.plugins.structs.describable.DescribableModel.buildArguments(DescribableModel.java:409)
	at org.jenkinsci.plugins.structs.describable.DescribableModel.buildArguments(DescribableModel.java:409)
	     at org.jenkinsci.plugins.structs.describable.DescribableModel.coerce(DescribableModel.java:492)

My old @DataBoundConstructor looked like:
@DataBoundConstructor
public AnsibleTowerStep(@Nonnull String towerServer, Boolean importTowerLogs, Boolean param2, ...) {

My new constructor needs to be:
@DataBoundConstructor
public AnsibleTowerStep(
@Nonnull String towerServer, String importTowerLogs, Boolean param2, ...) {

I tried having two @DataBoundConstructors, one for the old Boolean and one for the String, but this caused an exception when compiling the plugin:
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:compile (default-compile) on project ansible-tower: Compilation failure
[ERROR] javax.annotation.processing.FilerException: Attempt to reopen a file for path /Users/jowestco/IdeaProjects/ansible-tower-plugin/target/classes/org/jenkinsci/plugins/ansible_tower/AnsibleTowerStep.stapler
[ERROR] at com.sun.tools.javac.processing.JavacFiler.checkFileReopening(JavacFiler.java:535)
[ERROR] at com.sun.tools.javac.processing.JavacFiler.createResource(JavacFiler.java:431)
[ERROR] at org.kohsuke.stapler.jsr269.AbstractProcessorImpl.createResource(AbstractProcessorImpl.java:81)


After that I tried to have a constructor (not decorated with @DataBoundConstructor) that would take a Boolean instead of a String and would call my DataBoundConstructor like:
public AnsibleTowerStep(@Nonnull String towerServer, Boolean importTowerLogs, Boolean param2, ...) { this(towerServer, importTowerLogs.toString(), param2…); }
But the pipelines were unable to find and use this constructor and threw the same stack trace as above.

As another solution, I also tried to change the constructor to just an Object like:
@DataBoundConstructor
public AnsibleTowerStep(@Nonnull String towerServer, Object importTowerLogs, Boolean param2, ...) {
With this the groovy scripts worked and I was able to use importTowerLogs.toString() to get a string for either a boolean or string object. Unfortunately, with this solution, the pipeline syntax generator would raise an exception that it couldn’t convert a string param into an object..

Does anyone have any other ideas on how I may be able to achieve what I am trying to do seamlessly (change a boolean field to a string)? Or do I need to tell my users that the new version of my plugin requires changing groovy scripts from 'importTowerLogs = true’ to ‘importTowerLogs = ‘true”’?
If making users update groovy scripts is my only solution, what are the recommend way(s) to warn users about this change? Obviously I will put it in the release nodes and spike it out somewhere in my README.md file as well; but is there anything else I can/should do? i.e. can I mark the new version of the plugin as using an incomparable syntax with the old version?

Thanks in advance,

-John

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/DDE53530-48EF-4910-A085-2A8F695BB525%40redhat.com.

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CAG%3D_DusrOaR%2BJ-S2WdDgg-bT-VHce%3DbuvHasuzRgLVvbu70A6w%40mail.gmail.com.
Reply | Threaded
Open this post in threaded view
|

Re: Changing a plugin parameter type

slide
Yes, definitely use a @DataBoundSetter, it will get called automatically when the configuration is saved after the @DataBoundConstructor is called.

On Thu, Jun 4, 2020 at 3:00 PM 'Gavin Mogan' via Jenkins Developers <[hidden email]> wrote:
You should be able to make @DataBoundSetter and make a setter

I think I remember people saying that you need to at least leave the old constructor in place for legacy freestyle jobs. I know that's why I prefer setters.

On Thu., Jun. 4, 2020, 1:43 p.m. John Westcott, <[hidden email]> wrote:
In the Jenkins Ansible Tower plugin I had a checkbox for importing logs. In order to satisfy a new requirement I would like to change this into a select menu. So I modified my code to switch from a true/false checkbox into a select menu which has values of true, false, full and vars as options.
My plugin supported both Freestyle and Pipeline Jenkins jobs.
After changing my code all existing freestyle jobs made the conversion seamlessly an old boolean true or false values mapped into the string values “true” and “false” without any issues.

However, groovy scripts using the pipeline portion of the plugin are throwing stack exceptions because the grooy scripts specify boolean values like :
importTowerLogs = true,

These values are not mapping to strings and Jenkins is putting out an exception when running the pipeline:
java.lang.ClassCastException: org.jenkinsci.plugins.ansible_tower.AnsibleTowerStep.importTowerLogs expects class java.lang.String but received class java.lang.Boolean
	at org.jenkinsci.plugins.structs.describable.DescribableModel.coerce(DescribableModel.java:492)
	at org.jenkinsci.plugins.structs.describable.DescribableModel.buildArguments(DescribableModel.java:409)
	at org.jenkinsci.plugins.structs.describable.DescribableModel.buildArguments(DescribableModel.java:409)
	     at org.jenkinsci.plugins.structs.describable.DescribableModel.coerce(DescribableModel.java:492)

My old @DataBoundConstructor looked like:
@DataBoundConstructor
public AnsibleTowerStep(@Nonnull String towerServer, Boolean importTowerLogs, Boolean param2, ...) {

My new constructor needs to be:
@DataBoundConstructor
public AnsibleTowerStep(
@Nonnull String towerServer, String importTowerLogs, Boolean param2, ...) {

I tried having two @DataBoundConstructors, one for the old Boolean and one for the String, but this caused an exception when compiling the plugin:
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.8.1:compile (default-compile) on project ansible-tower: Compilation failure
[ERROR] javax.annotation.processing.FilerException: Attempt to reopen a file for path /Users/jowestco/IdeaProjects/ansible-tower-plugin/target/classes/org/jenkinsci/plugins/ansible_tower/AnsibleTowerStep.stapler
[ERROR] at com.sun.tools.javac.processing.JavacFiler.checkFileReopening(JavacFiler.java:535)
[ERROR] at com.sun.tools.javac.processing.JavacFiler.createResource(JavacFiler.java:431)
[ERROR] at org.kohsuke.stapler.jsr269.AbstractProcessorImpl.createResource(AbstractProcessorImpl.java:81)


After that I tried to have a constructor (not decorated with @DataBoundConstructor) that would take a Boolean instead of a String and would call my DataBoundConstructor like:
public AnsibleTowerStep(@Nonnull String towerServer, Boolean importTowerLogs, Boolean param2, ...) { this(towerServer, importTowerLogs.toString(), param2…); }
But the pipelines were unable to find and use this constructor and threw the same stack trace as above.

As another solution, I also tried to change the constructor to just an Object like:
@DataBoundConstructor
public AnsibleTowerStep(@Nonnull String towerServer, Object importTowerLogs, Boolean param2, ...) {
With this the groovy scripts worked and I was able to use importTowerLogs.toString() to get a string for either a boolean or string object. Unfortunately, with this solution, the pipeline syntax generator would raise an exception that it couldn’t convert a string param into an object..

Does anyone have any other ideas on how I may be able to achieve what I am trying to do seamlessly (change a boolean field to a string)? Or do I need to tell my users that the new version of my plugin requires changing groovy scripts from 'importTowerLogs = true’ to ‘importTowerLogs = ‘true”’?
If making users update groovy scripts is my only solution, what are the recommend way(s) to warn users about this change? Obviously I will put it in the release nodes and spike it out somewhere in my README.md file as well; but is there anything else I can/should do? i.e. can I mark the new version of the plugin as using an incomparable syntax with the old version?

Thanks in advance,

-John

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/DDE53530-48EF-4910-A085-2A8F695BB525%40redhat.com.

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CAG%3D_DusrOaR%2BJ-S2WdDgg-bT-VHce%3DbuvHasuzRgLVvbu70A6w%40mail.gmail.com.


--

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CAPiUgVdMVoF4sC7VNMcPfpg2yQEW0Cc_yHYazHgK5Va3DDywqw%40mail.gmail.com.
Reply | Threaded
Open this post in threaded view
|

Re: Changing a plugin parameter type

John Westcott IV
In reply to this post by John Westcott
Thanks for your responses Gavin Mogan and slide.

When I tried to remove the parameter from the @DataBoundConstructor and added a second @DataBoundSetter like:
    @DataBoundSetter
public void setImportTowerLogs(Boolean importTowerLogs) { this.importTowerLogs = importTowerLogs.toString(); }
@DataBoundSetter
public void setTImportTowerLogs(String importTowerLogs) { this.importTowerLogs = importTowerLogs; }

both pipelines worked with the constructor but only the second @DataBoundConstructor would be honored.

To solve my problem I removed the parameter from the @DataBoundConstructor and then replaced the old Boolean importTowerLogs with String towerLogLevel with the exception that I left the @DataBoundConstructor for importTowerLog as:
      @DataBoundSetter
   public void setImportTowerLogs(Boolean importTowerLogs) { this.towerLogLevel = importTowerLogs.toString(); }

Things now seem to work fine for both the old boolean parameter like importTowerLogs = {true|false} or with a new string parameter like towerLogLevel = {'true'|'false'|'vars'|'full'}.

-John

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/d264323c-7a8d-48a4-af50-a526a46633ddo%40googlegroups.com.
Reply | Threaded
Open this post in threaded view
|

Re: Changing a plugin parameter type

Tim Van Holder
Note that if there is any compiled code (e.g. other plugins) using your class, that constructor change will break.

I would recommend keeping the old constructor signature (as @Deprecated and not @DataBoundConstructor); it can just forward to the new constructor and call the setter.

(I also wonder why you had the old field as Boolean but then do not handle null.)

On Fri, 5 Jun 2020 at 15:29, John Westcott IV <[hidden email]> wrote:
Thanks for your responses Gavin Mogan and slide.

When I tried to remove the parameter from the @DataBoundConstructor and added a second @DataBoundSetter like:
    @DataBoundSetter
public void setImportTowerLogs(Boolean importTowerLogs) { this.importTowerLogs = importTowerLogs.toString(); }
@DataBoundSetter
public void setTImportTowerLogs(String importTowerLogs) { this.importTowerLogs = importTowerLogs; }

both pipelines worked with the constructor but only the second @DataBoundConstructor would be honored.

To solve my problem I removed the parameter from the @DataBoundConstructor and then replaced the old Boolean importTowerLogs with String towerLogLevel with the exception that I left the @DataBoundConstructor for importTowerLog as:
      @DataBoundSetter
   public void setImportTowerLogs(Boolean importTowerLogs) { this.towerLogLevel = importTowerLogs.toString(); }

Things now seem to work fine for both the old boolean parameter like importTowerLogs = {true|false} or with a new string parameter like towerLogLevel = {'true'|'false'|'vars'|'full'}.

-John

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/d264323c-7a8d-48a4-af50-a526a46633ddo%40googlegroups.com.

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/CAKMi--BSSHxEg4nin-6XXQF_8E55G%3DU--fbiSCEd4H8aXqRhhA%40mail.gmail.com.