Skip to content

Conversation

mjgallag
Copy link
Contributor

General items:

If your code changes how something works on the device (i.e., it affects the companion):

  • I branched from ucr
  • My pull request has ucr as the base

Further, if you've changed the blocks language or another user-facing designer/blocks API (added a SimpleProperty, etc.):

  • I have updated the corresponding version number in appinventor/components/src/.../common/YaVersion.java
  • I have updated the corresponding upgrader in appinventor/appengine/src/.../client/youngandroid/YoungAndroidFormUpgrader.java (components only)
  • I have updated the corresponding entries in appinventor/blocklyeditor/src/versioning.js

For all other changes:

  • I branched from master
  • My pull request has master as the base

What does this PR accomplish?

Description

Fixes # .

Resolves # .

Copy link
Contributor

@mark-friedman mark-friedman left a comment

Choose a reason for hiding this comment

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

I'm not as well acquainted with the App Inventor client-server Java code as I once was, and of course it has changed since then, so I won't comment much on the particulars. The overall intent and structure looks generally good to me.

I am reminded, though, how wordy and over-engineered Java code can be (or at least App Inventor's Java code). I suppose I have no one to blame but myself, for much of that, though ;-)

protected static final String REST_BASE = "/rest/v1";
private static final Map<Class<?>, ResponseParser> RESPONSE_PARSERS = new HashMap<>();
static {
RESPONSE_PARSERS.put(Config.class, (json) -> new Config(json));
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you planning for there to be additional response classes and response parsers? I only see the one here. Otherwise, this may be premature abstraction.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, one for each rpc object. You can't instainte from *.class in gwt client side ... unless I am missing something ...

@mjgallag
Copy link
Contributor Author

@jisqyv you had mentioned I should check into this when you were looking over my code. I don't think I need to do anything as I send relative URLs with no protocol & host to com.google.gwt.http.client.RequestBuilder so it should just essentially be using Window.Location.getProtocol() + "//" + Window.Location.getHost(). Since setupOrigin does nothing unless service instanceof ServiceDefTarget it should just correctly do nothing when com.google.appinventor.shared.rpc.user.UserInfoService is replaced with com.google.appinventor.client.rest.UserRestService.

setupOrigin(projectService);
setupOrigin(userInfoService);
setupOrigin(getMotdService);
setupOrigin(componentService);
setupOrigin(adminInfoService);
setupOrigin(tokenAuthService);

public static void setupOrigin(Object service) {
if (service instanceof ServiceDefTarget) {
String host = Window.Location.getProtocol() + "//" + Window.Location.getHost();
String oldUrl = ((ServiceDefTarget)service).getServiceEntryPoint();
if (oldUrl.startsWith(GWT.getModuleBaseURL())) {
String newUrl = host + "/" + GWT.getModuleName() + "/" + oldUrl.substring(GWT.getModuleBaseURL().length());
((ServiceDefTarget)service).setServiceEntryPoint(newUrl);
}
}
}

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