Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for #32 #46

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Fix for #32 #46

wants to merge 10 commits into from

Conversation

bflorat
Copy link
Contributor

@bflorat bflorat commented Aug 7, 2018

Note :

  • The algorithm and most of the code is from clapointe, thanks to him/her.
  • For some reasons, some package names were wrong, fixed along the way
  • All UT pass but I suggest to upgrade the sample docs for this use case with the one I provided in the issue
  • Not sure if the problem can still occur with table cells, I didn't reproduced any issue with deleted rows but please have a look, I didn't change anything with them
  • Could you please integrate it and release a new maven release ?

@thombergs
Copy link
Owner

Thanks for the PR.

If I use the file from your issue, it still fails with an IndexOutOfBoundsException, though. Can you have another look at that?

@bflorat
Copy link
Contributor Author

bflorat commented Aug 17, 2018

Sorry, didn't see your response. I check this ASAP.

@bflorat
Copy link
Contributor Author

bflorat commented Aug 17, 2018

Strange, I cannot reproduce the OutOfBoundException with the fix, everything works fine. The PR now includes a new UT with associated test docx (src/test/resources/org/wickedsource/docxstamper/ConditionalDisplayOfTablesBug32Test.docx) . Without the fix, I get :

java.lang.ClassCastException: javax.xml.bind.JAXBElement cannot be cast to org.docx4j.wml.P
	at org.wickedsource.docxstamper.ConditionalDisplayOfTablesBug32Test.globalTablesAreRemoved(ConditionalDisplayOfTablesBug32Test.java:31)
	at org.wickedsource.docxstamper.ConditionalDisplayOfTablesBug32Test.test(ConditionalDisplayOfTablesBug32Test.java:26)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.internal.runners.TestMethod.invoke(TestMethod.java:59)
	at org.junit.internal.runners.MethodRoadie.runTestMethod(MethodRoadie.java:98)
	at org.junit.internal.runners.MethodRoadie$2.run(MethodRoadie.java:79)
	at org.junit.internal.runners.MethodRoadie.runBeforesThenTestThenAfters(MethodRoadie.java:87)
	at org.junit.internal.runners.MethodRoadie.runTest(MethodRoadie.java:77)
	at org.junit.internal.runners.MethodRoadie.run(MethodRoadie.java:42)
	at org.junit.internal.runners.JUnit4ClassRunner.invokeTestMethod(JUnit4ClassRunner.java:88)
	at org.junit.internal.runners.JUnit4ClassRunner.runMethods(JUnit4ClassRunner.java:51)
	at org.junit.internal.runners.JUnit4ClassRunner$1.run(JUnit4ClassRunner.java:44)
	at org.junit.internal.runners.ClassRoadie.runUnprotected(ClassRoadie.java:27)
	at org.junit.internal.runners.ClassRoadie.runProtected(ClassRoadie.java:37)
	at org.junit.internal.runners.JUnit4ClassRunner.run(JUnit4ClassRunner.java:42)
	at org.eclipse.jdt.internal.junit4.runner.JUnit4TestReference.run(JUnit4TestReference.java:86)
	at org.eclipse.jdt.internal.junit.runner.TestExecution.run(TestExecution.java:38)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:538)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.runTests(RemoteTestRunner.java:760)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.run(RemoteTestRunner.java:460)
	at org.eclipse.jdt.internal.junit.runner.RemoteTestRunner.main(RemoteTestRunner.java:206)

@thombergs
Copy link
Owner

@bflorat OK, I'll check this again, perhaps I did something wrong.

@bflorat
Copy link
Contributor Author

bflorat commented Oct 9, 2018

Hi there, any update ? Thanks. I'm available for any test.

@bflorat
Copy link
Contributor Author

bflorat commented Nov 19, 2018

Hi, for the record, I just tested the last upstream version and the bug is still there [1] :

Note for users : it is safe to use the official upsteam version if you don't drop tables in your templates, otherwise you may experience OOB expections.

[1]
 gradle test
:compileJava
[Fatal Error] commons-lang3-3.4.pom:772:6: The processing instruction target matching "[xX][mM][lL]" is not allowed.
Download https://repo1.maven.org/maven2/org/apache/commons/commons-lang3/3.4/commons-lang3-3.4.pom
Note: /home/bflorat/workspaces/workspace-playground/docx-stamper-official/src/main/java/org/wickedsource/docxstamper/proxy/ProxyBuilder.java uses unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.
:processResources UP-TO-DATE
:classes
:compileTestJava
[Fatal Error] commons-lang3-3.4.pom:772:6: The processing instruction target matching "[xX][mM][lL]" is not allowed.
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.
:processTestResources
:testClasses
:test
[Fatal Error] commons-lang3-3.4.pom:772:6: The processing instruction target matching "[xX][mM][lL]" is not allowed.

org.wickedsource.docxstamper.ConditionalDisplayOfTablesBug32Test > test FAILED
    java.lang.ClassCastException at ConditionalDisplayOfTablesBug32Test.java:31

64 tests completed, 1 failed
:test FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':test'.
> There were failing tests. See the report at: file:///home/bflorat/workspaces/workspace-playground/docx-stamper-official/build/reports/tests/index.html

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output.

BUILD FAILED

@bflorat
Copy link
Contributor Author

bflorat commented Nov 20, 2021

I forked this project to fix this. It is available on Maven central at

  <dependency>
            <groupId>net.florat</groupId>
            <artifactId>docx-stamper</artifactId>
            <version>1.4.0~1</version>
        </dependency>

@bflorat bflorat mentioned this pull request Sep 10, 2022
@thombergs
Copy link
Owner

merged into this branch for inclusion in version 2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants