-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce Oppia proto API v1: Android #1
base: develop
Are you sure you want to change the base?
Changes from 43 commits
1bfefe8
a69a240
b41cf7d
be028d2
0ab3fc3
abbdf13
c1e97fd
0e921d1
f6b3210
47dfb60
8dc3eb1
a5edbf5
13fad56
fa11822
cdf7150
0231af8
ab70643
0b2896c
5d868e2
0b937e3
5ceec82
78d06fa
ee332e2
ee5b117
b10a3a4
9afead9
97a134d
cdd8c67
fcf4a60
eabbdf2
937c883
458d489
0250c69
1b3ef9e
bb559ea
886e1fc
8f3cde8
4ae1907
bc78dd9
6c3b65a
58b1b7b
1c129b7
7f766e1
7219d37
8bcb631
4ea008b
cb3434e
87422ba
98d3122
9cf993e
f6d167c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
4.0.0 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
bazel-* |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
""" | ||
Top-level definitions for Oppia's proto API project. Only libraries that are part of the project's | ||
public, importable API should be defined here (and all such libraries should be defined here and | ||
nowhere else). | ||
""" | ||
|
||
load("@com_github_bazelbuild_buildtools//buildifier:buildifier.bzl", "buildifier") | ||
load("@rules_java//java:defs.bzl", "java_lite_proto_library", "java_proto_library") | ||
load("@rules_proto//proto:defs.bzl", "proto_library") | ||
load("//defs:defs.bzl", "BUILDIFIER_LINT_WARNINGS", "define_buildifier_tests") | ||
|
||
package_group( | ||
name = "api_visibility", | ||
packages = [ | ||
"//", | ||
], | ||
) | ||
|
||
package_group( | ||
name = "proto_impl_visibility", | ||
packages = [ | ||
"//org/oppia/proto/...", | ||
], | ||
) | ||
|
||
# The protos needed to interact with Oppia's proto API. This is meant to be used in cases when these | ||
# protos are used by domain-specific protos in downstream projects. | ||
# NOTE TO DEVELOPERS: 'deps' specifically needs to be used (versus just 'exports') since downstream | ||
# proto_library targets otherwise won't pull in the protos, and language libraries need it to | ||
# actually generate corresponding proto code. | ||
proto_library( | ||
name = "android_protos", | ||
visibility = ["//visibility:public"], | ||
deps = [ | ||
"//org/oppia/proto/v1/api:android_proto", | ||
"//org/oppia/proto/v1/structure:structure_proto", | ||
"//org/oppia/proto/v1/versions:versions_proto", | ||
], | ||
) | ||
|
||
# The Java versions of the protos needed to interact with Oppia's proto API. | ||
java_proto_library( | ||
name = "android_java_protos", | ||
visibility = ["//visibility:public"], | ||
deps = [":android_protos"], | ||
) | ||
|
||
# The Java lite versions of the protos needed to interact with Oppia's proto API. | ||
java_lite_proto_library( | ||
name = "android_java_lite_protos", | ||
visibility = ["//visibility:public"], | ||
deps = [":android_protos"], | ||
) | ||
|
||
buildifier( | ||
name = "fix_bazel_lint_problems", | ||
lint_mode = "fix", | ||
lint_warnings = BUILDIFIER_LINT_WARNINGS, | ||
) | ||
|
||
define_buildifier_tests() |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,42 @@ | ||
# oppia-android-api | ||
Published Android-specific API for the Oppia backend. | ||
# oppia-proto-api | ||
Published common proto API for the Oppia server/client. | ||
|
||
This API governs the transfer of data between the Oppia Python server and the Android app. | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @BenHenning I think it might be worth adding notes in this README on "If you feel that you need to update a proto version, this is the specific process to follow". The procedure should specify what things to think about (e.g. maintain backwards compatibility, grep for any comment with the name of the proto you're modifying since there may be dependent protos, etc.), and also include non-coding steps for getting sign-off from the tech lead, communicating with the leads of the client repos, etc. Will proto versions just get incremented or do we need to preserve information about the older versions in some way? Having this written down would help ensure that everyone is on the same page about the upgrade process -- I'm currently having a bit of trouble auditing it / thinking through it during review, because I don't quite know what it is. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ack. I plan to take a stab at writing a full README in a later revision (didn't quite get to it since covering the rest of the documentation was rather involved). |
||
### API Versions | ||
Each version of this API is formatted as a major & minor version (e.g. 1.0) and corresponds to a separate release. | ||
|
||
- The major version is represented directly in the directory structure (e.g. 'v1'), and is only incremented if a breaking change must be introduced into the API. (Note that this repository might house multiple major versions, in order to maintain long-term backward compatibility with older API versions.) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is the major version specified in code anywhere? (If so, where? Suggest explaining in the doc.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ack. I plan to take a stab at writing a full README in a later revision (didn't quite get to it since covering the rest of the documentation was rather involved). |
||
- The minor version is incremented whenever a compatible change to the API is released. It is only ever represented in the release numbers, and never in code form. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does "change to the API" mean "change to any proto version defined in this repository"? If so, maybe state that explicitly. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ack. I plan to take a stab at writing a full README in a later revision (didn't quite get to it since covering the rest of the documentation was rather involved). |
||
|
||
Because protobuf is backward/forward interoperable, incompatibilities are unlikely as long as proto versions are respected (though, even in those cases, there should be potential for graceful failures). | ||
|
||
### Content Versions | ||
Content versions represent the version of the corresponding entity instance (e.g. a topic, skill, or exploration). These are used to track whether a particular structure has content updates. They map to the versions stored in the corresponding structure's VersionedModel in the backend. | ||
|
||
### Proto Versions | ||
Proto versions correspond to the proto structures defined in this repository. They only need to be incremented when a proto structure is updated in such a way that the default data will break existing logical assumptions (thus requiring a data migration). Such upgrades should always happen in a compatible way (except for major API version upgrades -- see the "API Version" section above). | ||
|
||
Three notes: | ||
|
||
1. Some proto versions correspond to **groups** of substructures (such as SubtitledHtml, or other language-based substructures), because these substructures are shared across high-level structures. | ||
2. There is no need to version interactions separately. This is because their structure and how they relate to the exploration/question experience is implied as part of State's structure version. | ||
3. If any changes happen in the proto structure which are getting tracked, the version specified in the comment must be incremented. | ||
|
||
The following are the list of structure types whose versions are tracked: | ||
- TopicSummaryProtoVersion | ||
- RevisionCardProtoVersion | ||
- ConceptCardProtoVersion | ||
- ExplorationProtoVersion | ||
- QuestionProtoVersion | ||
- StateProtoVersion | ||
- LanguageProtosVersion | ||
- ImageProtoVersion | ||
|
||
## Support | ||
If you have any feature requests or bug reports, please log them on our [issue tracker](https://github.com/oppia/oppia-proto-api/issues/new?title=Describe%20your%20feature%20request%20or%20bug%20report%20succinctly&body=If%20you%27d%20like%20to%20propose%20a%20feature,%20describe%20what%20you%27d%20like%20to%20see.%0A%0AIf%20you%27re%20reporting%20a%20bug,%20please%20be%20sure%20to%20include%20the%20expected%20behaviour,%20the%20observed%20behaviour,%20and%20steps%20to%20reproduce%20the%20problem.%20Console%20copy-pastes%20and%20any%20background%20on%20the%20environment%20would%20also%20be%20helpful.%0A%0AThanks!). | ||
|
||
Please report security issues directly to [email protected]. | ||
|
||
## Licence | ||
The Oppia-Proto-API code is released under the [Apache v2 license](https://github.com/oppia/oppia-proto-api/blob/master/LICENSE). |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
""" | ||
The top-level WORKSPACE definition for the Oppia proto API Bazel workspace. | ||
""" | ||
|
||
load("//repo:deps.bzl", "initializeDepsForWorkspace") | ||
|
||
initializeDepsForWorkspace() | ||
|
||
load("//repo:toolchains.bzl", "initializeToolchainsForWorkspace") | ||
|
||
initializeToolchainsForWorkspace() |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
""" | ||
Starlark macros that help ensure clean proto & Bazel dependency management. | ||
""" | ||
|
||
exports_files(["buf.yaml"]) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
version: v1 | ||
lint: | ||
use: | ||
- DEFAULT | ||
except: | ||
- PACKAGE_VERSION_SUFFIX | ||
- ENUM_VALUE_PREFIX |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
""" | ||
Defines Starlark macros that are used to define protos and set up Bazel build directories (and | ||
automatically define Buf & Buildifier lint tests). | ||
""" | ||
|
||
load("@com_github_bazelbuild_buildtools//buildifier:buildifier.bzl", "buildifier_test") | ||
load("@rules_buf//buf:defs.bzl", "buf_lint_test") | ||
load("@rules_proto//proto:defs.bzl", "proto_library") | ||
|
||
# Configured lint warnings for Buildifier linter tests & autofixing configuration. | ||
BUILDIFIER_LINT_WARNINGS = [ | ||
"+out-of-order-load", | ||
"+unsorted-dict-items", | ||
] | ||
|
||
def oppia_proto_library(name, relative_parent_dir_path, **kwargs): | ||
""" | ||
Defines an Oppia API-compatible proto library, and a lint test with the same name ending in | ||
"_lint_test"." | ||
|
||
Args: | ||
name: str. The name of the library being defined. | ||
relative_parent_dir_path: str. The relative directory path to the containing Bazel package. | ||
**kwargs: additional parameters passed into proto_library. | ||
""" | ||
|
||
proto_library( | ||
name = name, | ||
import_prefix = relative_parent_dir_path, # Make directory prefix match declared package. | ||
strip_import_prefix = "", # Strip the default file prefix so that it may be redefined. | ||
**kwargs | ||
) | ||
|
||
buf_lint_test( | ||
name = "%s_lint_test" % name, | ||
config = "//defs:buf.yaml", | ||
targets = [name], | ||
) | ||
|
||
def define_buildifier_tests(): | ||
""" | ||
Defines a Bazel lint test for this package with the name "bazel_lint_check_test". | ||
""" | ||
|
||
buildifier_test( | ||
name = "bazel_lint_check_test", | ||
srcs = native.glob(["WORKSPACE"]) + native.glob(["BUILD"]) + native.glob(["*.bazel"]) + native.glob(["*.bzl"]), | ||
lint_mode = "warn", | ||
lint_warnings = BUILDIFIER_LINT_WARNINGS, | ||
verbose = True, | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
""" | ||
The top-level directory for the Oppia proto API. Generally, no actual proto libraries should be | ||
defined at this level. Instead, protos should go into the corresponding version in which they | ||
belong. See the README for more details on the API is versioned. | ||
""" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
""" | ||
Version 1 of the Oppia proto API. See the README for specifics on how the API is versioned. | ||
""" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
""" | ||
Libraries that define endpoint API protos for interacting with Oppia's backend. | ||
""" | ||
|
||
load("//defs:defs.bzl", "define_buildifier_tests", "oppia_proto_library") | ||
|
||
oppia_proto_library( | ||
name = "android_proto", | ||
srcs = ["android.proto"], | ||
relative_parent_dir_path = "org/oppia/proto/v1/api", | ||
visibility = ["//:api_visibility"], | ||
deps = [ | ||
"//org/oppia/proto/v1/structure:structure_proto", | ||
"//org/oppia/proto/v1/versions:versions_proto", | ||
], | ||
) | ||
|
||
define_buildifier_tests() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@BenHenning flagging this and other similar TODOs for you to look at.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in the latest changes.