Resolve Opaque Key coherency issues

[kdmccormick]

Development notes

PRs

The OEP:

1. docs: OEP-68 Learning Content Identifiers openedx-proposals#773

openedx-core PRs:

1. feat!: Collection.key -> Collection.collection_code #542
2. feat!: Component.local_key -> Component.component_code #544
3. feat!: Add Container.container_code field #545
4. feat!: Package and Entity keys are now opaque refs #546
5. feat!: ComponentVersionMedia.key -> ComponentVersionMedia.path #547

openedx-platform PR:

• refactor: Upgrade to openedx-core 0.44.0 (for OEP-68) openedx-platform#38402

Follow-ups

• Remove spurious migration swappable dependencies on AUTH_USER (doing this now)
• Opaque Key Simplification? opaque-keys#411
• Consider moving opaque_keys in, as openedx_keys #437
• Add container_type to container's entity_refs
• Standardize argument patterns across openedx_content APIs

Testing

TBC

AI Usage in PRs

I used Claude Code to help me write all the PRs. I developed a plan for the entire set of 5 changes, and then had it break the plan into 5 parts, one for each commit, before having it implement everything. I've reviewed each changeset by hand and tweaked it, both using Claude and on my own. All commits which Claude was involved in have co-author credit.

Original ticket:

Problems and questions

• Inconsistent usage of id, pk, uuid, slug, key throughout learning core
• Confusion between usage keys and our new suite of non-usage-key "keys" (ie, PublishableEntity.key)
• Lack of standard parsing functions for PublishableEntity.key
• Wasted time and energy marshaling between UsageKeys and PublishableEntity.keys
• Should PE keys be usage keys?
• Should we delete PE UUIDs?

Original issue

Working with Learning Core's authoring api is mostly a joy, except for the inconsistent usage of the terms "id", "key", and "slug". This applies to the LC-related objects and models in opaque-keys and edx-platform, as well.

I would propose standardizing on these meanings of each term for all new content-related code, and for any existing code which is not too burdensome to change:

• *_id is an integer primary key.
• *_uuid is a UUID.
• *_slug is a URL-friendly string which identifies something within a local scope. On the Component model, we currently call this is a "local key".
• *_key is a string (or an opaque-keys object) which uniquely identifies something across a whole Open edX instance. Can be composed of slugs, or of other slug-like things like a block/component type.

For example, one change I'd like to make is:

• On the model, Collection.key should be renamed Collection.slug
• The corresponding field on the opaque-keys object, LibraryCollectionLoctator.collection_id should be renamed LibraryCollectionLocator.slug.
• Function arguments and dataclass members should be renamed accordingly.

[ormsbee]

I actually don't think we should be storing site-global identifiers within a LearningPackage at all, since that content could be moved to a different instance entirely. My initial thought was that all key/slug-like identifiers would be local, and that instance-unique identifiers would be derived from them. I'm actually coming to think that even assuming UUIDs would be globally unique was a mistake on my part.

[kdmccormick]

I actually don't think we should be storing site-global identifiers within a LearningPackage at all, since that content could be moved to a different instance entirely.

This makes sense.

My initial thought was that all key/slug-like identifiers would be local, and that instance-unique identifiers would be derived from them

I agree, but calling them "keys" seems confusing to me. The most common use of Learning Core will be as a backend for edx-platform learning content, which almost always uses "key" to mean a global identifier for a learning context or a usage. I found myself tripping over this while developing the Import API. If it weren't for the type hints, it would have been very challenging to keep straight.

[kdmccormick]

I'm actually coming to think that even assuming UUIDs would be globally unique was a mistake on my part.

Interesting. Just trying to understand this-- is this because we'd want to be able to entirely duplicate the entities of one LP into a new LP without worrying about conflicting identifiers, which would work with the keys/slugs but break with uuids?

Are we using the UUIDs at all? If not, and if we see no clear use for them, I wonder if it would make sense to remove them before anyone depends on them.

[bradenmacdonald]

I've generally not used the UUIDs at all, and they seem kind of redundant given we have integer primary keys for intra-Learning Core relationships, and tend to use high-level keys (opaque keys) based on key/slug for everything else.

[bradenmacdonald]

Also worth noting that in the REST APIs I've generally tended to use only the opaque key, but call it id because some frontend tooling expects / only works when you have a field called id.

See https://github.com/openedx/edx-platform/blob/3651b74738012686a66c311d82d65b5c8d4354d6/openedx/core/djangoapps/content_libraries/rest_api/serializers.py#L34-L39 for example.

We usually define a "write-only" field called slug that can be used to set the slug/key on creation, but is not used when reading objects back from the API.

We also have an old TODO here to rename a legacy definition_id field from the blockstore era to slug: https://github.com/openedx/edx-platform/blob/3651b74738012686a66c311d82d65b5c8d4354d6/openedx/core/djangoapps/content_libraries/rest_api/serializers.py#L196-L204

