Unable to replace paragraph in ODF Text Document #235

rsoika · 2023-06-21T15:19:15Z

I need to replace a text fragment of a ODT Text Document with a new text.
My code looks something like this:

	private void replaceODFTextFragment(OdfTextDocument doc, String pattern, String replace)
			throws InvalidNavigationException {
		TextNavigation searchPattern = new TextNavigation(pattern, doc);
		while (searchPattern.hasNext()) {
			logger.info("..found match "+pattern + "    -----> Replace: "+replace);
			TextSelection textSelection = (TextSelection) searchPattern.getCurrentItem();
			textSelection.replaceWith(replace);
			break;
		}

	}

But this is not working.
The OdfTextDocument is not updated at all.

Also it is not possible to navigate to the next element as stated in the documentation [here](https://odftoolkit.org/api/odfdom/org/odftoolkit/odfdom/incubator/search/Navigation.html#getNextMatchElement(org.w3c.dom.Node).

There is no getNextMatchElement method in version 0.11.0 of this project.

Can anybody help me out here.

I use the library successful for OdfSpreadsheetDocument objects and this works well.

The text was updated successfully, but these errors were encountered:

svanteschubert · 2023-06-22T05:57:15Z

Dear Ralph,

Michael is on vacation and I have my head currently wrapped around a different difficult problem, so let me just give you a pointer how I would solve it.

Background

This functionality comes likely from the IBM team that had abandoned ODFDOM, when IBM abandoned OpenOffice (OO). My guess for this abandonment: LibreOffice (LO) could copy new features from OO but OO could not copy features from LO due to copyright.

Basic Approach

If something does not work as expected, create a regression test (best reuse an existing one) and see what failes - narrow it down.

I love to use command-line/bash on Linux to check this.

Go into the test directory and search for "TextNavigation":

/dev/odf/odftoolkit-latest/odfdom/src/test$ find . -name ".java" ./java/org/odftoolkit/odfdom/doc/table/TableRowColumnTest.java:import ./java/org/odftoolkit/odfdom/doc/table/TableRowColumnTest.java: ./java/org/odftoolkit/odfdom/incubator/search/TextStyleNavigationTest.java: ./java/org/odftoolkit/odfdom/incubator/search/TextStyleNavigationTest.java: ./java/org/odftoolkit/odfdom/incubator/search/TextStyleNavigationTest.java: ./java/org/odftoolkit/odfdom/incubator/search/TextStyleNavigationTest.java: ./java/org/odftoolkit/odfdom/incubator/search/TextStyleNavigationTest.java: ./java/org/odftoolkit/odfdom/incubator/search/TextStyleNavigationTest.java: ./java/org/odftoolkit/odfdom/incubator/search/TextStyleNavigationTest.java: ./java/org/odftoolkit/odfdom/incubator/search/TextSelectionTest.java: ./java/org/odftoolkit/odfdom/incubator/search/TextSelectionTest.java: ./java/org/odftoolkit/odfdom/incubator/search/TextSelectionTest.java: ./java/org/odftoolkit/odfdom/incubator/search/TextSelectionTest.java: ./java/org/odftoolkit/odfdom/incubator/search/TextSelectionTest.java: ./java/org/odftoolkit/odfdom/incubator/search/TextSelectionTest.java: ./java/org/odftoolkit/odfdom/incubator/search/TextSelectionTest.java: ./java/org/odftoolkit/odfdom/incubator/search/TextSelectionTest.java: ./java/org/odftoolkit/odfdom/incubator/search/TextSelectionTest.java: ./java/org/odftoolkit/odfdom/incubator/search/TextSelectionTest.java: ./java/org/odftoolkit/odfdom/incubator/search/TextSelectionTest.java: ./java/org/odftoolkit/odfdom/incubator/search/TextSelectionTest.java: ./java/org/odftoolkit/odfdom/incubator/search/TextSelectionTest.java: ./java/org/odftoolkit/odfdom/incubator/search/TextSelectionTest.java: ./java/org/odftoolkit/odfdom/incubator/search/TextSelectionTest.java: ./java/org/odftoolkit/odfdom/incubator/search/TextSelectionTest.java: ./java/org/odftoolkit/odfdom/incubator/search/TextSelectionTest.java: ./java/org/odftoolkit/odfdom/incubator/search/TextSelectionTest.java: ./java/org/odftoolkit/odfdom/incubator/search/MONPTest.java: ./java/org/odftoolkit/odfdom/incubator/search/TextNavigationTest.java:/** ./java/org/odftoolkit/odfdom/incubator/search/TextNavigationTest.java:public ./java/org/odftoolkit/odfdom/incubator/search/TextNavigationTest.java: ./java/org/odftoolkit/odfdom/incubator/search/TextNavigationTest.java: ./java/org/odftoolkit/odfdom/incubator/search/TextNavigationTest.java: ./java/org/odftoolkit/odfdom/incubator/search/TextNavigationTest.java: ./java/org/odftoolkit/odfdom/incubator/search/TextNavigationTest.java: ./java/org/odftoolkit/odfdom/incubator/search/TextNavigationTest.java: ./java/org/odftoolkit/odfdom/incubator/search/TextNavigationTest.java: ./java/org/odftoolkit/odfdom/incubator/search/TextNavigationTest.java: | xargs grep TextNavigation
org.odftoolkit.odfdom.incubator.search.TextNavigation;
TextNavigation search = new TextNavigation("cell", odtdoc);
TextNavigation search2;
TextNavigation search3;
search2 = new TextNavigation("delete", doc);
search3 = new TextNavigation("Roman16 Romanl16delete", doc);
search2 = new TextNavigation("delete", doc);
search3 = new TextNavigation("deleteRoman16 Romanl16", doc);
search2 = new TextNavigation("Century22", doc);
TextNavigation search;
search = new TextNavigation("delete", doc);
TextNavigation nextsearch = new TextNavigation("next", doc);
search = new TextNavigation("delete", doc);
search = new TextNavigation("delete", doc);
TextNavigation search1 = new TextNavigation("change", doc);
search = new TextNavigation("changedelete", doc);
search = new TextNavigation("delete", doc);
TextNavigation search1 = new TextNavigation("change", doc);
search = new TextNavigation("deletechange", doc);
search = new TextNavigation("delete", doc);
search = new TextNavigation("ODFDOM", doc);
TextNavigation nextsearch = new TextNavigation("next", doc);
search = new TextNavigation("Odf Toolkit", doc);
final List navigations =
.map(s -> new TextNavigation(Pattern.compile(s), doc2))
search = new TextNavigation("^delete", doc);
search = new TextNavigation("<%([^>])%>", doc);
TextNavigation search = new TextNavigation("mnop", doc);
Test the method of class org.odftoolkit.odfdom.incubator.search.TextNavigation /
class TextNavigationTest {
private static final Logger LOG = Logger.getLogger(TextNavigationTest.class.getName());
TextNavigation search;
Logger.getLogger(TextNavigationTest.class.getName()).log(Level.SEVERE, e.getMessage(), e);
/* Test getCurrentItem method of org.odftoolkit.odfdom.incubator.search.TextNavigation /
search = new TextNavigation("delete", doc);
/* Test getNextMatchElement method of org.odftoolkit.odfdom.incubator.search.TextNavigation */
search = new TextNavigation("delete", doc);
Logger.getLogger(TextNavigationTest.class.getName()).log(Level.SEVERE, e.getMessage(), e);

Does one of the tests is similar to your required functionality?
Why those tests not failing?

I hope I could point you into some useful direction, Ralph!

Godspeed and good luck!
Svante

rsoika · 2023-06-22T06:28:00Z

@svanteschubert thanks for your response. Gives me hope again that I am not alone ;-)
Ok, the package name org.odftoolkit.odfdom.incubator. for the TextNavigation class was already suspicious.
I think I will now follow a different approach using the general methods findFirstChildNode, countChildComponents, findNextChildNode to navigate through the document and find the node of interest. I have seen that it is, of course, possible to replace parts of a document (which is my main goal here).

