You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by Radu Cotescu <ra...@apache.org> on 2019/07/04 15:00:02 UTC

[org.apache.sling.jcr.contentparser] can we decouple it from JCR?

Hi Stefan,

I do know that the module has JCR in its name, but do you think that we could make the JCR stuff optional? I’d really like to reuse this module to parse JSON into Sling resource trees, but I’d like the JCR dependencies to be optional, since in my experimental setup I use a different resource provider.

Thanks,
Radu

Re: [org.apache.sling.jcr.contentparser] can we decouple it from JCR?

Posted by Radu Cotescu <ra...@apache.org>.
And also sling-org-apache-sling-contentparser-testutils.

> On 22 Jul 2019, at 09:45, Radu Cotescu <ra...@apache.org> wrote:
> 
> I will create the following repositories in the meantime:
> 
> sling-org-apache-sling-contentparser-api
> sling-org-apache-sling-contentparser-json
> sling-org-apache-sling-contentparser-xml
> sling-org-apache-sling-contentparser-xml-jcr


Re: [org.apache.sling.jcr.contentparser] can we decouple it from JCR?

Posted by Radu Cotescu <ra...@apache.org>.
Hi Jason,

> On 19 Jul 2019, at 23:05, Jason E Bailey <ja...@24601.org> wrote:
> 
> My opinion is that the old API will need to be deprecated for the new one. And semantically the new one should be 1.0.0

We will definitely deprecate the old API and reference the new one in the module’s README file. At the same time I partially understand Stefan’s concerns, and given that it’s just a number I’m fine with releasing o.a.s.contentparser.api with version 2.0.0.

I will create the following repositories in the meantime:

sling-org-apache-sling-contentparser-api
sling-org-apache-sling-contentparser-json
sling-org-apache-sling-contentparser-xml
sling-org-apache-sling-contentparser-xml-jcr

Thanks,
Radu


Re: [org.apache.sling.jcr.contentparser] can we decouple it from JCR?

Posted by Jason E Bailey <ja...@24601.org>.
Okay, I'm going to counter this argument. The new content parser is a different bundle, a different import, then the old content parser.

the old content parser package is 
org.apache.sling.jcr.contentparser

That can and will co-exist with 
org.apache.sling.contentparser

My opinion is that the old API will need to be deprecated for the new one. And semantically the new one should be 1.0.0


--
Jason

On Fri, Jul 19, 2019, at 11:51 AM, Radu Cotescu wrote:
> Hi Stefan,
> 
> > On 19 Jul 2019, at 15:20, Stefan Seifert <ss...@pro-vision.de> wrote:
> > 
> > i think the bundle users will not be really aware oft the removal of the tiny "jcr" part in the package name, for them it's only "the content parser".
> > and if the new version after 1.2.6 is 1.0.0 has the same features as 1.2.6 but a different API, this gets very puzzling.
> > 
> > stefan
> > 
> 
> Ok, then let’s start everything here with version 2.0.0 - all bundle 
> versions, all exported packages.
> 
> Cheers,
> Radu

Re: [org.apache.sling.jcr.contentparser] can we decouple it from JCR?

Posted by Radu Cotescu <ra...@apache.org>.
Hi Stefan,

> On 19 Jul 2019, at 15:20, Stefan Seifert <ss...@pro-vision.de> wrote:
> 
> i think the bundle users will not be really aware oft the removal of the tiny "jcr" part in the package name, for them it's only "the content parser".
> and if the new version after 1.2.6 is 1.0.0 has the same features as 1.2.6 but a different API, this gets very puzzling.
> 
> stefan
> 

Ok, then let’s start everything here with version 2.0.0 - all bundle versions, all exported packages.

Cheers,
Radu

RE: [org.apache.sling.jcr.contentparser] can we decouple it from JCR?

Posted by Stefan Seifert <ss...@pro-vision.de>.
i think the bundle users will not be really aware oft the removal of the tiny "jcr" part in the package name, for them it's only "the content parser".
and if the new version after 1.2.6 is 1.0.0 has the same features as 1.2.6 but a different API, this gets very puzzling.

stefan

