Details
-
Type:
Task
-
Status: Resolved
-
Priority:
Minor
-
Resolution: Fixed
-
Affects Version/s: 7.6.0, 8.0.0-M3, 6.26.0
-
Component/s: wicket, wicket-atmosphere, wicket-extensions
-
Labels:
Description
Because of license issues it is required to switch from json.org:
https://github.com/stleary/JSON-java
to open-json:
https://github.com/tdunning/open-json
For more information see:
http://apache-wicket.1842946.n4.nabble.com/Fwd-JSON-License-and-Apache-Projects-td4676261.html
Issue Links
- links to
- mentioned in
-
Page Loading...
Activity
- All
- Comments
- Work Log
- History
- Activity
- Transitions
Commit eba3a3064dc53505cfd09ec3f2ec1a7c044436f8 in wicket's branch refs/heads/wicket-6.x from Andrea Del Bene
[ https://git-wip-us.apache.org/repos/asf?p=wicket.git;h=eba3a30 ]
WICKET-6287 fix for https://github.com/openjson/openjson/issues/8
Commit c83c6fcb628f1024ef53ecf32cf5722277895719 in wicket's branch refs/heads/wicket-7.x from Andrea Del Bene
[ https://git-wip-us.apache.org/repos/asf?p=wicket.git;h=c83c6fc ]
WICKET-6287 fix for https://github.com/openjson/openjson/issues/8
Github user bitstorm commented on the issue:
https://github.com/apache/wicket/pull/195
thank you all!
Github user klopfdreh commented on the issue:
https://github.com/apache/wicket/pull/195
@bitstorm - the changes are made - JSONObject does not throw any checked exceptions anymore.
Commit 31727f8543d74396d44c8f2a8db83a6e82af7272 in wicket's branch refs/heads/master from Tobias Soloschenko
[ https://git-wip-us.apache.org/repos/asf?p=wicket.git;h=31727f8 ]
WICKET-6287 Switch from json.org to open-json - 8.x
- Updated version to 1.8 - ref: #193
- Update to user guide
Github user tdunning commented on the issue:
https://github.com/apache/wicket/pull/195
The artifacts for version 1.8 Open Json are on central.
https://repo1.maven.org/maven2/com/tdunning/json/1.8/
On Thu, Jan 19, 2017 at 11:44 AM, Tobias Soloschenko <
notifications@github.com> wrote:
> Thanks a lot @tdunning <https://github.com/tdunning> !
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/wicket/pull/195#issuecomment-273877731>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/AAPSeqZiYOyPvjpxOA4khkQgBk3PZVDhks5rT70MgaJpZM4K8fqL>
> .
>
Github user klopfdreh commented on the issue:
https://github.com/apache/wicket/pull/195
Thanks a lot @tdunning !
Github user tdunning commented on the issue:
https://github.com/apache/wicket/pull/195
Should get that release out later today.
On Thu, Jan 19, 2017 at 11:29 AM, Tobias Soloschenko <
notifications@github.com> wrote:
> @bitstorm <https://github.com/bitstorm> - the issue about the checked
> exceptions is fixed in the master branch we only have to wait until the
> release and increase our version in the pom.xml. Then there should be no
> further issue.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/wicket/pull/195#issuecomment-273873780>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/AAPSeuwHYOQSm09tF5fbT6vB5vcXRb8Pks5rT7mCgaJpZM4K8fqL>
> .
>
Github user klopfdreh commented on the issue:
https://github.com/apache/wicket/pull/195
@bitstorm - the issue about the checked exceptions is fixed in the master branch we only have to wait until the release and increase our version in the pom.xml. Then there should be no further issue.
Github user klopfdreh commented on the issue:
https://github.com/apache/wicket/pull/195
I changed the method propertiesAsMap to throw the JSONException, because the first statement of a constructor always has to be the super / this call.
Github user klopfdreh commented on the issue:
https://github.com/apache/wicket/pull/195
Hi @bitstorm ,
I saw that JSONObject throws a JSONException which extends RuntimeException - I thought it is unchecked.
Github user bitstorm commented on the issue:
https://github.com/apache/wicket/pull/195
@klopfdreh
Hi,
One minor issue I've seen with this PR is that now ' new JSONObject' throws checked exceptions, so the code must be adapted some how. This happened with WicketStuff [here](https://travis-ci.org/wicketstuff/core/builds/188994145). It's not a big issue, but maybe this could be a problem with other libraries.
Github user klopfdreh commented on the issue:
https://github.com/apache/wicket/pull/194
Closed due to:
- https://github.com/apache/wicket/commit/0ba87f062cad8664d216abccb2a47126f637b87c
- https://github.com/apache/wicket/commit/6f764146a007ab38686a7d54210db0fcdfc2a7ef
- https://github.com/apache/wicket/commit/a6bf0e29bbc9ccebaa7588f566e15f0ac1d2eaa6
- https://github.com/apache/wicket/commit/ad978211a5a550e7902eba354b03ec6618649579
Github user klopfdreh closed the pull request at:
Commit ad978211a5a550e7902eba354b03ec6618649579 in wicket's branch refs/heads/wicket-6.x from Tobias Soloschenko
[ https://git-wip-us.apache.org/repos/asf?p=wicket.git;h=ad97821 ]
WICKET-6287 Switch from json.org to open-json - 6.x
- Resolved compile error with JDK 6, Objects is not available
Commit a6bf0e29bbc9ccebaa7588f566e15f0ac1d2eaa6 in wicket's branch refs/heads/wicket-6.x from Tobias Soloschenko
[ https://git-wip-us.apache.org/repos/asf?p=wicket.git;h=a6bf0e2 ]
WICKET-6287 Switch from json.org to open-json - 6.x
- Fixed clirr issues
Commit 6f764146a007ab38686a7d54210db0fcdfc2a7ef in wicket's branch refs/heads/wicket-6.x from Tobias Soloschenko
[ https://git-wip-us.apache.org/repos/asf?p=wicket.git;h=6f76414 ]
WICKET-6287 Switch from json.org to open-json - 6.x
- Updated to open-json 1.3
Commit 0ba87f062cad8664d216abccb2a47126f637b87c in wicket's branch refs/heads/wicket-6.x from Tobias Soloschenko
[ https://git-wip-us.apache.org/repos/asf?p=wicket.git;h=0ba87f0 ]
WICKET-6287 Switch from json.org to open-json - 6.x
Github user klopfdreh closed the pull request at:
Github user klopfdreh commented on the issue:
https://github.com/apache/wicket/pull/195
Closed due to:
Commit 5348b3e00518e5636126e724f1e62ea1aebbd8b6 in wicket's branch refs/heads/wicket-7.x from Tobias Soloschenko
[ https://git-wip-us.apache.org/repos/asf?p=wicket.git;h=5348b3e ]
WICKET-6287 Switch from json.org to open-json - 7.x
- Fixed clirr issues
Commit 6b1d53b17a0aa77a8b131fd161e12e08d335a251 in wicket's branch refs/heads/wicket-7.x from Tobias Soloschenko
[ https://git-wip-us.apache.org/repos/asf?p=wicket.git;h=6b1d53b ]
WICKET-6287 Switch from json.org to open-json - 7.x
- Updated to open-json 1.3
Commit 3dacec4c55bb95618f604ad2ccea279346de279c in wicket's branch refs/heads/wicket-7.x from Tobias Soloschenko
[ https://git-wip-us.apache.org/repos/asf?p=wicket.git;h=3dacec4 ]
WICKET-6287 Switch from json.org to open-json - 7.x
Github user klopfdreh closed the pull request at:
Github user klopfdreh commented on the issue:
https://github.com/apache/wicket/pull/193
Closed due to:
Commit 49a486ac9fb379426b36ee29a8c9101fa818e669 in wicket's branch refs/heads/master from Tobias Soloschenko
[ https://git-wip-us.apache.org/repos/asf?p=wicket.git;h=49a486a ]
WICKET-6287 Switch from json.org to open-json - 8.x
- Update to open-json 1.6
Commit cc98cb0675ffbe9be956e33d6b827f4d61f9f3d7 in wicket's branch refs/heads/master from Tobias Soloschenko
[ https://git-wip-us.apache.org/repos/asf?p=wicket.git;h=cc98cb0 ]
WICKET-6287 Switch from json.org to open-json - 8.x
- Switched to version 1.3 of open-json
Commit 4e3194466c4df50c9d38ea1ed50800dea592c72b in wicket's branch refs/heads/master from Tobias Soloschenko
[ https://git-wip-us.apache.org/repos/asf?p=wicket.git;h=4e31944 ]
WICKET-6287 Switch from json.org to open-json - 8.x
Repository: https://github.com/tdunning/open-json
Github user klopfdreh commented on the issue:
https://github.com/apache/wicket/pull/193
Hi @tdunning - thanks for keeping us up to date with the changes! 👍
I am going to set the version to 1.6 so that we get the latest version.
Github user tdunning commented on the issue:
https://github.com/apache/wicket/pull/193
I have pushed a few additional versions of open-json that might help (or
hinder) wicket.
The basic issue was that in the bean constructor, objects were not
converted all the way down. This led to really bad behavior because the
native to-string was used on things like lists that led to ugly and
non-functional results.
This was resolved in open-json 1.6 in a way that I don't think would hurt
wicket, but it might be good for you guys to let me know if it does and if
there is a middle ground that does not.
On Mon, Dec 12, 2016 at 7:35 PM, Andrea Del Bene <notifications@github.com>
wrote:
> +1 to merge. I've tested with WicketStuff and with @sebfz1
> <https://github.com/sebfz1> wicket-jquery-ui. Two PR have been created
> for this two project but everything seems to work fine with them.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/wicket/pull/193#issuecomment-266395023>, or mute
> the thread
> <https://github.com/notifications/unsubscribe-auth/AAPSeqb2rypinCugy7pJriZdbrJzLbB7ks5rHSONgaJpZM4K74MM>
> .
>
Github user bitstorm commented on the issue:
https://github.com/apache/wicket/pull/193
+1 to merge. I've tested with WicketStuff and with @sebfz1 wicket-jquery-ui. Two PR have been created for this two project but everything seems to work fine with them.
I will test the 7.x branch against our main application today or tomorrow!
Github user klopfdreh commented on the issue:
https://github.com/apache/wicket/pull/193
I vote to merge the 8.x PR. For 6.x / 7.x @martin-g already did build tests and pointed out some issues and I would suggest to check out the PRs and test them even with wicketstuff to ensure not to break something.
Github user bitstorm commented on the issue:
https://github.com/apache/wicket/pull/193
Sorry I wasn't clear. I asked if this PR is complete and if we should proceed merging it and wicketstuff/core#566.
Github user klopfdreh commented on the issue:
https://github.com/apache/wicket/pull/193
@bitstorm - there are two different PRs for 6.x and 7.x to preserve the API integrity.
Github user klopfdreh commented on the issue:
https://github.com/apache/wicket/pull/195
Great! 😄 - so if everything is working - we should integrate all PR for open-json. But I suggest to test all dependencies (also for wicketstuff)
Github user bitstorm commented on the issue:
https://github.com/apache/wicket/pull/193
@klopfdreh @martin-g
Shall we go ahead merging or do you prefer to port the pr to the other two branches first?
Github user martin-g commented on the issue:
https://github.com/apache/wicket/pull/195
The build passes with JDK1.7!
Github user klopfdreh commented on the issue:
https://github.com/apache/wicket/pull/195
@martin-g I rebased it and pushed it again. Should be up to date now.
Github user duesenklipper commented on the issue:
https://github.com/apache/wicket/pull/195
@martin-g I hate German translations of computer terms, which is the main reason my systems are all in English. But as mentioned elsewhere, I use a split locale instead, with formatting set to German. Thus I break things in other ways 
Github user martin-g commented on the issue:
https://github.com/apache/wicket/pull/195
@duesenklipper I am the non-German guy here but it seems it is just me who uses de_DE locale to catch such kind of problems 
Your fix is good!
@klopfdreh Please update this branch to latest wicket-7.x and I'll retest it again with JDK1.7
Github user duesenklipper commented on the issue:
https://github.com/apache/wicket/pull/195
@klopfdreh I know you did
@martin-g I found it. @klopfdreh is right, it has something to do with Locale. My fault - for some reason I lost the `@Before` method in `CssUrlReplacerTest` that set the Locale to English, when I updated that test for data urls. I'll push the fix to the main branches. Sorry about that!
Github user klopfdreh commented on the issue:
https://github.com/apache/wicket/pull/193
Summary before integration:
- json.org is replaced with open-json, now
- Because Apache Wicket 8 is at milestone level API changes are allowed and because of this all JSON library classes are removed and the external dependency of open-json has been added. This way it is easier to keep the library updated. Users have to organize imports.
Github user klopfdreh commented on the issue:
https://github.com/apache/wicket/pull/194
Summary before integration:
- json.org is replaced with open-json, now
- Method calls not supported by open-json throw a WicketRuntimeException to inform the user
- If users still need json.org they have to reorganize imports and add the json.org dependency in maven (beware of the license issues in this case!!)
- All classes are preserved due to clirr checks, but are converted to dummies if there is no functionality left
- There has been a special change to JSONObject, because Objects.hashCode is only available since jdk > 7
Github user klopfdreh commented on the issue:
https://github.com/apache/wicket/pull/195
Summary before integration:
- json.org is replaced with open-json, now
- Method calls not supported by open-json throw a WicketRuntimeException to inform the user
- If the users still needs json.org they have to reorganize imports and add the json.org dependency in maven (beware of the license issues in this case!!)
- All classes are preserved due to clirr checks, but are converted to dummies if there is no functionality left
Github user klopfdreh commented on the issue:
https://github.com/apache/wicket/pull/194
Hi @martin-g - this was because Objects.hashCode is only available since idk > 7.
I replaced the call with a return 0; - this is done by Objects.hashCode(null) anyway.
I also created a ticket for open-json to change the code so that there is a "real" jdk 6 compatibility. 😄
Github user klopfdreh commented on the issue:
https://github.com/apache/wicket/pull/194
Hi @martin-g - thanks for the hint - I am going to have a look at it, soon.
Github user klopfdreh commented on the issue:
https://github.com/apache/wicket/pull/195
Haha @duesenklipper - you wrote a bit earlier - yep I know the CssUrlReplacer and @martin-g , too. We implemented it 😊
Github user klopfdreh commented on the issue:
https://github.com/apache/wicket/pull/195
I don't think this is regarding the JSON change. The String is to long and it is a different as the expected one and the CssUrlReplacer don't use JSON at all (only regex) - maybe it has something to do with the locale changes? (Only an assumption)
Github user duesenklipper commented on the issue:
https://github.com/apache/wicket/pull/195
@klopfdreh I don't think it does. It appears to be a problem on @martin-g's machine. The JSON stuff doesn't even come close to CssUrlReplacer.
Github user klopfdreh commented on the issue:
https://github.com/apache/wicket/pull/195
To come to the point: In which way the JSON lib change affects CssUrlReplacerTest.
Github user duesenklipper commented on the issue:
https://github.com/apache/wicket/pull/195
Oh, you mean a problem on your machine? If that's the case, I'll stop trying to recreate it here
Github user martin-g commented on the issue:
https://github.com/apache/wicket/pull/195
I was saying that if BuildBot is fine (at wicket-core) then the problem must be here.
The problem with Arquillian is known. It waits someone to debug it. Too bad the JavaEE fans do not give it more love
Github user duesenklipper commented on the issue:
https://github.com/apache/wicket/pull/195
That's a different problem though (and it worked for me too). I'm building 5bfa280b2b7fb9dc7861d582a9c6e37d106be879 locally now to see what it does (the one you linked to)
Github user martin-g commented on the issue:
https://github.com/apache/wicket/pull/195
The problem seems to be here.
BuildBot passes the build until Arquilian tests: https://ci.apache.org/builders/wicket-branch-7.x/builds/333
Github user duesenklipper commented on the issue:
https://github.com/apache/wicket/pull/195
(I ran `mvn clean package`)
Github user duesenklipper commented on the issue:
https://github.com/apache/wicket/pull/195
I figured out to do
$ git fetch github-upstream refs/pull/195/head
$ git checkout FETCH_HEAD
From that I got commit 4dbdbdfe, and that builds cleanly for me. I also tried to just run that particular test in IDEA, and it worked too.
Github user martin-g commented on the issue:
https://github.com/apache/wicket/pull/195
The build fails for me at branch `wicket-7.x` with the same error as in this PR.
I merge PRs from Github with https://github.com/martin-g/dotfiles/blob/master/bin/github-pr.sh:
- $ cd ~/git/apache/wicket-7.x
- $ github-pr.sh 195
Github user duesenklipper commented on the issue:
https://github.com/apache/wicket/pull/195
(that said, that test works in the original wicket-7.x branch. i'll try and figure this out as soon as i can get at the branch)
Github user duesenklipper commented on the issue:
https://github.com/apache/wicket/pull/195
This might be a stupid question, but I must be missing something. How do I check out the pull request branch?
Github user martin-g commented on the issue:
https://github.com/apache/wicket/pull/195
wicket-7.x fails the same way.
@duesenklipper Ping!
Github user martin-g commented on the issue:
https://github.com/apache/wicket/pull/195
Compilation and Clirr pass but a test fails:
```
Failed tests:
CssUrlReplacerTest.base64EncodedImage:165->Assert.assertThat:923->Assert.assertThat:956
Expected: a string containing ".class {background-image: url(data:image/gif;base64,R0lGODlh1wATAXAAACH5BAEAAP8ALAAAAADXA"
but: was ".class {background-image: url(data:image/gif;base64,R0lGODlhUAB4ANUAANbX1hQSEXV0c/fz8tCBfc3NzFBNTPO6ttzIx/LKyIGAf84jGri4t9avrainppiXlut9dqZSTI41MNzDwbCMitWQjLF0cJVzcc52cctKQ8zW1sLCwe+WkdnQz8U4MIVaWFxaWYuJiKg+Oa2sq52dnMygncvOz83QzzAuLbdbVpKRkEJAP9LDwpxmY+enpN3c3NLR0WdmZcnIyLKxsPHl5L++vc5oY8rLyuO9vMfGxuxqY6OhodZYUcXFxM7Ozv///yH5BAAAAAAALAAAAABQAHgAAAb/wJ9wSCwaj0jjAEFpfZwt54XSGSSv2Kw2OZh9QJ9paUwpfy+srXrNfkEpjdcuJqgNdiRW86Oysv+ARAhOIR01MQMOKCs7PyAqPw......
````
Github user martin-g commented on the issue:
https://github.com/apache/wicket/pull/194
This PR fails with:
```
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.5:compile (default-compile) on project wicket-core: Compilation failure: Compilation failure:
[ERROR] /home/martin/git/apache/wicket-6.x/wicket-core/src/main/java/org/apache/wicket/ajax/json/JSONObject.java:[30,17] cannot find symbol
[ERROR] symbol : class Objects
[ERROR] location: package java.util
[ERROR] /home/martin/git/apache/wicket-6.x/wicket-core/src/main/java/org/apache/wicket/ajax/json/JSONObject.java:[119,20] cannot find symbol
[ERROR] symbol: variable Objects
[ERROR] -> [Help 1]
```
Github user klopfdreh commented on the issue:
https://github.com/apache/wicket/pull/195
@martin-g , @dashorst , @bitstorm - would you be so kind and review the changes / check if there are no clirr issues for the 7.x branch? I don't have jdk 7 installed so I get a "Invalid byte tag in constant pool: 18" of clirr.
Github user klopfdreh commented on the issue:
https://github.com/apache/wicket/pull/194
@martin-g , @dashorst , @bitstorm - would you be so kind and review the changes / check if there are no clirr issues for the 6.x branch? I don't have jdk 6 / jdk 7 installed so I get a "Invalid byte tag in constant pool: 18" of clirr.
Github user klopfdreh commented on the issue:
https://github.com/apache/wicket/pull/194
Hi @solomax ,
I managed to resolve the clirr issues: https://github.com/apache/wicket/pull/195#issuecomment-265422034
Github user klopfdreh commented on the issue:
https://github.com/apache/wicket/pull/195
I managed to resolve the clirr issues.
What I did was:
- Revert back all class signatures that were copied from json.org and throw a corresponding exception to inform users about the switch
If the user updates Apache Wicket and is ok with the license issue of JSON.org the users code has to be switched to the corresponding maven dependency of the JSON.org library and the imports has to be reorganized, but for 90% the code will work as before.
Github user klopfdreh commented on the issue:
https://github.com/apache/wicket/pull/194
I am going to try this out, soon!
Github user solomax commented on the issue:
https://github.com/apache/wicket/pull/194
Actually this can be handled using double-proxy, but I would vote for API break in this case
Github user klopfdreh commented on the issue:
https://github.com/apache/wicket/pull/194
@solomax - yep - I updated the classes with the versions of 1.3. Anyway there are clirr issues even if we copy all missing classes and override methods / constructors to throw WicketRuntineExceptions, because some method signatures have been changed.
Github user solomax commented on the issue:
https://github.com/apache/wicket/pull/194
I believe open-json files need to be updated to correspond 1.3 ....
Github user klopfdreh commented on the issue:
https://github.com/apache/wicket/pull/193
Hi @bitstorm - I switched it to version 1.3.
Github user bitstorm commented on the issue:
https://github.com/apache/wicket/pull/193
@tdunning @klopfdreh
yes, the 1.2-SNAPSHOT must turn into 1.3
Github user tdunning commented on the issue:
https://github.com/apache/wicket/pull/193
Not sure why, but I see 1.2-SNAPSHOT in the comments coming through.
That is going to point at open-json 1.3 when you actually move forward,
right?
(just call me "better safe than sorry")
On Mon, Dec 5, 2016 at 9:13 PM, Andrea Del Bene <notifications@github.com>
wrote:
> @bitstorm approved this pull request.
>
> Green light for me. I've also prepared a pr for Wicketstuff to work with
> this new library:
> wicketstuff/core#566 <https://github.com/wicketstuff/core/pull/566>
> ------------------------------
>
> In pom.xml
> <https://github.com/apache/wicket/pull/193#pullrequestreview-10368392>:
>
> > @@ -447,6 +447,12 @@
> <type>jar</type>
> </dependency>
> <dependency>
> + <groupId>com.tdunning</groupId>
> + <artifactId>json</artifactId>
> + <version>1.2-SNAPSHOT</version>
>
> I've already opened an issue. Just in case...
>
> tdunning/open-json#2 <https://github.com/tdunning/open-json/issues/2>
>
> —
> You are receiving this because you commented.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/wicket/pull/193#pullrequestreview-10368392>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AAPSeqjqTjPZGaOQmErVakoBjXmRza0Gks5rE__-gaJpZM4K74MM>
> .
>
Github user bitstorm commented on a diff in the pull request:
https://github.com/apache/wicket/pull/193#discussion_r89835461
— Diff: pom.xml —
@@ -447,6 +447,12 @@
<type>jar</type>
</dependency>
<dependency>
+ <groupId>com.tdunning</groupId>
+ <artifactId>json</artifactId>
+ <version>1.2-SNAPSHOT</version>
— End diff –
I've already opened an issue. Just in case...
Github user tdunning commented on a diff in the pull request:
https://github.com/apache/wicket/pull/193#discussion_r90758219
— Diff: pom.xml —
@@ -447,6 +447,12 @@
<type>jar</type>
</dependency>
<dependency>
+ <groupId>com.tdunning</groupId>
+ <artifactId>json</artifactId>
+ <version>1.2-SNAPSHOT</version>
— End diff –
I just pushed version 1.3 that has quoting fixes and a bean constructor.
Github user klopfdreh commented on a diff in the pull request:
https://github.com/apache/wicket/pull/193#discussion_r89832914
— Diff: pom.xml —
@@ -447,6 +447,12 @@
<type>jar</type>
</dependency>
<dependency>
+ <groupId>com.tdunning</groupId>
+ <artifactId>json</artifactId>
+ <version>1.2-SNAPSHOT</version>
— End diff –
Hi @bitstorm ,
I am going to write a Unit Test for this and request another PR for open-json, soon.
kind regards
Tobias
Github user bitstorm commented on a diff in the pull request:
https://github.com/apache/wicket/pull/193#discussion_r89825120
— Diff: pom.xml —
@@ -447,6 +447,12 @@
<type>jar</type>
</dependency>
<dependency>
+ <groupId>com.tdunning</groupId>
+ <artifactId>json</artifactId>
+ <version>1.2-SNAPSHOT</version>
— End diff –
I think the problem is that the new library escapes special characters also for JSONFunction, so 'alert("")' becomes 'alert(\"\")' and can not be evaluated by javascript.
Github user bitstorm commented on a diff in the pull request:
https://github.com/apache/wicket/pull/193#discussion_r89820402
— Diff: pom.xml —
@@ -447,6 +447,12 @@
<type>jar</type>
</dependency>
<dependency>
+ <groupId>com.tdunning</groupId>
+ <artifactId>json</artifactId>
+ <version>1.2-SNAPSHOT</version>
— End diff –
@klopfdreh
JSONFunction are rendered as regular strings, surrounded by double quotes. But I think the culprit is here:
https://github.com/tdunning/open-json/blob/master/src/main/java/org/json/JSONStringer.java#L261
We should rename JsonFunction back to JSONFunction
Github user bitstorm commented on a diff in the pull request:
https://github.com/apache/wicket/pull/193#discussion_r89802532
— Diff: pom.xml —
@@ -447,6 +447,12 @@
<type>jar</type>
</dependency>
<dependency>
+ <groupId>com.tdunning</groupId>
+ <artifactId>json</artifactId>
+ <version>1.2-SNAPSHOT</version>
— End diff –
@klopfdreh
Please don't go ahead with the integration! I think we have a problem with the new JSON library and AJAX. The Modal Window doesn't work...
Github user bitstorm commented on a diff in the pull request:
https://github.com/apache/wicket/pull/193#discussion_r89743045
— Diff: pom.xml —
@@ -447,6 +447,12 @@
<type>jar</type>
</dependency>
<dependency>
+ <groupId>com.tdunning</groupId>
+ <artifactId>json</artifactId>
+ <version>1.2-SNAPSHOT</version>
— End diff –
@klopfdreh
I'm done with wicketstuff. I think 4-5 modules (select2, whiteboard, etc...) should be manually tested but don't expect problems.
Github user bitstorm commented on a diff in the pull request:
https://github.com/apache/wicket/pull/193#discussion_r89698452
— Diff: pom.xml —
@@ -447,6 +447,12 @@
<type>jar</type>
</dependency>
<dependency>
+ <groupId>com.tdunning</groupId>
+ <artifactId>json</artifactId>
+ <version>1.2-SNAPSHOT</version>
— End diff –
@klopfdreh ok, i'm working on few minor problems with wicketstuff. The main one is to substitute JSONWriter with JSONStringer which don't have exactly the same methods. When I'm done I will proceed with the merge.
Github user klopfdreh commented on a diff in the pull request:
https://github.com/apache/wicket/pull/193#discussion_r89693470
— Diff: pom.xml —
@@ -447,6 +447,12 @@
<type>jar</type>
</dependency>
<dependency>
+ <groupId>com.tdunning</groupId>
+ <artifactId>json</artifactId>
+ <version>1.2-SNAPSHOT</version>
— End diff –
@bitstorm - if also for wicketstuff then lets integrate 👍 😄
Github user bitstorm commented on a diff in the pull request:
https://github.com/apache/wicket/pull/193#discussion_r89690436
— Diff: pom.xml —
@@ -447,6 +447,12 @@
<type>jar</type>
</dependency>
<dependency>
+ <groupId>com.tdunning</groupId>
+ <artifactId>json</artifactId>
+ <version>1.2-SNAPSHOT</version>
— End diff –
@klopfdreh
mvn clean test works just fine for me, I just changed 1.2-SNAPSHOT to 1.2
Github user bitstorm commented on a diff in the pull request:
https://github.com/apache/wicket/pull/193#discussion_r89680957
— Diff: pom.xml —
@@ -447,6 +447,12 @@
<type>jar</type>
</dependency>
<dependency>
+ <groupId>com.tdunning</groupId>
+ <artifactId>json</artifactId>
+ <version>1.2-SNAPSHOT</version>
— End diff –
@klopfdreh
Great! Good job Tobias! I think we can go ahead with this PR and merge it
Github user klopfdreh commented on a diff in the pull request:
https://github.com/apache/wicket/pull/193#discussion_r89677866
— Diff: pom.xml —
@@ -447,6 +447,12 @@
<type>jar</type>
</dependency>
<dependency>
+ <groupId>com.tdunning</groupId>
+ <artifactId>json</artifactId>
+ <version>1.2-SNAPSHOT</version>
— End diff –
Hi @bitstorm ,
1.2 has been just released. 😄
Anyway the were two reasons why to wait for this version.
1. The slash / should only be escaped after a "<" bracket (https://github.com/tdunning/open-json/commit/e1541126bf3206fa7a2b243f3791e274822128c1)
2. JSONFunction was not implemented yet. (https://github.com/tdunning/open-json/commit/174ab93ac966f0213ac4a4bd3ac5b17e0920a7c8) - this is required for json like this:
We can switch the version to 1.2 and it should work with this PR.
kind regards
Tobias
Github user bitstorm commented on a diff in the pull request:
https://github.com/apache/wicket/pull/193#discussion_r89675392
— Diff: pom.xml —
@@ -447,6 +447,12 @@
<type>jar</type>
</dependency>
<dependency>
+ <groupId>com.tdunning</groupId>
+ <artifactId>json</artifactId>
+ <version>1.2-SNAPSHOT</version>
— End diff –
Hi @klopfdreh ,
why we can't use the current version 1.1 and wait for 1.2?
Github user klopfdreh commented on the issue:
https://github.com/apache/wicket/pull/195
Hi,
after some research I found out that there is no way to use open-json without API breaks, because it uses the same class names / methods so that you can't redirect the calls, also the API is slightly different in some places. I just tried to implement the missing classes with stubs that only throws RuntimeExceptions so that the API is preserved, but without success.
This also affects the PR https://github.com/apache/wicket/pull/194
I would suggest to go for it anyway, because open-json mostly the same as json.org
Github user klopfdreh commented on the issue:
https://github.com/apache/wicket/pull/194
fyi @martin-g
Github user klopfdreh commented on the issue:
https://github.com/apache/wicket/pull/195
fyi @martin-g
GitHub user klopfdreh opened a pull request:
https://github.com/apache/wicket/pull/195
WICKET-6287 Switch from json.org to open-json - 7.x
There are clirr errors, because of some classes missing in open-json
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/klopfdreh/wicket WICKET-6287_7.x
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/wicket/pull/195.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #195
commit cc057d0cca748c537762b3ed354dc795c455a424
Author: Tobias Soloschenko <tsoloschenko@apache.org>
Date: 2016-11-25T07:29:40Z
WICKET-6287 Switch from json.org to open-json - 7.x
GitHub user klopfdreh opened a pull request:
https://github.com/apache/wicket/pull/194
WICKET-6287 Switch from json.org to open-json - 6.x
There are clirr errors, because of some classes missing in open-json
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/klopfdreh/wicket WICKET-6287_6.x
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/wicket/pull/194.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #194
commit 9be0c0faaa9e6f4343378254ce94755510b688f9
Author: Tobias Soloschenko <tsoloschenko@apache.org>
Date: 2016-11-25T08:18:27Z
WICKET-6287 Switch from json.org to open-json - 6.x
GitHub user klopfdreh opened a pull request:
https://github.com/apache/wicket/pull/193
WICKET-6287 Switch from json.org to open-json - 8.x
Repository: https://github.com/tdunning/open-json
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/klopfdreh/wicket WICKET-6287
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/wicket/pull/193.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #193
commit ee0fd3b0cb4e0102e5969065826d05a11a6f3376
Author: Tobias Soloschenko <tsoloschenko@apache.org>
Date: 2016-11-24T17:47:20Z
WICKET-6287 Switch from json.org to open-json - 8.x
Repository: https://github.com/tdunning/open-json
There are some changes required in open-json, so that it can be used within Apache Wicket:
- JSONFunction class for value as a hack to not surround it with quotes
- Only escape "/" slash after a "<" bracket, because it is allowed
- JDK 1.5
A PR is provided already: https://github.com/tdunning/open-json/pull/1
Commit 733bc395372b1b35a6d5aaa6010c9916995d73d8 in wicket's branch refs/heads/wicket-6.x from Andrea Del Bene
[ https://git-wip-us.apache.org/repos/asf?p=wicket.git;h=733bc39 ]
WICKET-6287more code from openjson/openjson#8