The REST API also has an inconsistency with collections that I didn't catch in time. While the other objects (blocks, containers, etc.) all use opaque keys in their REST API URLs, e.g. /api/xblock/v2/xblocks/lb:BradenX:LC1:problem:example-prolem@draft/fields/ for an XBlock, the Collections API uses a pair of key and the library opaque key, e.g. /api/libraries/v2/lib:BradenX:LC1/collections/basic-collection/. This is despite us having a LibraryCollectionLocator that we could be using to be similar to all the other library APIs.

[kdmccormick]

Renaming this to "Key coherency" in order to capture the variety of discussions and issues around this

[kdmccormick]

@ormsbee @bradenmacdonald I'm becoming increasingly convinced that this is a major issue with Learning Core as it stands today. Here's an instance where an experienced edx-platform CC is mixing up UsageKeys and PE keys while having to hand-roll logic to parse the latter and convert it to the former: openedx/openedx-platform#37405 (comment) (sorry to use you as an example if you're reading this Navin, my point is that the API is confusing, and if you made this mistake then many many devs will in the future :)

With OpaqueKeys, we get parsing and serialization, we get type checking, we get inheritance. We can say things like "a UsageKey is a ContextKey plus a block key", and that continues to work when we introduce new kinds of LearningContexts. We can coherently think and say things like "this should be a BlockKey, not a UsageKey" and this makes sense and is unambiguous. By going back to stringly-typed keys, we're throwing that all away.

I understand that we want LC to be generic, so we are hesitant to introduce a very Open-edX-ey thing (OpaqueKeys) to it. Still, could we introduce some structured key data types into learning core, so that we can parse/serialize them consistently and having coherent shared language?

[bradenmacdonald]

With the XBlock API, there was a goal to keep the core API LMS-agnostic (because of Google Course Builder and other theoretical LMSs), but that never paid off in any way. Instead, the result was that the XBlock API was insufficient to do anything useful, and you had to supplement it with the un-specified, undocumented edx-platform XBlock extensions (the emergent API).

It seems we may now be doing something similar with Learning Core where you can't do much useful with the Core APIs that don't know anything about UsageKeys or libraries, so you have to use the high-level wrappers from edx-platform to do anything at all, which is opposite to the goal of only needing the Learning Core API to do useful things.

I think there are three core problems to solve that will unlock the desired simplicity and value:

1. Opaque Keys. I don't think anyone loves opaque keys. The name is awkward, the import syntax is long and hard to remember (from opaque_keys.edx.locator import LibraryLocatorV2), the key-vs-locator distinction is confusing, and there's a ton of deprecated/legacy code within the key codebase. But, tthey provide a ton of benefits that Kyle outlined ("we get parsing and serialization, ... this makes sense and is unambiguous"). Because Learning Core has avoided opaque keys, we are "forced" to create wrapper APIs for everything in edx-platform, e.g. the content_libraries app.
   I suggest we consider something like the following:

   • Embrace opaque keys in Learning Core to get the benefits that Kyle mentioned.
   • Make a major cleanup of Opaque Keys, removing all the code that's been deprecated for years. Make they key-vs-locator thing much simpler or even combine them as aliases of each other.
   • Consider making opaque keys available from Learning Core via more sensible module path, e.g. from openedx_learning.keys import CourseKey (instead of from opaque_keys.edx.keys import CourseKey)
   • Publish a visual diagram of the key hierarchy in the opaque keys README.

2. Events. Because Learning Core doesn't emit events, it forces us to create/use high level wrapper APIs - see Fire events from openedx_content so that downstream effects can be handled in platform #462

3. Users and Authentication. Because Learning Core is forbidden from asking questions about authorization, it forces us to create/use high-level wrapper REST APIs. I hope that openedx-authz could help solve this in the future, but I have no idea if that has been accounted for.

---

But even when all that is in place, I wonder if concepts like "content libraries" and "upstream/downstream syncing" also need to be moved into learning core, or can you do useful things with just "learning package" and "components" ?

[ormsbee]

I'm going to avoid discussing (2) and (3) for now to keep this thread on topic, though I agree they are points that need more discussion.

I think I'm coming around to the idea of adding opaque-key-awareness in Learning Core, but I'd want to have it as a translation layer. I really do not want to store system-global UsageKeys because I think that will give us a lot of grief if we start storing varying Course Runs in Libraries. I don't want to get into a place where LC assumes that it can create and store a UsageKey based on the LearningPackage.key and the publishable entity key, when in fact the UsageKeys necessary to run courses will be constructed differently (i.e. UsageKeys can be M:1 to PublishableEntities).

[navinkarkera]

Here's an instance where an experienced edx-platform CC is mixing up UsageKeys and PE keys while having to hand-roll logic to parse the latter and convert it to the former: openedx/openedx-platform#37405 (comment) (sorry to use you as an example if you're reading this Navin, my point is that the API is confusing, and if you made this mistake then many many devs will in the future :)

@kdmccormick No worries, it is indeed confusing and in this specific case I was also lazy and did not check the actual column data before naming the variable, so that is my bad. I do agree that not having a standard way to parse entity keys to UsageKeys is not optimal.