>-----Original Message-----
>From: Radu Cotescu [mailto:radu@apache.org]
>Sent: Friday, July 19, 2019 3:17 PM
>To: Sling Dev
>Subject: Re: [org.apache.sling.jcr.contentparser] can we decouple it from
>JCR?
>
>Hi Stefan,
>
>> On 19 Jul 2019, at 12:36, Stefan Seifert <ss...@pro-vision.de> wrote:
>>
>> in my pov the new modules should get version 2.0.0 as start version, as
>they are a successor of the old content parser with the same features, just
>bundled in a new way.
>
>I know it’s just a number, but does it really make sense? We anyways
>changed the package name so consumers of the new API will have to perform a
>minor refactoring when migrating from the previous one.
>
>Thanks,
>Radu

Re: [org.apache.sling.jcr.contentparser] can we decouple it from JCR?

Posted by Radu Cotescu <ra...@apache.org>.
Hi Stefan,

> On 19 Jul 2019, at 12:36, Stefan Seifert <ss...@pro-vision.de> wrote:
> 
> in my pov the new modules should get version 2.0.0 as start version, as they are a successor of the old content parser with the same features, just bundled in a new way.

I know it’s just a number, but does it really make sense? We anyways changed the package name so consumers of the new API will have to perform a minor refactoring when migrating from the previous one.

Thanks,
Radu

RE: [org.apache.sling.jcr.contentparser] can we decouple it from JCR?

Posted by Stefan Seifert <ss...@pro-vision.de>.
+1 with the latest changes from https://github.com/apache/sling-whiteboard/pull/47

in my pov the new modules should get version 2.0.0 as start version, as they are a successor of the old content parser with the same features, just bundled in a new way.

stefan


>-----Original Message-----
>From: Radu Cotescu [mailto:radu@apache.org]
>Sent: Thursday, July 4, 2019 5:00 PM
>To: Sling Dev
>Cc: Stefan Seifert
>Subject: [org.apache.sling.jcr.contentparser] can we decouple it from JCR?
>
>Hi Stefan,
>
>I do know that the module has JCR in its name, but do you think that we
>could make the JCR stuff optional? I’d really like to reuse this module to
>parse JSON into Sling resource trees, but I’d like the JCR dependencies to
>be optional, since in my experimental setup I use a different resource
>provider.
>
>Thanks,
>Radu

Re: [org.apache.sling.jcr.contentparser] can we decouple it from JCR?

Posted by Radu Cotescu <ra...@apache.org>.
https://github.com/apache/sling-whiteboard/pull/47

> On 17 Jul 2019, at 20:10, Radu Cotescu <ra...@apache.org> wrote:
> 
> Duly noted. I’ll try to implement these changes tomorrow.


Re: [org.apache.sling.jcr.contentparser] can we decouple it from JCR?

Posted by Radu Cotescu <ra...@apache.org>.
Hi Jason,

> On 17 Jul 2019, at 15:16, Jason E Bailey <ja...@24601.org> wrote:
> 
> My opinion - I'd prefer a greater decoupling of the API and implementations. We should be able to design this in such a way that the addition of a new format for parsing content doesn't necessitate a release of the API. To that end, I think the ParserOptions should not be a final class but encompass the generic options available to all content parsers and then have extended classes in the other packages that reflect the ParserOptions for that package.
> 
> So there'd be a JsonParserOptions which extends ParserOptions and contains json specific options.
> 
> So for me to implement a YamlContentParser I should just be able to implement the appropriate classes and install an implementation package.
> 
> That would also mean moving implementation specific files to their respective packages as well, like the JsonParserFeatures
> 
> --
> Jason
> 

Duly noted. I’ll try to implement these changes tomorrow.

Cheers,
Radu

Re: [org.apache.sling.jcr.contentparser] can we decouple it from JCR?

Posted by Jason E Bailey <ja...@24601.org>.
My opinion - I'd prefer a greater decoupling of the API and implementations. We should be able to design this in such a way that the addition of a new format for parsing content doesn't necessitate a release of the API. To that end, I think the ParserOptions should not be a final class but encompass the generic options available to all content parsers and then have extended classes in the other packages that reflect the ParserOptions for that package.

So there'd be a JsonParserOptions which extends ParserOptions and contains json specific options.

So for me to implement a YamlContentParser I should just be able to implement the appropriate classes and install an implementation package.

That would also mean moving implementation specific files to their respective packages as well, like the JsonParserFeatures

--
Jason

