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

Invalid Json fetched from jv_snapshot.state column oracle sql database #1055

Closed
WojtekPWas opened this issue Jan 27, 2021 · 8 comments
Closed

Comments

@WojtekPWas
Copy link
Contributor

Clear description of my expectations versus reality

We have got recently an error
com.google.gson.JsonSyntaxException: com.google.gson.stream.MalformedJsonException: Use JsonReader.setLenient(true) to accept malformed JSON at line 1 column 3 path $
at com.google.gson.Gson.assertFullConsumption(Gson.java:864) ~[gson-2.8.2.jar!/:na]
at com.google.gson.Gson.fromJson(Gson.java:854) ~[gson-2.8.2.jar!/:na]
at com.google.gson.Gson.fromJson(Gson.java:802) ~[gson-2.8.2.jar!/:na]
at com.google.gson.Gson.fromJson(Gson.java:774) ~[gson-2.8.2.jar!/:na]
at org.javers.core.json.JsonConverter.fromJsonToJsonElement(JsonConverter.java:70) ~[javers-core-3.10.2.jar!/:na]

The reason was in line 22 in org/javers/repository/sql/finders/CdoSnapshotMapper.java (javers-persistence-sql module)
.withSnapshotState(resultSet.getString(SNAPSHOT_STATE))
The method withSnapshotState in CdoSnapshotSerialized class gets string as a parameter. This parameter has a value, which in debugger looks very similar to
https://stackoverflow.com/questions/46198533/java-sql-resultset-getstring-returns-string-with-extra-characters

Contents of column state (of type CLOB) in the table jv_snapshots in the Database (retrieved with SQL Developer tool) looks ok.

Method getString is not the right way to retrieve data from the CLOB column, although it brings correct results in most cases.

The solution was to use getClob method as described in https://stackoverflow.com/questions/19486648/how-to-retrive-the-clob-value-from-oracle-using-java/19486749.

I prepared custom Javers version basen on 3.10.2 and the problem disappeared.

I looked at the current version of Javers 5.14.0. getString(SNAPSHOT_STATE) is still there.
That is why I suggest the issue. Of course I would like to contribute and implement it in the current javers version.

Steps To Reproduce
I have no idea how to produce test case. Problem arise with fetching data from the database. The data is correct. STATE column of problematic row contains many characters, but there were cases with more characters and no error. I suppose the problem has to do with how Oracle Server maintains the data, that data not necessarily occupy continuous space.

Javers' Version
3.10.2
5.14.0 contains invalid method call getString(SNAPSHOT_STATE)

Additional context
The error was reported after 2 years of production use of the system. Table jv_snapshot contains more than 36 milion rows.

In our next project we will use current javers version. I find Javers well designed, easy to use and powerful.

Best regards
Wojtek Wąs

@bartoszwalacik
Copy link
Member

Hi Wojtek. Thanks for the good report, please contribute a PR with the fix for the Oracle dialect. Let me know if you need some hints how to start.

@WojtekPWas
Copy link
Contributor Author

Hi Bartosz,
thanks for the response. I will begin soon. As I understand, I should commit my changes to the new branch named after issue number. I think this fix should apply for all SQL databases and not only for Oracle dialect, but I will look closely at the current version of code.

@bartoszwalacik
Copy link
Member

nice, you dont have to create a branch, just fork this repo and create a PR from your fork's master to this master

WojtekPWas added a commit to WojtekPWas/javers that referenced this issue Mar 9, 2021
@WojtekPWas
Copy link
Contributor Author

I created the PR with two modifications and a small test class. But I could not reproduce my problem. I have tried on the same database and data as before, but this time getString() performed well. Probably it is because of database maintenance. I will try to investigate it farther.

@bartoszwalacik
Copy link
Member

@bartoszwalacik
Copy link
Member

you should not change the behavior for all dialects, getString() works perfectly well on most databases. Clob is a contraption of Oracle. second, you have change the read method, what about the write method?

WojtekPWas added a commit to WojtekPWas/javers that referenced this issue Mar 13, 2021
@WojtekPWas
Copy link
Contributor Author

WojtekPWas commented Mar 13, 2021

I have missed integration tests, sorry. Now tests for MySQL and PostgreSQL run on my workstation. I see, that STATE column is of TEXT type for PostgreSQL and MySQL. My solution needs injection of Dialect into CdoSnapshotFinder and SnapshotQuery, which is not very elegant, but I could not find place for additional parametrizing of Servers. You have written "consult the design" in the Gulideline for developers, so I treat my commit as an input for the consultation.

I have not changed anything in write method because it worked fine. In all my error cases the data stored in the database was correct.

Best regards Wojtek

WojtekPWas added a commit to WojtekPWas/javers that referenced this issue Mar 14, 2021
bartoszwalacik added a commit that referenced this issue Apr 17, 2021
bartoszwalacik added a commit that referenced this issue Apr 17, 2021
bartoszwalacik added a commit that referenced this issue Apr 17, 2021
bartoszwalacik added a commit that referenced this issue Apr 17, 2021
bartoszwalacik added a commit that referenced this issue Apr 17, 2021
lucas-prestes pushed a commit to lucas-prestes/javers that referenced this issue Jun 19, 2021
lucas-prestes pushed a commit to lucas-prestes/javers that referenced this issue Jun 19, 2021
lucas-prestes pushed a commit to lucas-prestes/javers that referenced this issue Jun 19, 2021
lucas-prestes pushed a commit to lucas-prestes/javers that referenced this issue Jun 19, 2021
lucas-prestes pushed a commit to lucas-prestes/javers that referenced this issue Jun 19, 2021
lucas-prestes pushed a commit to lucas-prestes/javers that referenced this issue Jun 19, 2021
lucas-prestes pushed a commit to lucas-prestes/javers that referenced this issue Jun 19, 2021
lucas-prestes pushed a commit to lucas-prestes/javers that referenced this issue Jun 19, 2021
@bartoszwalacik
Copy link
Member

fixed in 6.1.0

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

No branches or pull requests

2 participants