Proposals for version 3

[PatrickGotthard]

Here are some proposals for version 3:

• remove Cloneable
• remove Serializable
• replace reflection (e.g. in equals and hashCode) with explicit code
• replace rome.properties through ServiceLoader
• replace JDOM with Java DOM or StAX
• integrate all modules into "rome" (effectively only rome-modules and rome-opml will be integrated)
• remove URL support from XmlReader
• support for JSR 310 (new Date Time API)
• enhance GraalVM support → see GH-599: Add support for GraalVM native images #636 (comment)

[antoniosanct]

Hi, @PatrickGotthard

Nice to try the first three! Maybe the old JDK DOM migration (or maybe StAX parser!) could be a nice chance to include it. Branch reorganization seems a big problem at this moment for JDK DOM migration in my forked repo (>600 files changed). Step by step.

Regards,
Antonio.

[artembilan]

I would suggest to stay away from Lombok: you develop here a library, not the target application, so better to be as specific with your code as possible.
And less reflection, please! 😄

[neroux]

As I mentioned previously, please make sure you are not breaking Android compatibiity with these changes, otherwise the library would become unusable.

The DOM change was mentioned at some point and the date API may be also an issue, even though I believe this was addressed.

[antoniosanct]

@neroux how could I check if Android or ASOP have Java DOM compatibility?
@PatrickGotthard JSR 310, How could I start? (hehe!)

Regards,
Antonio.

[antoniosanct]

@PatrickGotthard Yesterday I started migrating all Date/Calendar/DateFormat/SimpleDateFormat references by ZonedDateTime/DateTimeFormatter, and I had some problems using Google Base module. I hope I make a new PR ASAP of this feature.

Regards,
Antonio.

[PatrickGotthard]

Hi @antoniosanct,

I'm glad about your commitment, but please wait a little bit. All above are just ideas without digging deeper into the code. Before breaking backwards-compatibility we should discuss it 🙂

Regarding the Android question: just try it out using the official IDE and SDK 😏

Regards,
Patrick

[antoniosanct]

Hi, everyone:

@neroux I found this link about it: https://developer.android.com/reference/org/w3c/dom/package-summary. I think that AOSP has DOM compatibility, but in all Android versions?
@PatrickGotthard I've finished the feature, but I agree with you about discuss this own feature. I have a master_jsr310 branch in my own forked repo to evaluate it.

Regards,
Antonio.

[neroux]

I guess the best way will be to verify as mentioned at #637 (comment). Maybe you can check the date libraries as well, as there may be also compatibility issues.

[antoniosanct]

Hi again!
@neroux @PatrickGotthard I don't have experience with AOSP. Is this test correct?

Regards,
Antonio

[neroux]

The exception is not very descriptive in what it actually complains aboout. I found a bit more on that error at https://stackoverflow.com/questions/41500970/parserconfigurationexception-with-javax-xml-parsers-on-android and it seems to refer to some enabled XML features. Did you enable any of that?

[antoniosanct]

Hi, @neroux
In actual version, ROME create SAXBuilder from JDOM2 and deactivates external general entities. In that case, it needs to deactivate for DOM to main compatibility with test cases:

WireFeed build(final InputSource is) throws IllegalArgumentException, FeedException {
        try {
        	DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
        	dbf.setNamespaceAware(true);
        	dbf.setFeature("http://xml.org/sax/features/external-general-entities", false);
        	dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
        	dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
        	DocumentBuilder db = dbf.newDocumentBuilder();
        	db.setErrorHandler(new DefaultHandler());
		return this.build(db.parse(is));
        } catch (final Exception ex) {
            throw new FeedException("Invalid XML", ex);
        }
    }

Maybe it is my fault by using DOM implementation instead of SAX o StAX implementation. Then I'd like to review again and check if this implementation is correct.

Thanks for your help. Regards,
Antonio.

[antoniosanct]

@PatrickGotthard @neroux
Other question is JDOM has more performance than JDK DOM (almost in my own tests), because SAX has greater performance than DOM (by definition). I need to investigate more about this feature.
Regards,
Antonio.

[antoniosanct]

Hi, everyone:

I keep running into problems integrating ROME on Android with the Java DOM adapted version.

First of all, The "load-external-dtd" feature is mandatory for the tests to run correctly. If I still compile without this feature, I get errors with the StAX library with Android API version 24.

Another problem I have encountered is the Java DOM implementation itself in ROME, since JDOM generates an org.w3c.dom.Document object from a SAXBuilder object. However, from the Java API, if we want to have a Document object, we have to use a DOM parser, which is slower and more expensive in memory.

I was very happy to integrate it, especially because of the lack of JDOM maintenance, but I'm afraid that finally we will not be able to do it (unless we remove Android support, which of course I do not consider at all).

@neroux @PatrickGotthard

Regards,
Antonio.

[neroux]

Thanks for checking. That was my concern, I am afraid, that it wouldn't be straightforward on Android.

[antoniosanct]

My first impressions:

• Serializable: Easy, only fails two unnecessary tests. I could integrate soon (developed and tested).
• JSR310: Easy/Medium: A possible backwards-compatibility lack in rome-modules. I could integrate later than Serializable (developed and tested).
• Cloneable: Medium/Hard: The related tests break the pipeline and remove feature implies full code review. In addition to, equals/hashcode feature is affected by this.
• JDK DOM/StAX: Hard/(maybe) unfeasible: A lot of problems at memory-cost and not implemented in Android. In fact, SAX implementation implies ROME redesign at build/parse/convert because of cursor/event iterator APIs.

The rest of features I have no opinion because I don't have experience in reflection and service locator.

Regards,
Antonio.

[antoniosanct]

Hi, @neroux

JSR 310 was introduced in Android API Level 26 (Android 8.0 or later). Then, the same config project used in DOM compatibility was unsuccessful, launching a simple test. Including this JSR would be the next start point of ROME and Android.

Regards,
Antonio.

[neroux]

So it wouldn't work before 26?

[antoniosanct]

That's correct! Until API 26, CNFE for java.time.*. From API 26 works perfectly.

[neroux]

Thanks for confirming. That was my concern.

There is, technically, a backported version at https://github.com/ThreeTen/threetenbp and that might work as an intermediate step, but of course it won't be java.time.

there are the following questions now

1. How important is the migration to java.time?
2. If it is important, should the library drop support for <26 Android?
   • If yes, java.time would be a viable choice
   • If no, we'd need to consider the backport
3. How quickly should the migration take place?
   • If the new version should be available soon-ish, I'd be hesitant to drop <26 just yet
   • If it will still take several months, pre-8 Android might be really around 5% at that point

[neroux]

Do we have a list of where we would like to introduce java.time?

[PatrickGotthard]

Hi @neroux and @antoniosanct,

please do not invest too much time implementing the >proposed< changes. First we should analyze the impact on ROME users and discuss whether we should make backwards-incompatible changes.

Regards,
Patrick

[neroux]

That's precisely why I asked the questions in the first place.