apiVersioning and developerContact implementation for AnkiDroid functions call from WebView #6521

krmanik · 2020-06-21T06:31:22Z

Pull Request template

Purpose / Description

This is implementation for api versioning for usage of AnkiDroid native functions in WebView.

Fixes

Approach

Getting api version & developer contact from card developer, then allow them to call the above functions implementation.

How Has This Been Tested?

Tested on Emulator
Calling this function before calling #6307 implemented functions

<script>
   var jsApi = {"version" : "0.0.1", "developer" : "dev@mail.com"};

   var apiStatus = AnkiDroidJS.init(JSON.stringify(jsApi));
   console.log(apiStatus);
   
   api = JSON.parse(apiStatus);
   
   if (api['markCard']) {
      console.log("API mark card available");
   }   

   if (api['toggleFlag']) {
      console.log("API toggle flag available");
   }   
</script>

apiStatus contain enabled/disabled status of api in true/false (boolean value).

Learning (optional, can help others)

https://stackoverflow.com/questions/51513715/node-js-rest-api-versioning-the-right-way
https://stackoverflow.com/questions/32228300/how-to-prevent-my-snackbar-text-from-being-truncated-on-android

Checklist

Please, go through these checks before submitting the PR.

[ x ] You have not changed whitespace unnecessarily (it makes diffs hard to read)
[ x ] You have a descriptive commit message with a short title (first line, max 50 chars).
[ x ] Your code follows the style of the project (e.g. never omit braces in if statements)
[ x ] You have commented your code, particularly in hard-to-understand areas
[ x ] You have performed a self-review of your own code

krmanik · 2020-06-21T06:32:13Z

This is clean PR for api versioning earlier one #6377 have conflicts. This may not have conflicts.

david-allison

@mikehardy These are mostly nits. Your thoughts?

AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.java

david-allison

This looks good to me! Implementer's choice on whether more refactoring should occur.

AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.java

david-allison

Few things that extraction to the class has raised.

RE: getHashCode, it'll be slightly complex. Probably best to remove .equals(), rather than implement it. If you do implement it, do it over the int array.

AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.java

krmanik · 2020-06-27T11:11:42Z

I will update it as suggested.

mikehardy · 2020-06-28T17:08:42Z

Hey guys, SemVer is actually kind of complicated. We can just re-use this library, it already implements the Comparable interface as well: https://github.com/zafarkhaja/jsemver

Let's do that and not add code we don't need to

krmanik · 2020-06-28T19:17:15Z

I have implemented it. It is far more easier than my implementation. But I have learned new things. Thanks

mikehardy · 2020-06-28T21:24:56Z

Yeah, I think there is value in really looking deeply at semver to understand how much they really pack into it and how as even a very small problem domain it is still hard to get right! Then just using (and contributing to) a general-purpose library is the right way to go in in the end :)

krmanik · 2020-06-30T07:33:54Z

Are there any improvements to this?

david-allison

Two comments - we'll need to improve the error messages as they'll confuse users.

david-allison · 2020-06-30T14:41:17Z

AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.java