On Wed, Jul 17, 2019, at 4:02 AM, Radu Cotescu wrote:
> https://github.com/apache/sling-whiteboard/tree/master/contentparser/org-apache-sling-contentparser-api
> 
> > On 16 Jul 2019, at 16:41, Radu Cotescu <ra...@apache.org> wrote:
> > 
> > Sure, I’ll prepare a README.md file for each module.
> 
>

Re: [org.apache.sling.jcr.contentparser] can we decouple it from JCR?

Posted by Radu Cotescu <ra...@apache.org>.
https://github.com/apache/sling-whiteboard/tree/master/contentparser/org-apache-sling-contentparser-api

> On 16 Jul 2019, at 16:41, Radu Cotescu <ra...@apache.org> wrote:
> 
> Sure, I’ll prepare a README.md file for each module.


Re: [org.apache.sling.jcr.contentparser] can we decouple it from JCR?

Posted by Radu Cotescu <ra...@apache.org>.
Hi Robert,

> On 16 Jul 2019, at 13:13, Robert Munteanu <ro...@apache.org> wrote:
> 
> Could you add a short description of what changed, what was added, what
> was removed (if anything was removed)?

Sure, I’ll prepare a README.md file for each module.

Thanks,
Radu

Re: [org.apache.sling.jcr.contentparser] can we decouple it from JCR?

Posted by Robert Munteanu <ro...@apache.org>.
Hi Radu,

On Mon, 2019-07-15 at 13:56 +0200, Radu Cotescu wrote:
> Hi,
> 
> > On 8 Jul 2019, at 22:20, Radu Cotescu <ra...@apache.org> wrote:
> > 
> > Thanks for the +1s. I’ll start working in the whiteboard and hope
> > to have everything ready for review by the end of the week.
> 
> The code is now available at [0]. Let me know if you have any
> objections or feedback that I should incorporate before moving the
> modules to their own git repositories.

Could you add a short description of what changed, what was added, what
was removed (if anything was removed)?

Thanks!

Robert


Re: [org.apache.sling.jcr.contentparser] can we decouple it from JCR?

Posted by Radu Cotescu <ra...@apache.org>.
Hi,

> On 8 Jul 2019, at 22:20, Radu Cotescu <ra...@apache.org> wrote:
> 
> Thanks for the +1s. I’ll start working in the whiteboard and hope to have everything ready for review by the end of the week.

The code is now available at [0]. Let me know if you have any objections or feedback that I should incorporate before moving the modules to their own git repositories.

Thanks,
Radu

[0] - https://github.com/apache/sling-whiteboard/tree/master/contentparser/

Re: [org.apache.sling.jcr.contentparser] can we decouple it from JCR?

Posted by Radu Cotescu <ra...@apache.org>.
Thanks for the +1s. I’ll start working in the whiteboard and hope to have everything ready for review by the end of the week.

Re: [org.apache.sling.jcr.contentparser] can we decouple it from JCR?

Posted by Nicolas Peltier <np...@apache.org>.
+1

Le ven. 5 juil. 2019 à 19:49, Julian Sedding <js...@gmail.com> a écrit :

> +1 I like your idea!
>
> I vaguely remember having similar thoughts in the past, but considered
> it too much effort for what I was after at the time.
>
> Regards
> Julian
>
> On Fri, Jul 5, 2019 at 3:41 PM Jason E Bailey <ja...@24601.org>
> wrote:
> >
> > +1 for this idea
> >
> > --
> > Jason
> >
> > On Thu, Jul 4, 2019, at 11:55 AM, Radu Cotescu wrote:
> > > Let me add more details. Three classes use JCR or JackRabbit
> dependencies:
> > >
> > > JcrXmlContentParser
> > > JcrXmlValueConverter
> > > XmlContentParser
> > > ParserHelper
> > >
> > > What I’d suggest would be to separate the API from the implementation
> > > and then provide the parsers from different pluggable modules. In
> > > addition to that, I think that the ParserHelper could maybe be exposed
> > > in the API as a final class, much like the ContentParserFactory, and
> > > use a different Calendar parser instead of
> > > org.apache.jackrabbit.util.ISO8601.
> > >
> > > I could prepare a PR if you agree with the changes.
> > >
> > > Thanks,
> > > Radu
> > >
> > > > On 4 Jul 2019, at 17:00, Radu Cotescu <ra...@apache.org> wrote:
> > > >
> > > > Hi Stefan,
> > > >
> > > > I do know that the module has JCR in its name, but do you think that
> we could make the JCR stuff optional? I’d really like to reuse this module
> to parse JSON into Sling resource trees, but I’d like the JCR dependencies
> to be optional, since in my experimental setup I use a different resource
> provider.
> > > >
> > > > Thanks,
> > > > Radu
> > >
> > >
>

