Time to get rid of this. :-)

This should be very simple. Let's fix it for 7.4 please.

Sure, it's on my list but I need to fix 7.3.1 problems first.

Created attachment 136037 [details]
Sketch of possible solution

I've done following things within the patch:

- web.clientproject does not expose any friend packages anymore

- most of the constants were moved to the web.clientproject.api (some of them are shared by both client modules, some of them are used only by web.clientproject at the moment - not sure if I should split them or let them together in one class)

- package web.clientproject.api.project were created and contains newly created class ClientSideProjectGenerator which encapsulate most of the code needed by web.clientproject.samples (could be simply used also by web.clientproject in the future)

- SiteHelper and OnlineSites were moved from web.clientproject module to newly created package called web.clientproject.api.project.sites (both classes are used in both clients web.clientproject and web.clientproject.samples)

- two utility classes were moved from web.clientproject to web.clientproject.api module (these are FileUtilities and ValidationUtilities)

Feel free to comment-review the patch. Thanks!

I got about 7 reject files while trying to apply the patch but I can still read the diff. I think it is good. There is one thing which I'm not sure about though: moving most of the project creation logic and constants outside of clientside project. Could we keep ClientSideProjectGenerator directly in web.clientproject module in some new API package? That way most of the internal code for project creation would stay in web.clientproject and there would be also no need to put all the constants into the API either (apart from perhaps ClientSideProjectConstants.DEFAULT_* - but these could be hardcoded in samples module). I would probably rename ClientSideProjectGenerator.ProjectProperties into ClientSideProjectGenerator.CreateProjectProperties just to avoid confusion with SiteTemplateImplementation.ProjectProperties. The rest seems to be OK. Moving most of the Util classes into API should be OK. What do you think Tomas - you authored many of those util classes.

Btw. your OnlineSampleWizardIterator could be simplified. It does not have to use OnlineSites at all - all it needs is to call SiteHelper.download() on your URL followed by SiteHelper.unzipProjectTemplate(). OnlineSites do not provide that much help here I think and instantiate() could be more straightforward - no need to call siteTemplate.configure() which does nothing anyway etc.

If I could apply your diff I would tweak it myself but unfortunately it failed.

David's comments make sense to me (BTW Davide I thought that you don't want any API in clientside project module at all).

Martine, be careful when applying your patch against trunk - you e.g. use 1.34 version for HTML5 API/SPI but in trunk, there is already 1.36. It is always good to make these (API) changes using mq [1] so they can be easily refreshed. If you have already commited your changes, you could use histedit extension [2] but mq is much more comfortable and powerful, of course.

Thanks a lot for doing that!
[1] http://wiki.netbeans.org/HgHowTos#Develop_API_review_patches_using_MQ
[2] http://mercurial.selenic.com/wiki/HisteditExtension

(In reply to comment #5)
> I got about 7 reject files 

Ye, this is probably because I started the work about month ago then I stopped and finally get back to it yesterday, but the local repository is quite obsolete now. I'll update the patch with respect to the latest sources.

> There is one thing which I'm not sure about
> though: moving most of the project creation logic and constants outside of
> clientside project. Could we keep ClientSideProjectGenerator directly in
> web.clientproject module in some new API package?

Guess I can do that but I thought the whole point of this issue was to get rid of the API classes from web.clientproject :)

> there would be
> also no need to put all the constants into the API either (apart from perhaps
> ClientSideProjectConstants.DEFAULT_*

If public constants are the problem I can split them to the public part (only about half of them are needed in both client module) and private part that will stay in web.clientproject
Initially I moved them all together to make finding existing constants easier but everything is good to me ;)

> I would probably rename ClientSideProjectGenerator.ProjectProperties
> into ClientSideProjectGenerator.CreateProjectProperties just to avoid confusion
> with SiteTemplateImplementation.ProjectProperties. 

Sure, make sense.

> Btw. your OnlineSampleWizardIterator could be simplified. It does not have to
> use OnlineSites at all - all it needs is to call SiteHelper.download() on your
> URL followed by SiteHelper.unzipProjectTemplate(). OnlineSites do not provide
> that much help here I think and instantiate() could be more straightforward -
> no need to call siteTemplate.configure() which does nothing anyway etc.