I will post my results here.....

svanteschubert · 2023-06-22T08:43:35Z

@rsoika of course you are not alone - instantly reminded me of the song fragment:
'hundred billion bottles washed up on the shore' ;-)

I double checked the JavaDoc problem you mentioned, I guess the docu is right but the access rights might not:
TextNavigation is a subclass of Navigation and getNextMatchElement is only protected at Navigation.

Feel free improving the situation! You help (pull request) is very much welcome... :-)
You might want to discuss design changes ahead to receive quick feed-back... this is what I usually do, when I call Michael or write GitHub issues to sort my thoughts.. ;-)

PS: the 'incubator' package was a bad idea to indicate beta versions, in Google Guava they are using @Beta annotations. Now we are uncertain if we should 'break' the API by dropping those package names... Opinions?

rsoika · 2023-06-22T10:11:41Z

Yes I verified this. Your are right, it is the wrong protection of the methods.
At the moment it took me to much time and I can't have a deeper look (there were other problems with getting the selection OdfElement.

Maybe I find time the next days to test it further. In the moment I have created an ugly search method in my project to solve the problem somehow. (not the perfect world).

Regarding the incubator package: I would suggest to remove the package and place the code in a official package. Independent from this the problems can be fixed over the time. But with the incubator package other projects (like my own) have more problems by generating source code with need to be refactored too in the future which should not be the case...

