Plugin compatibility when adding a class

[Christian McHugh]

Greetings community,

Earlier versions of the saltstack plugin were pretty ugly and didn't use a class for the different options in a dropdown selector, but instead just parsed the json in the databoundconstructor. Thankfully, this has been corrected, but now brings up the issue of how to support older configs.

I have the following code added to readResolve() or

public static class ConverterImpl extends XStream2.PassthruConverter<SaltAPIBuilder> {
    public ConverterImpl(XStream2 xstream) { super(xstream); }
    @Override
    protected void callback(SaltAPIBuilder obj, UnmarshallingContext context) {

        // Support 1.7 and before
        if (obj.clientInterfaces != null) {  
            obj.arguments = obj.arguments.replaceAll(",", " ");
            if (obj.clientInterfaces.has("clientInterface")) {
                if (obj.clientInterfaces.getString("clientInterface").equals("local")) {
                    obj.clientInterface = new LocalClient(obj.function, obj.arguments + " " + obj.kwarguments, obj.target, obj.targettype);
                    ((LocalClient) obj.clientInterface).setJobPollTime(obj.clientInterfaces.getInt("jobPollTime"));
                    ((LocalClient) obj.clientInterface).setBlockbuild(obj.clientInterfaces.getBoolean("blockbuild"));
                } else if (obj.clientInterfaces.getString("clientInterface").equals("local_batch")) {
                    obj.clientInterface = new LocalBatchClient(obj.function, obj.arguments + " " + obj.kwarguments, obj.batchSize, obj.target, obj.targettype);
                } else if (obj.clientInterfaces.getString("clientInterface").equals("runner")) {
                    obj.clientInterface = new RunnerClient(obj.function, obj.arguments + " " + obj.kwarguments, obj.mods, obj.pillarvalue);
                }
            }
            final XStream xStream = new XStream(new DomDriver());
            final String xmlResult = xStream.toXML(obj).toString();
            System.out.println("!!!!!! Running ConverterImpl\n" + xmlResult);

            OldDataMonitor.report(context, "1.7.2");
       }
    }
}

This code attempts to parse the old format, and creates the appropriate new classes. In the above, the system.out.println result shows the expected output, but unfortunately there's a problem.

When viewing a job with an older config, nothing is displayed. When viewing the manage old data page the following error is shown:

Type	Name	Version
hudson.model.FreeStyleProject	job1.7.1	1.7.2	ConversionException: Cannot construct com.waytta.clientinterface.BasicClient : com.waytta.clientinterface.BasicClient : Cannot construct com.waytta.clientinterface.BasicClient : com.waytta.clientinterface.BasicClient ---- Debugging information ---- message : Cannot construct com.waytta.clientinterface.BasicClient : com.waytta.clientinterface.BasicClient cause-exception : com.thoughtworks.xstream.converters.reflection.ObjectAccessException cause-message : Cannot construct com.waytta.clientinterface.BasicClient : com.waytta.clientinterface.BasicClient class : com.waytta.clientinterface.BasicClient required-type : com.waytta.clientinterface.BasicClient converter-type : hudson.util.RobustReflectionConverter path : /project/builders/com.waytta.SaltAPIBuilder/clientInterface line number : 52 -------------------------------, InstantiationError: null

Does anyone have a recomendation on how to support this migration and creating a class from the ConverterImpl?

Thanks :)

[Stephen Connolly]

Sometimes it is just simpler to move to a new implementation class entirely. You keep Just Enough™ of the old class around that it can be read from disk... and then you add a `readResolve` method that instantiates and returns the new class

Here is a very old example of doing this for a complex migration: https://github.com/jenkinsci/cloudbees-deployer-plugin/blob/master/src/main/java/org/jenkins/plugins/cloudbees/CloudbeesPublisher.java#L102

--
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 jenkinsci-dev+unsubscribe@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/3825faa2-a769-4a81-9c92-dc0065dea0c8%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Christian McHugh]

Great example, thanks!

Let's say that a release had already been made which changed the databoundconstructor. In that case, the constructor's api is already modified. In this theoretical example, is there a mechanism by which the job config.xml could be updated to match the new constructor layout before the initialization call? 

[Robert Sandell]

The XStream deserialization from config.xml to the object graph don't use the DataboundConstructor et.al. It is only interested in mapping xml data to fields (in most cases).

DataboundConstructor is only used for formbinding from the UI and pipeline step definitions etc.

/B

2017-06-16 12:03 GMT+02:00 Christian McHugh <christia...@gmail.com>:

Great example, thanks!

Let's say that a release had already been made which changed the databoundconstructor. In that case, the constructor's api is already modified. In this theoretical example, is there a mechanism by which the job config.xml could be updated to match the new constructor layout before the initialization call? 

--

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 jenkinsci-dev+unsubscribe@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/6d3e9c5d-69a2-4535-8444-2f1dab44f9b2%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--

Robert Sandell

Software Engineer

CloudBees Inc.

[Christian McHugh]

On Friday, June 16, 2017 at 11:12:27 AM UTC+1, Robert Sandell wrote:

The XStream deserialization from config.xml to the object graph don't use the DataboundConstructor et.al. It is only interested in mapping xml data to fields (in most cases).

DataboundConstructor is only used for formbinding from the UI and pipeline step definitions etc.

So a ConverterImpl should be able to update the config properly? At the end of my example from the first part of the thread I had a system.out.println which displays the desired config, but when hitting the convert button in Jenkins, those config.xml files are written without a build step. Would this mean I just need to make a better ConverterImpl? 

[Robert Sandell]

Maybe, or you might just haven't registered the converter in the correct places. But I would urge you to first try using plain old readResolve to handle the conversion from old data before attempting implementing an XStream converter.

--
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 jenkinsci-dev+unsubscribe@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/3db7ba8c-7f22-4e06-88ba-8bb048e0720d%40googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

[Christian McHugh]

I think I have this almost going. The issue seems to be that I changed my "clientInterface" field from a String to a custom object. Even though my readResolve() returns the 'this' object with the new BasicClient type, for any old projects that have the old String field in their jobs/jobname/config.xml, Jenkins complains that the BasicClient class cannot be created. If I remove the String object field from the older jobs/jobname/config.xml, everything works as expected.

So somehow the string included in the config.xml seems to make it past the readResolve. Any thoughts on killing it off earlier?

Thanks again!

[Jesse Glick]

On Fri, Jun 16, 2017 at 7:53 PM, Christian McHugh
<christia...@gmail.com> wrote:
> I changed my
> "clientInterface" field from a String to a custom object.

You cannot change the type of an existing field. You can introduce a
new field with a different type, and have `readResolve` do the data
conversion.