Re: Exemplar configuration thoughts

On 1/29/07, Kohsuke Kawaguchi <Kohsuke.Kawaguchi@xxxxxxx> wrote:

> Now if you took getDescriptor() away, you suddenly don't have the problem of
> requiring a singleton. Any Descriptor could make the ExtensionPoint. You
> only need to add one to the Hudson instance, and I think that would be it.
> There's not a whole a lot of getDescriptor() that seemed to be needed, so
> this seemed simpler to me since the circular dependency is gone, and the
> singleton requirement is gone.

I guess this is where I need to understand you better. For example,
today I need descriptors for listing up the kind of Jobs that are
installed.

 

I'm really not that solid on my jelly skills yet, so some of what happens between the XML and the Java is a bit of a mystery but I'll try and show you what I was thinking of.

To list up the various kinds of Jobs,

// Not-the-current-APIs, just making what I'm doing clear

// Create a Hudson instance
Hudson h = new Hudson();

// Give it the JobDescriptors
h.addJobDescriptor ( new JPRTJobDescriptor() );

h.addJobDescriptor( new ExternalJobDescriptor() );
h.addJobDescriptor( new HudsonJobDescriptor() );

// When the jelly page that needs the job specific descriptors is created,
// whatever it is that gets things into jelly could hand it this,

Descriptor[] jobDescriptors = h.getJobDescriptors();

I don't need to have an existing Job to get a list of all the Descriptors that I know about.

If I remove getDescriptor(), how do I do that? and how do I
create a new Job instance?

The descriptors could still be used to instantiate the new Job instance.
I don't need to have an existing Job to make a brand new one.

// I'm not sure how I get the StaplerRequest in exact detail, but someone
// clicks the New Job, and the selected descriptor from the list that we
// gave the jelly page earlier is used

StaplerRequest req = ...;
Descriptor jobDescriptor = ...;
jobDescriptor.newInstance(req);

The Job instances themselves could be used to copy an existing Job using clone().
I do need a Job to create a copy but I don't need the Descriptor.

Or once loaded, the global configuration (like where Ant is installed)
is stored in Ant's Descriptor object. If you get rid of that, where do
you keep it, in a way easy to persist?

I can still share a state between these objects without a getDescriptor()

// Put the AntBuilder into Hudsone
h.addBuilderDescriptor( new HudsonAntDescriptor() );

class Ant extends ... {

  private final AntInstallation[] installations;

  Ant(AntInstallation[] installations) {
    this.installations = installations;
  }

  // ...

}

class AntDescriptor extends ... {
  
  private AntInstallation[] installations = ...;
 
  // ...

  public Builder newInstance(StaplerRequest req) {
    return new Ant(installations);
  }

  // ...

}

I'm not totally familar with XStream, so I'm not sure if this would store a duplicate - so maybe one of these fields needs to be marked transient. This is more of an XStream detail though and less what I'm trying to illustrate. If I need the semantics to be that the list of AntInstallations is shared and not copied, then I can share a j.u.l.c.CopyOnWriteArrayList.

When the global config page is displayed, we'd collect from the Hudson instance the arrays of the various Descriptor types and stick them into the jelly page.

The addExtensionPoint methods I was using on the Hudson instance to register the Descriptors actually might add to a HashSet for each type of Descriptor, eliminating duplicate Descriptors.

> This was something I was starting to think about and I think I would be
> inclined to reduce the coupling between those two classes. That's just me
> though. Ultimately its your project, (which is great so far btw), so its up
> to you.

I really appreciate all your comments.

I'd like to get more contributions, so I don't want to cut a discussion
by declaring "I'll do it this way because this is mine!"


--
Kohsuke Kawaguchi
Sun Microsystems                   kohsuke.kawaguchi@xxxxxxx

-- 

- Eric