rsoika · 2023-06-22T20:28:10Z

@svanteschubert I created a pull request fixing the issues with the TextNavigation.
The scope of the protected methods was not the reason of the problem. At least I it was simply a missing next() method in the navigator interface, to use the navigator in a way you would expect.
Also there was a minor issue with a OutOfBoundsException which I have also fixed.

See my pull request here: #236

And still think the package name should be changed form org.odftoolkit.odfdom.incubator.search to org.odftoolkit.odfdom.search. Better now than later.

Please let me know what you think.

svanteschubert · 2023-06-23T06:55:25Z

@rsoika
Cool, Ralph! That was quick!

Although it is a little early for x-mas, but I would love to have a regression test (extended or added) that tests your functionality!
Would be great if this test would fail without your patch and works with - and touch these edge states like indexoutofbound as well! Those regression tests did help a lot to keep sanity over the past years! :-)

Do not want to stretch it, but know you got it in your short term memory and it might be relative quick to produce for you! :-)
If you have a test, I would merge and create a Maven Snapshot release right away (after my review - dis/enable change - see how test reacts, etc.)...

Regarding the "incubator" I am on your side. I think, too, we should remove it,
Perhaps to keep the pain for users low it might make sense to do first another release iteration for ODF 1.2 with the old package name and not breaking their builds (and also right away for ODF 1.3 - only switchting the generated ODF) and then afterwards doing a 1.0 release with the "repaired" name, breaking compatibility but making it somehow more obvious! :-)

rsoika · 2023-06-23T11:01:24Z

@svanteschubert I added an additional test method to test the behaviour. I also did a lot of testing of other aspects when I wrote my test.
There are already some test methods but all these methods did only test the internal behaviour of the TextNavigator. There was no use case typical to that what a client would do: search for a specific part of a document and change the content.

