Skip to content

Google App Engine Support#198

Merged
xerial merged 9 commits intomsgpack:v07-developfrom
xerial:v07-no-unsafe
Feb 15, 2015
Merged

Google App Engine Support#198
xerial merged 9 commits intomsgpack:v07-developfrom
xerial:v07-no-unsafe

Conversation

@xerial
Copy link
Copy Markdown
Member

@xerial xerial commented Feb 10, 2015

Fixes for #194:

  • Refactored MessageBuffer to isolate the initialization steps between standard and Java6/Android/GAE platforms.
  • Extracted DirectBufferAccess code to a new file.

I confirmed with this fix, we can initialize MessageBuffer class in GAE platform.

@xerial xerial changed the title Google App Engine Support #194 Google App Engine Support Feb 10, 2015
@xerial
Copy link
Copy Markdown
Member Author

xerial commented Feb 11, 2015

@komamitsu
Could you review the changes? Thanks in advance.

@xuwei-k
Copy link
Copy Markdown
Contributor

xuwei-k commented Feb 11, 2015

I haven't been able to understand all changes, but I confirmed this code works on Google App Engine.
nice work 👍

xuwei-k/msgpack-json@00c289901a26396498f8ab

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: This variable isn't used

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed. Thanks

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about something like this to make it more robust?

try {
    if (!isUniversalBuffer) {
            :
        boolean isLittleEndian = ByteOrder.nativeOrder() == ByteOrder.LITTLE_ENDIAN;
         if(isLittleEndian) {
             bufferClsName = "org.msgpack.core.buffer.MessageBuffer";
         }
         else {
             bufferClsName = "org.msgpack.core.buffer.MessageBufferBE";
         }
    }
}
catch {
   // Let's use MessageBufferU
}
if (bufferClsName == null) {
    bufferClsName = "org.msgpack.core.buffer.MessageBufferU";
        :
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. With this fix, msgpack-java works anyway. To do this, I need to put the initialization code of static fields (bufferClsName, unsafe, etc.) after the last catch call.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 5fd6964

@komamitsu
Copy link
Copy Markdown
Member

Looks good

xerial added a commit that referenced this pull request Feb 15, 2015
@xerial xerial merged commit 119be9d into msgpack:v07-develop Feb 15, 2015
@komamitsu
Copy link
Copy Markdown
Member

It worked on Android 2.3.3, 4.0.4 and 5.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.

3 participants