Ok, not sure now how the implementation would look like at the end. I'll take a look at it..

Created attachment 136076 [details]
Sketch of possible solution - v2

Patch v2 updated:

- Should be possible to import it without rejected files (--> strong feature:))
- ClientSideProjectGenerator.ProjectProperties renamed to ClientSideProjectGenerator.CreateProjectProperties
- Constants were split and API module now contains only the subset that is needed by clients

Whats still on my list:

- I'm going to remove OnlineSites from API module
- I'm going to change usages of deprecated SiteHelper.download to NetworkSupport.download

--> please feel free to comment anything else from the patch;)

> BTW Davide I thought that you don't want any
> API in clientside project module at all

That was my first thought and overall is still the goal - module which implements a project type sits (in an architecture diagram) on top and integrates modules providing individual features.

Project samples can be implemented in many way. One way is to create an HTML5 project and preconfigured it and ZIP it as a project template; in runtime you then unzip it, update project name and download some content into its public_html folder. In such scenario you do not need much APIs. I guess this is how I imagined it to end up working in the end. But it does have drawbacks too.

A different solution is to use APIs for project creation which does have few advantages. However, in such scenario I would rather break my previous resolution and allow clientside project to have an API. It is lesser evil and I accept that as compromise. For an example see web.project which does the same: org.netbeans.modules.web.project.api.WebProjectCreateData and org.netbeans.modules.web.project.api.WebProjectUtilities which is used from EAR project.

I'm going to play with the patch a bit.

Created attachment 136106 [details]
alternative patch

The patch compiles but I have not tried to run it. The IDE refused to refactor and move constants back to the clientside module so I had to start from a scratch just with ProjectGenerator class. I dropped OnlineSites from the API and rewrote the code. FileUtilities are not needed in the API either. The ClientSideProjectGenerator intentionally returns only Project unless there is strong need for AntProjectHelper. I wonder whether ClientSideProjectGenerator is complete or something else is needed there to be performed to complete project creation? Tomas?

David's patch seems fine to me (I did not test it though). Yes, ProjectGenerator has to return just Project and not AntProjectHelper. Moreover, this project must be fully configured according the given properties - this ensures that every new HTML5 project is correct, for every client.

Thanks.

(In reply to comment #11)
> I wonder whether ClientSideProjectGenerator
> is complete or something else is needed there to be performed to complete
> project creation? Tomas?

It seems to me to be complete, however:

TM01: CreateProjectProperties.startFile is not used by ClientSideProjectGenerator - please use it or remove it from properties completely.

TM02: CreateProjectProperties.encoding - the same case as TM01.

TM03: Avoid casting Project to ClientSideProject (see ClientSideProjectGenerator) - lookup it and then check (similarly like ClientSideProjectUtilities.setupProject() does).

One more comment:

TM04: Martine, please add Parameters.notNull(...) and similar to public API methods that do not accept nulls, empty string etc. (and consider adding @NonNull, @NullAllowed etc. annotations).

Thanks.

Thanks for the alternative patch Davide and thanks for valuable comments Tomasi. I have changed the code with respect to the TM01, TM02, TM03 and TM04. Could you guys take a look at it one more time? If it will be OK, I would like to integrate the patch tomorrow.

Created attachment 136224 [details]
Alternative patch - v2

Includes TM01, TM02, TM03 and TM04

Looks OK to me. Thx.

The patch is fine, IMHO. Just please add @since to new API classes/methods.

Thanks.

Created attachment 136247 [details]
Final patch

--> @since annotation added
--> JavaDoc improved
--> ProjectManager.getDefault().clearNonProjectCache(); line removed

I'm going to test the patch a little bit and push the changes later today.

Fixed in: web-main #f4164181904c
Thank you for your help guys!

Integrated into 'main-golden', will be available in build *201306252301* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main-golden/rev/f4164181904c
User: Martin Janicek <mjanicek@netbeans.org>
Log: #222525 - Get rid of the web.clientproject exported API