Re: [org.apache.sling.jcr.contentparser] can we decouple it from JCR?

Posted by Julian Sedding <js...@gmail.com>.
+1 I like your idea!

I vaguely remember having similar thoughts in the past, but considered
it too much effort for what I was after at the time.

Regards
Julian

On Fri, Jul 5, 2019 at 3:41 PM Jason E Bailey <ja...@24601.org> wrote:
>
> +1 for this idea
>
> --
> Jason
>
> On Thu, Jul 4, 2019, at 11:55 AM, Radu Cotescu wrote:
> > Let me add more details. Three classes use JCR or JackRabbit dependencies:
> >
> > JcrXmlContentParser
> > JcrXmlValueConverter
> > XmlContentParser
> > ParserHelper
> >
> > What I’d suggest would be to separate the API from the implementation
> > and then provide the parsers from different pluggable modules. In
> > addition to that, I think that the ParserHelper could maybe be exposed
> > in the API as a final class, much like the ContentParserFactory, and
> > use a different Calendar parser instead of
> > org.apache.jackrabbit.util.ISO8601.
> >
> > I could prepare a PR if you agree with the changes.
> >
> > Thanks,
> > Radu
> >
> > > On 4 Jul 2019, at 17:00, Radu Cotescu <ra...@apache.org> wrote:
> > >
> > > Hi Stefan,
> > >
> > > I do know that the module has JCR in its name, but do you think that we could make the JCR stuff optional? I’d really like to reuse this module to parse JSON into Sling resource trees, but I’d like the JCR dependencies to be optional, since in my experimental setup I use a different resource provider.
> > >
> > > Thanks,
> > > Radu
> >
> >

Re: [org.apache.sling.jcr.contentparser] can we decouple it from JCR?

Posted by Jason E Bailey <ja...@24601.org>.
+1 for this idea

--
Jason

On Thu, Jul 4, 2019, at 11:55 AM, Radu Cotescu wrote:
> Let me add more details. Three classes use JCR or JackRabbit dependencies:
> 
> JcrXmlContentParser
> JcrXmlValueConverter
> XmlContentParser
> ParserHelper
> 
> What I’d suggest would be to separate the API from the implementation 
> and then provide the parsers from different pluggable modules. In 
> addition to that, I think that the ParserHelper could maybe be exposed 
> in the API as a final class, much like the ContentParserFactory, and 
> use a different Calendar parser instead of 
> org.apache.jackrabbit.util.ISO8601.
> 
> I could prepare a PR if you agree with the changes.
> 
> Thanks,
> Radu
> 
> > On 4 Jul 2019, at 17:00, Radu Cotescu <ra...@apache.org> wrote:
> > 
> > Hi Stefan,
> > 
> > I do know that the module has JCR in its name, but do you think that we could make the JCR stuff optional? I’d really like to reuse this module to parse JSON into Sling resource trees, but I’d like the JCR dependencies to be optional, since in my experimental setup I use a different resource provider.
> > 
> > Thanks,
> > Radu
> 
>

Re: [org.apache.sling.jcr.contentparser] can we decouple it from JCR?

Posted by Radu Cotescu <ra...@apache.org>.
Let me add more details. Three classes use JCR or JackRabbit dependencies:

JcrXmlContentParser
JcrXmlValueConverter
XmlContentParser
ParserHelper

What I’d suggest would be to separate the API from the implementation and then provide the parsers from different pluggable modules. In addition to that, I think that the ParserHelper could maybe be exposed in the API as a final class, much like the ContentParserFactory, and use a different Calendar parser instead of org.apache.jackrabbit.util.ISO8601.

I could prepare a PR if you agree with the changes.

Thanks,
Radu

> On 4 Jul 2019, at 17:00, Radu Cotescu <ra...@apache.org> wrote:
> 
> Hi Stefan,
> 
> I do know that the module has JCR in its name, but do you think that we could make the JCR stuff optional? I’d really like to reuse this module to parse JSON into Sling resource trees, but I’d like the JCR dependencies to be optional, since in my experimental setup I use a different resource provider.
> 
> Thanks,
> Radu