+    private boolean requireApiVersion(String apiVer) {
+
+        Version mVersionCurrent = Version.valueOf(sCurrentJsApiVersion);
+        Version mVersionSupplied = Version.valueOf(apiVer);


Do you need validation here? What happens with an invalid version?

I think apiVer passed to valueof so may be Sem Ver library may have internal check for invalid input.

What happens? Does it throw an exception? If not, which code path would it take?

Exception handling to be implemented.

david-allison · 2020-06-30T14:42:31Z

AnkiDroid/src/main/res/values/02-strings.xml

+    <string name="api_version_no_developer_contact">This card uses unsupported AnkiDroid features. View the wiki to resolve this manually.</string>
+    <string name="invalid_json_data">Card provided invalid data. %s</string>
+    <string name="valid_js_api_version">Please provide a valid JavaScript API. Current API version is %s</string>
+    <string name="update_js_api_version">Please update JavaScript API version. Current API version is %s</string>


We'll need to go through these strings and make them user-friendly. Can't think about how to do this right now.

Possibly with error codes that can be given to a developer.

So error code should also defined.

We need some way of telling a user "something went wrong, it's not AnkiDroid's fault, please contact the deck author", but with additional information for the deck author to handle.

Okay, I will update it

david-allison · 2020-06-30T18:11:14Z

AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.java

+        Version mVersionCurrent = Version.valueOf(sCurrentJsApiVersion);
+        Version mVersionSupplied = Version.valueOf(apiVer);
+
+        if (mVersionCurrent.equals(mVersionSupplied)) {


@mikehardy - I don't know enough about semVer to handle this one. What do we want here? We're going to get a lot of API version increases, and at what point do we want to say "break all API clients", and how do we provide a warning, or allow clients to update while keeping the legacy code going.

great question. So semVer allows for "compatible version range specification".
You always assume the requested version range will work

Ranges may be unbounded, saying just "anything higher than X.Y.Z is good". In practice most ranges are like that.

You just do a quick match and and if the requested API range is within our current API range, it's good to go. If it is not, then you say "Current AnkiDroid Javascript API is incompatible with the range requested by your template. Contact the author XXX or click here for more info" (or similar)

This link https://nodesource.com/blog/semver-a-primer/ has a good explanation of "Semver Ranges" with examples of how they are specified and what they mean.

Semver by itself is actually not useful without the range specification and comparison.

Thank goodness the library I pointed to implements it already or it would be nasty :-) https://github.com/zafarkhaja/jsemver#semver-expressions-api-ranges

I will update in between ranges.

AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.java

krmanik · 2020-07-27T03:58:35Z

Are there any improvement to these implementations?

…to js-api-version

david-allison

A few queries about variables which aren't reset when a new card is shown, this leaves the Viewer with an incorrect state.

AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.java

david-allison

Two minor requests - but nothing to stop this going in.

Thanks so much for your patience, looks awesome!

david-allison · 2020-08-06T13:06:17Z

AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.java

+                return false;
+            }
+        } catch (Exception e) {
+          Timber.i(e, "requireApiVersion::exception");


Suggested change

Timber.i(e, "requireApiVersion::exception");

Timber.w(e, "requireApiVersion::exception");

david-allison · 2020-08-06T13:06:45Z

AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.java

@@ -3531,12 +3643,58 @@ protected void showTagsDialog() {
        showDialogFragment(dialog);
    }

+    // list of api that can be accessed
+    private final String[] sApiList = {"toggleFlag", "markCard"};


Would be better at the top of the class

david-allison · 2020-08-06T13:10:25Z

Needs a rebase onto current master to fix the CI failure. master was broken for a short while

krmanik · 2020-08-06T15:42:50Z

To be updated

mikehardy

I had two minor nitpicks on variable naming to maintain consistency
But otherwise, I think this is good to go also, I'm excited to see this out there in use!

AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.java

mikehardy

Approved! 🤜 🤛

david-allison requested changes Jun 21, 2020

View reviewed changes

david-allison added the Needs Review label Jun 21, 2020

krmanik and others added 2 commits June 26, 2020 11:06

Revert "Fork Update"

bc30c1d

Update fork

6a3339c

david-allison reviewed Jun 26, 2020

View reviewed changes

AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.java Outdated Show resolved Hide resolved

david-allison requested review from mikehardy and david-allison June 26, 2020 13:09

david-allison reviewed Jun 27, 2020

View reviewed changes

Merge branch 'master' of https://github.com/ankidroid/Anki-Android

c8181c3

Merge branch 'master' of https://github.com/ankidroid/Anki-Android

0d37a81

david-allison reviewed Jun 30, 2020

View reviewed changes

david-allison changed the title ~~apiVersioning and developerContact implementation for AndkiDroid functions call from WebView~~ apiVersioning and developerContact implementation for AnkiDroid functions call from WebView Jun 30, 2020

krmanik added 5 commits July 2, 2020 05:26

api versioning

6184bef

Updated AbstractFlashcardViewer.java, 02-strings.xml

7254e0b

Added Vesrion Compare using Comparable

939e6c1

Added Java SemVer for JS API

638512e

Updated AbstractFlashcardViewer.java, 02-strings.xml

ef8cdcb

krmanik force-pushed the js-api-version branch from 0db6f6e to ef8cdcb Compare July 1, 2020 21:28

krmanik commented Jul 1, 2020

View reviewed changes

AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.java Outdated Show resolved Hide resolved

krmanik requested a review from david-allison July 2, 2020 16:25

krmanik force-pushed the js-api-version branch 2 times, most recently from d127e55 to ef8cdcb Compare July 4, 2020 21:19

Updated 02-strings.xml

474b616

Merge branch 'master' of https://github.com/ankidroid/Anki-Android in…

1399b90

…to js-api-version

david-allison removed the Needs Author Reply Waiting for a reply from the original author label Aug 4, 2020

Update 02-strings.xml

7204912

david-allison reviewed Aug 4, 2020

View reviewed changes

Update AbstractFlashcardViewer.java

793063e

david-allison reviewed Aug 5, 2020

View reviewed changes

AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.java Outdated Show resolved Hide resolved

init js api

c055f95

david-allison added the Needs Review label Aug 5, 2020

david-allison approved these changes Aug 6, 2020

View reviewed changes

Updated

6a2b548

mikehardy requested changes Aug 7, 2020

View reviewed changes

AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.java Outdated Show resolved Hide resolved

AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.java Outdated Show resolved Hide resolved

mikehardy added Needs Author Reply Waiting for a reply from the original author Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) and removed Needs Review labels Aug 7, 2020

