Difference between revisions of "JSON RPC Adapter Project"

From CDOT Wiki
Jump to: navigation, search
(Current Status)
 
(46 intermediate revisions by 2 users not shown)
Line 1: Line 1:
<big><big> JSON_Adapter</big></big>
+
{{Admon/obsolete}}
{{NexJ_Express Index}}
+
 
 +
[[Category: NexJ_Past_Projects]][[Category: NexJ Express JSON Adapter]] [[Category: NexJ Express Development]][[Category: NexJ Express]]
 
== Project Goal==
 
== Project Goal==
Develope an adapter to expose NexJ Express Server objects using JSON
+
Develop an adapter to expose NexJ Express Server objects using JSON <br/>
 +
- [https://docs.google.com/document/d/1z8KfOa7sLD6l3g-1djnm6eU4ErfXDcqRX5v7OR0RJho/edit?hl=en&authkey=CJS3xcQP Final Project Proposal]
 +
 
 +
===Contributors===
 +
* [[User:Gbatumbya|Grace Batumbya]]
 +
 
  
 
== Current Status==
 
== Current Status==
'''Phase 10: Code Review 5''' <br/>
+
<big><big>'''Project Completed'''</big></big>
Ongoing...
 
 
<br/>
 
<br/>
 +
  
 
==Project Phases==
 
==Project Phases==
Line 13: Line 19:
 
* Get requirements from NexJ
 
* Get requirements from NexJ
 
* Incorporate requirements from NexJ into Design of JSON Adapater
 
* Incorporate requirements from NexJ into Design of JSON Adapater
* Proposal proposal for review, make changes as by NexJ until approved
+
* Proposal proposal for review, make changes as by NexJ until approved<br/>
 +
- [https://docs.google.com/document/d/1z8KfOa7sLD6l3g-1djnm6eU4ErfXDcqRX5v7OR0RJho/edit?hl=en&authkey=CJS3xcQP Final Project Proposal]
  
 
===Phase 2: Coding (Done) ===
 
===Phase 2: Coding (Done) ===
Line 55: Line 62:
 
* JSONMarshaller
 
* JSONMarshaller
 
** Remove unused methods
 
** Remove unused methods
** Remove visReferencable()</code>, do lookup in TransferObject Marshaller
+
** Remove <code>isReferencable()</code>, do lookup in TransferObject Marshaller
 
* JSONUnmarshaller
 
* JSONUnmarshaller
 
** Use <code>JSONLookup</code> to find unmarshllers
 
** Use <code>JSONLookup</code> to find unmarshllers
Line 66: Line 73:
 
===Phase 7: Code Review 3 (Done) ===
 
===Phase 7: Code Review 3 (Done) ===
 
* Send patch to NexJ for review
 
* Send patch to NexJ for review
===Phase 8: Apply Changes per Code Review 3===
+
===Phase 8: Apply Changes per Code Review 3 (Done)===
 +
*JSONWriter.java:
 +
**Can a <code>StringBuilder</code> be used instead of a <code>StringBuffer</code>? Maybe the same change can be done in <code>TextMarshaller</code> as well? The member field can still be called <code>m_buffer</code>. Using <code>StringBuilder</code> instead of <code>StringBuffer</code> avoids the synchronization overhead.
 +
**writeString: remove the unnecessary conversion to a character array; the length can be accessed via <code>String.length()</code>.
 +
*JSONMarshaller.java
 +
**Can you fill in the comment for parameter <code>"writer"</code> on method <code>"marshal"</code> of interface <code>Marshaller</code>? It is currently marked with <code>TODO</code>.
 +
**You can use the string id <code>err.rpc.mshType</code> instead of <code>err.rpc.json.mshType</code>
 +
**Can you define the <code>Timestamp</code> marshaller first, then the <code>java.util.Date</code> marshaller? Vassiliy wanted the default framework type’s marshaller first, followed by its synonyms.
 +
*JSONParser.java
 +
**Putting parts of the error message in the error arguments Object array that is passed to <code>fail()</code> isn’t localizable. The error message should be part of the value of the string id. String ids are stored in the <code>*.strings files</code>. For string ids used by Java code, the .strings files are in <code>core/src/nexj/core/meta/sys</code>. You need to ensure that there is an entry in this file for each id that you use. These are then translated into French (and possibly other languages) by people who specialize in translating software interfaces. The only things passed in the error arguments Object array should be data.
 +
**There should be a blank line before <code>break</code> and <code>return</code> statements.
 +
*JSONUnmarshaller.java
 +
**For the <code>castTo*</code> methods, can you rename them to <code>get*</code> and have them take a <code>String sKey</code> argument, which is the key to lookup in <code>m_valueMap</code>? This will more closely follow what Vassiliy intended when he said “Rather than directly type casting values retrieved from json map, you can have unmsh accessor methods for the common types, which can also do error checking, so that one does not get a generic type cast error and wonder what went wrong and where.”
 +
**<code>parseObject()</code>: I don’t think you need separate branches of code for handling the first key in the iterator and subsequent keys. Also, for iteration, prefer <code>for()</code> to <code>while()</code>.
 +
*JSONUnmarshallerException.java
 +
**The overload with <code>(String sExpected, Class actual)</code> isn’t localizable. Remove this overload and use the <code>(String sErrCode, Object[] argArray)</code> constructor. In <code>JSONUnmarshaller get*</code> methods, pass an <code>Object[]</code> of length two for the <code>argArray</code>. Ensure that both are strings.
 +
**Please add a string for err.rpc.json.unmsh.CastException to en.strings in core/src/nexj/core/meta/sys. The various parts of the string id should all start with lowercase letters, e.g. CastException à castException. Use a meaningful name instead of “CastException”—CastException is what it is, not what the error is.
 +
*JSONTests.java
 +
**<code>testSerializeException</code> doesn’t need to initialize the <code>marshaller</code> and <code>unmarshaller</code>: they are initialized already by <code>setUp()</code>, which is run before each <code>test*()</code> method.
 +
**please remove the parts of tests that are duplicated. For example, in <code>testServerObjects()</code> you test the serialization of a Symbol called <code>sym</code>, a <code>PrivilegeSet</code> of “480000”, and a <code>PCodeFunction</code> with <code>code</code> 0, 1, 2. But these have already been tested by the <code>testSerializeRequestCheckJSON</code> which uses the m_request object initialized by the superclass.
 +
**Use <code>Primitive.createInteger</code>, <code>Primitive.createLong</code>, etc. instead of <code>new Integer(…), new Long(…)</code>, etc. where possible.
 +
 
 +
===Phase 9: Code Review 4 (Done) ===
 +
* Send patch to NexJ for review
 +
 
 +
===Phase 10: Apply Changes per Code Review 4 (Done) ===
 +
*The <code>m_tokenValue</code> used in some of the error messages won’t always be populated. For example, in <code>parseArray()</code>, <code>m_tokenValue</code> is used, but it will only be valid if the current token is <code>TOKEN_ATOM</code>.
 +
 
 +
*The <code>boolean.invalidToken</code> error message can be removed and you can use just <code>err.parse.unexpectedToken</code> instead, as one doesn’t know for certain that the input is a <code>Boolean</code>. Same for <code>null.invalidToken</code> and <code>number.invalidToken</code>.
 +
 
 +
* <code>parseObject()</code>: the <code>if (sKey == null)</code> check can never succeed, because <code>instanceof</code> should return false if its argument is <code>null</code>.
 +
 
 +
===Phase 11: Code Review 5 (Done) ===
 +
* Send code for review
  
 
==Links==
 
==Links==
 
'''JSON''' <br/>
 
'''JSON''' <br/>
 
http://www.json.org <br/>
 
http://www.json.org <br/>

Latest revision as of 20:34, 26 January 2014

Important.png
This page may be obsolete.
It contains historical information.

Project Goal

Develop an adapter to expose NexJ Express Server objects using JSON
- Final Project Proposal

Contributors


Current Status

Project Completed


Project Phases

Phase 1: Design Proposal (Done)

  • Get requirements from NexJ
  • Incorporate requirements from NexJ into Design of JSON Adapater
  • Proposal proposal for review, make changes as by NexJ until approved

- Final Project Proposal

Phase 2: Coding (Done)

  • JSONMarshaller
    • Develop marshaller to transform NexJ Server Objects into JSON representation
  • JSONUnmarshaller
    • Develop an marshaller to transform JSON representation into NexJ Server Objects
  • JSON Server
    • Develop a JSON Server to expose NexJ Server objects in JSON
  • Demo application that interacts with the Server using JSON
    • Read Server Objects
    • Update Server Objects
    • Create Server objects

Phase 3: Code Review 1 (Done)

  • Send patch to NexJ for review

Phase 4: Make Changes as per Code Review 1 (Done)

  • Refactor JSONSever and TextServer to inherit from GenericCharacterStreamHTTPServer
  • Optimize marshaling of nested Pair objects
  • Rename variables and methods as per NexJ Developer's Guide

Phase 5: Code Review 2(Done)

  • Send patch to NexJ for review

Phase 6: Apply Changes per Code Review 2 (Done)

  • Minor Clean Up
    • Removed unnecessary files
    • Ensure all files use CRLF line-endings
  • Add Servlet Mapping to cert/web.xml
  • GenericCharacterStreamServer
    • Use abstract String getType() for creating error codes
    • Use getLogger() for lazy initialization of logger
  • TextServer
    • Members should be protected
  • JSONWriter
    • Methods names should start with "write"
  • JSONMarshaller
    • Remove unused methods
    • Remove isReferencable(), do lookup in TransferObject Marshaller
  • JSONUnmarshaller
    • Use JSONLookup to find unmarshllers
    • Create DetachableByteArrayOutputStream
    • Change logic in Base64Util.decode(String)
    • Use Base64Util.decode(String) to decode 64 streams
    • Only TO unmarshaller should use remove() otherwise use get() on m_jsonMap
    • Change instances of Collection.toArray() to use toArray(new Object[size])

Phase 7: Code Review 3 (Done)

  • Send patch to NexJ for review

Phase 8: Apply Changes per Code Review 3 (Done)

  • JSONWriter.java:
    • Can a StringBuilder be used instead of a StringBuffer? Maybe the same change can be done in TextMarshaller as well? The member field can still be called m_buffer. Using StringBuilder instead of StringBuffer avoids the synchronization overhead.
    • writeString: remove the unnecessary conversion to a character array; the length can be accessed via String.length().
  • JSONMarshaller.java
    • Can you fill in the comment for parameter "writer" on method "marshal" of interface Marshaller? It is currently marked with TODO.
    • You can use the string id err.rpc.mshType instead of err.rpc.json.mshType
    • Can you define the Timestamp marshaller first, then the java.util.Date marshaller? Vassiliy wanted the default framework type’s marshaller first, followed by its synonyms.
  • JSONParser.java
    • Putting parts of the error message in the error arguments Object array that is passed to fail() isn’t localizable. The error message should be part of the value of the string id. String ids are stored in the *.strings files. For string ids used by Java code, the .strings files are in core/src/nexj/core/meta/sys. You need to ensure that there is an entry in this file for each id that you use. These are then translated into French (and possibly other languages) by people who specialize in translating software interfaces. The only things passed in the error arguments Object array should be data.
    • There should be a blank line before break and return statements.
  • JSONUnmarshaller.java
    • For the castTo* methods, can you rename them to get* and have them take a String sKey argument, which is the key to lookup in m_valueMap? This will more closely follow what Vassiliy intended when he said “Rather than directly type casting values retrieved from json map, you can have unmsh accessor methods for the common types, which can also do error checking, so that one does not get a generic type cast error and wonder what went wrong and where.”
    • parseObject(): I don’t think you need separate branches of code for handling the first key in the iterator and subsequent keys. Also, for iteration, prefer for() to while().
  • JSONUnmarshallerException.java
    • The overload with (String sExpected, Class actual) isn’t localizable. Remove this overload and use the (String sErrCode, Object[] argArray) constructor. In JSONUnmarshaller get* methods, pass an Object[] of length two for the argArray. Ensure that both are strings.
    • Please add a string for err.rpc.json.unmsh.CastException to en.strings in core/src/nexj/core/meta/sys. The various parts of the string id should all start with lowercase letters, e.g. CastException à castException. Use a meaningful name instead of “CastException”—CastException is what it is, not what the error is.
  • JSONTests.java
    • testSerializeException doesn’t need to initialize the marshaller and unmarshaller: they are initialized already by setUp(), which is run before each test*() method.
    • please remove the parts of tests that are duplicated. For example, in testServerObjects() you test the serialization of a Symbol called sym, a PrivilegeSet of “480000”, and a PCodeFunction with code 0, 1, 2. But these have already been tested by the testSerializeRequestCheckJSON which uses the m_request object initialized by the superclass.
    • Use Primitive.createInteger, Primitive.createLong, etc. instead of new Integer(…), new Long(…), etc. where possible.

Phase 9: Code Review 4 (Done)

  • Send patch to NexJ for review

Phase 10: Apply Changes per Code Review 4 (Done)

  • The m_tokenValue used in some of the error messages won’t always be populated. For example, in parseArray(), m_tokenValue is used, but it will only be valid if the current token is TOKEN_ATOM.
  • The boolean.invalidToken error message can be removed and you can use just err.parse.unexpectedToken instead, as one doesn’t know for certain that the input is a Boolean. Same for null.invalidToken and number.invalidToken.
  • parseObject(): the if (sKey == null) check can never succeed, because instanceof should return false if its argument is null.

Phase 11: Code Review 5 (Done)

  • Send code for review

Links

JSON
http://www.json.org