And changing of the content during the navigating through the document was the part that (in my opinion) never was working correctly.
I tried a lot of refactoring, but at the end this all leads only to new problems, so I left the code as it is - just with my small changes.
The try/catch block which I added to the setCurrentTextAndGetIndex method is in my opinion the only way that works for all the cases. I am not super happy with that.
The main problem is the design of the navigator in its core (which I can't change at this point). In my eyes it should be an Iterator interface

while (nav.hasNext()) {
  result=nav.next();
 ...
}

But this is not the case and the next method was missing. The hasNext method did the navigation, which should be the task of the next method (for my understanding).

For example something like this did not work

while (nav.hasNext()) {
  result=nav.next();
  if (.... && nav.hasNext()) {
     // skip next
     nav.next();
   }
.....
 ...
}

In deed the core implementation has the idea to navigate in a paragraph and does not provide the idea to manipulate a paragrpah and skip to the next one.

But with my changes I think the code is fine and useful.

Let me know what you think about my work....

svanteschubert · 2023-06-23T14:23:12Z

Thanks again for your fix, Ralph!

I did my part and merged your patch now into the master and have uploaded a Maven SNAPSHOT release.
To not forget your request, I have written myself a follow up issue for the "incubator" package

Have a nice weekend!
Svante

rsoika · 2023-06-24T06:24:54Z

I wrote a short tutorial about the TextNavigation: https://ralph.blog.imixs.com/2023/06/24/find-and-replace-in-odf-documents/

rsoika · 2023-06-24T07:02:11Z

@svanteschubert I am sorry to bother you again, but I am still not happy with my last changes of the TextNavigation API.
I introduced the method next() to return the OdfElement containing the selection. This was because I simply missed a next method and the method .getCurrentItem() was in my eyes very confusing. But I am unhappy with this change and I want to ask you to change the methods of the Navigation interface in the following way:

next() returns the current Selection of the Navigator
getElement() returns the corresponding ODF element of the current Selection

This would lead to a much more clear API:

	textNav = new TextNavigation("myexpression", odt);
	while (textNav.hasNext()) {

		TextSelection selection = (TextSelection) textNav.next();
		logger.info("...Position=" + selection.getIndex());
		logger.info("...Text=" + selection.getText());

		// fetch paragraph of current selection....
		OdfElement paragraph = textNav.getElement();
		String content=paragraph.getTextContent();
		paragraph.setTextContent("Just a Example");}

	}

svanteschubert · 2023-06-26T06:32:37Z

@rsoika Not a problem as software is an iterative process ;-) Let me come back to you in the next days, I got a summer flue and have to take some days off..

rsoika mentioned this issue Jun 22, 2023

ODFDOMFindReplaceAdapter improve API imixs/imixs-adapters#135

Closed

svanteschubert linked a pull request Jun 23, 2023 that will close this issue

Improved navigation interface, added next() method #236

Merged

svanteschubert closed this as completed in #236 Jun 23, 2023

rsoika mentioned this issue Jun 24, 2023

implementation of a generic iterator interface, api changes #238

Closed

svanteschubert self-assigned this Jun 29, 2023

mistmist added this to the 0.12.0 milestone Jan 24, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unable to replace paragraph in ODF Text Document #235

Unable to replace paragraph in ODF Text Document #235

rsoika commented Jun 21, 2023

svanteschubert commented Jun 22, 2023 •

edited

rsoika commented Jun 22, 2023

svanteschubert commented Jun 22, 2023

rsoika commented Jun 22, 2023

rsoika commented Jun 22, 2023

svanteschubert commented Jun 23, 2023

rsoika commented Jun 23, 2023

svanteschubert commented Jun 23, 2023

rsoika commented Jun 24, 2023

rsoika commented Jun 24, 2023

svanteschubert commented Jun 26, 2023

Unable to replace paragraph in ODF Text Document #235

Unable to replace paragraph in ODF Text Document #235

Comments

rsoika commented Jun 21, 2023

svanteschubert commented Jun 22, 2023 • edited

Background

Basic Approach

rsoika commented Jun 22, 2023

svanteschubert commented Jun 22, 2023

rsoika commented Jun 22, 2023

rsoika commented Jun 22, 2023

svanteschubert commented Jun 23, 2023

rsoika commented Jun 23, 2023

svanteschubert commented Jun 23, 2023

rsoika commented Jun 24, 2023

rsoika commented Jun 24, 2023

svanteschubert commented Jun 26, 2023

svanteschubert commented Jun 22, 2023 •

edited