Updated

c0ff6c3

david-allison reviewed Aug 7, 2020

View reviewed changes

AnkiDroid/src/main/java/com/ichi2/anki/AbstractFlashcardViewer.java Show resolved Hide resolved

mikehardy approved these changes Aug 7, 2020

View reviewed changes

mikehardy removed Needs Author Reply Waiting for a reply from the original author Pending Merge Things with approval that are waiting future merge (e.g. targets a future release, CI wait, etc) labels Aug 7, 2020

mikehardy merged commit 677eb00 into ankidroid:master Aug 7, 2020

david-allison added this to the 2.13 release milestone Sep 7, 2020

krmanik deleted the js-api-version branch July 10, 2021 21:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

apiVersioning and developerContact implementation for AnkiDroid functions call from WebView #6521

apiVersioning and developerContact implementation for AnkiDroid functions call from WebView #6521

krmanik commented Jun 21, 2020 •

edited

Loading

krmanik commented Jun 21, 2020 •

edited

Loading

david-allison left a comment

david-allison left a comment

david-allison left a comment •

edited

Loading

krmanik commented Jun 27, 2020

mikehardy commented Jun 28, 2020

krmanik commented Jun 28, 2020 •

edited

Loading

mikehardy commented Jun 28, 2020

krmanik commented Jun 30, 2020

david-allison left a comment

david-allison Jun 30, 2020

krmanik Jun 30, 2020

david-allison Jun 30, 2020

krmanik Jun 30, 2020

david-allison Jun 30, 2020

krmanik Jun 30, 2020

david-allison Jun 30, 2020

krmanik Jun 30, 2020

david-allison Jun 30, 2020 •

edited

Loading

mikehardy Jun 30, 2020

krmanik Jun 30, 2020

krmanik commented Jul 27, 2020

david-allison left a comment

david-allison left a comment

david-allison Aug 6, 2020

david-allison Aug 6, 2020

david-allison commented Aug 6, 2020

krmanik commented Aug 6, 2020

mikehardy left a comment

mikehardy left a comment

	Timber.i(e, "requireApiVersion::exception");
	Timber.w(e, "requireApiVersion::exception");

apiVersioning and developerContact implementation for AnkiDroid functions call from WebView #6521

apiVersioning and developerContact implementation for AnkiDroid functions call from WebView #6521

Conversation

krmanik commented Jun 21, 2020 • edited Loading

Pull Request template

Purpose / Description

Fixes

Approach

How Has This Been Tested?

Learning (optional, can help others)

Checklist

krmanik commented Jun 21, 2020 • edited Loading

david-allison left a comment

Choose a reason for hiding this comment

david-allison left a comment

Choose a reason for hiding this comment

david-allison left a comment • edited Loading

Choose a reason for hiding this comment

krmanik commented Jun 27, 2020

mikehardy commented Jun 28, 2020

krmanik commented Jun 28, 2020 • edited Loading

mikehardy commented Jun 28, 2020

krmanik commented Jun 30, 2020

david-allison left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

david-allison Jun 30, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

krmanik commented Jul 27, 2020

david-allison left a comment

Choose a reason for hiding this comment

david-allison left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

david-allison commented Aug 6, 2020

krmanik commented Aug 6, 2020

mikehardy left a comment

Choose a reason for hiding this comment

mikehardy left a comment

Choose a reason for hiding this comment

krmanik commented Jun 21, 2020 •

edited

Loading

krmanik commented Jun 21, 2020 •

edited

Loading

david-allison left a comment •

edited

Loading

krmanik commented Jun 28, 2020 •

edited

Loading

david-allison Jun 30, 2020 •

edited

Loading