Skip to content

feat: Remote Instance Catalog Fetch - #676

Merged
RawanMatar89 merged 9 commits into
openedx:developfrom
zeit-labs:infra/03-remote-config-fetch
Sep 21, 2026
Merged

RawanMatar89 merged 9 commits into
openedx:developfrom
zeit-labs:infra/03-remote-config-fetch

Conversation

@RawanMatar89

@RawanMatar89 RawanMatar89 commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Description

Adds the ability to fetch the instance catalog from a remote URL at launch, with the bundled catalog as an offline fallback. Also retires the YAML config format — config.json is now the app's sole config source, front to back (bundled config, remote catalog, and build-time plist generation all read the same shape).

This PR only adds the fetch/cache/parse machinery. Nothing yet calls InstanceConfigLoader from the launch flow or shows a picker. Until then the app boots straight through on its bundled app-level config, same as before this stack.

What's in this PR

Remote fetch, with an offline-safe merge policy

  • InstanceApiService: plain URLSession GET of INSTANCES_CATALOG_URL (not routed through the authenticated Alamofire/API stack — fixed, anonymous, external endpoint). Returns raw JSON bytes; parsing stays in InstancesConfig.
  • InstanceConfigLoader.load(): baseline = last cached successful fetch, else the bundled catalog, else empty. Every launch awaits a live fetch:
    • Non-empty response → wholesale-replaces the baseline and becomes the new cache.
    • Fetch fails, or succeeds with 0 instances → baseline is returned untouched (a reachable-but-empty catalog must never wipe a good cache).
  • INSTANCES_CATALOG_URL now lives in bundled config.json's app-level keys, read directly and independent of ConfigProtocol (which is being retired there).

Instance schema additions (InstancesConfig.swift)

  • Parses the live/remote JSON shape alongside the bundled shape.
  • All schema keys we own are uniformly UPPER_SNAKE_CASE; a snake_case/mixed remote payload is normalized key-by-key at every nesting depth via remoteToYAMLKeyMap. Palette-internal field names (accent_color, etc.) are deliberately left alone — that's the Theme module's contract.

YAML retirement

  • config.yaml deleted (dev/stage/prod); config_script/process_config.py and whitelabel.py rewritten to read JSON; file_mappings.yaml trimmed.

Temporary compatibility bridge

  • Config.swift still hard-requires API_HOST_URL/SSO_URL/SSO_FINISHED_URL/OAUTH_CLIENT_ID at the top level — it's the sole ConfigProtocol implementation wired into DI until InstanceAwareConfig lands. These 4 keys are restored in config.json's app-level section, mirrored from the example instance, explicitly commented as removable once InstanceAwareConfig ships.

Tests

  • InstancesConfigJSONTests.swift (new): live/remote shape parsing, key normalization, loader baseline/replace/cache semantics (including the "fetch succeeds but empty" case), bundled catalog URL resolution.
  • InstanceStoreTests.swift: updated for the casing change.

Out of scope

  • Wiring InstanceConfigLoader into app launch / DI.
  • Instance picker / selection UI.
  • Removing the temporary Config.swift bridge keys once InstanceAwareConfig exists.

Adds TenantApiService to fetch the tenant catalog JSON from a remote
endpoint, and TenantConfigLoader to orchestrate a live-fetch-first,
cache-fallback policy: try the network first, fall back to the last
successfully-parsed config on failure.

The remote JSON uses lowercase snake_case keys, so TenantsConfig gains
a normalizeRemoteKeys step that maps them to the existing YAML
upper-snake-case keys and reuses Tenant(dictionary:) for parsing —
no separate remote-specific model.

Registers TenantApiServiceProtocol and TenantConfigLoader in
NetworkAssembly. Nothing calls TenantConfigLoader.load() yet; wiring
it into app launch is a later PR.
Renames TenantApiService -> InstanceApiService, TenantConfigLoader ->
InstanceConfigLoader, TenantsConfigJSONTests -> InstancesConfigJSONTests,
and the remote JSON schema's wire keys (tenant_name -> instance_name,
is_switch_tenant_login_enabled -> is_switch_instance_login_enabled,
the TENANTS/tenants catalog wrapper -> INSTANCES/instances), per the
community's naming feedback.
Adds an optional INSTANCES_CATALOG_URL config key and a config.json
bundled with the app. InstanceConfigLoader now falls back to this
bundled catalog when no URL is configured (or the live fetch and
cache both fail), instead of falling straight to an empty catalog.

- ConfigProtocol.instancesCatalogURL: URL? (nil when unconfigured)
- InstanceApiService.url is now optional; throws .notConfigured when nil
- InstanceConfigLoader.loadBundled() reads config.json from the bundle
- process_config.py copies json_files (config.json) into the built
  bundle alongside config.plist
- default_config/{dev,stage,prod}/file_mappings.yaml gain a json_files
  list; only dev ships an actual config.json for now
- Regenerated ConfigProtocolMock (Mockolo) in every module to add the
  new instancesCatalogURL property

Also removes API_HOST_URL_HIDDEN_LOGIN entirely: the Instance field
(baseURLHiddenLogin), its InstanceKeys/CodingKeys entries, remote
JSON key mapping, and the example value in config.yaml/config.json.
It was unused outside InstancesConfig.swift's own parsing.
Same restructuring as infra/01-tenant-model: LOGO_URL/HEADER_BACKGROUND_URL
now live inside THEME (as logo_url/header_background_url, siblings of
light/dark) instead of their own top-level instance fields. Removed the
now-unused remoteToYAMLKeyMap entries for them -- with no map entry they
pass through normalizeRemoteKeys unchanged (lowercase), landing exactly
where Instance.init?(dictionary:) now looks for them.

- default_config/dev/config.json and config.yaml's example instance
  updated to match: THEME.light/dark now carry only accent_color
  (ThemeColorSet.derived(fromHex:light:dark:) derives the rest), logo_url
  moved under THEME, and the flat color field is dropped.
- Updated InstancesConfigJSONTests to the nested shape.
Same fix as infra/01-tenant-model: NAME/COLOR get explicit uppercase raw
values, and THEME's LIGHT/DARK/LOGO_URL/HEADER_BACKGROUND_URL wrapper
keys go uppercase instead of the lowercase shortcut that let them pass
the remote-key normalizer unmapped. Added matching entries to
remoteToYAMLKeyMap for all six so remote catalog responses (still
naturally lowercase/snake_case) keep normalizing correctly -- without
this, a remote instance missing an explicit uppercase NAME would have
silently been dropped as malformed.

Left untouched: the light/dark palette dicts' own internal field names
(accent_color and siblings) -- Theme's already-shipped contract from the
merged theme/01-theme-engine PR, not this file's to rename.

config.json's own casing isn't touched here -- it's about to be
restructured wholesale (app-level keys + INSTANCES wrapper) in the next
commit, so fixing it twice would be wasted work.
…le replace)

Replaces the old live-fetch-first/cache/bundled/empty fallback chain with
product's actual merge rule:

- Baseline = last-cached successful remote response, else the bundled
  catalog, else empty.
- A live fetch that returns >0 instances wholesale-replaces the baseline
  and becomes the new cache -- so a later offline launch shows the same
  catalog the user had last time, not a reset to the bundled default.
- A fetch that fails, or succeeds with zero instances, leaves the
  baseline (and the cache) untouched -- a reachable-but-empty catalog is
  not the same as an unreachable one and must not wipe out a good cache.
- No catalog URL configured -> InstanceApiService throws immediately,
  same fallback-to-baseline path as any other fetch failure.

fatalError guard for "no catalog URL and no valid local instance" is
deliberately not here -- that's launch-sequencing (RouteController), not
loader concern; it belongs with the DI/launch wiring work.
- default_config/{dev,stage,prod}/config.json restructured to
  {app-level keys, INSTANCES}: FIREBASE/FACEBOOK/MICROSOFT/GOOGLE/
  APPLE_SIGNIN/BRANCH/URI_SCHEME/APP_STORE_ID/INSTANCES_CATALOG_URL
  alongside the instance array under INSTANCES. Remote catalog
  responses are unaffected -- still a bare array, only the local file
  gained the wrapper.
- config.yaml deleted in all three environments; nothing read it
  anymore once process_config.py's build-time source moved to JSON
  (dev's INSTANCES block there was already dead code -- confirmed
  nothing but InstanceStoreTests hand-constructs Instance dictionaries
  directly, no runtime path ever parsed it).
- process_config.py: PlistManager now parses json_files via
  json.load() instead of config_files via yaml.safe_load() --
  load_config()/yaml_to_plist() -> load_config()/json_to_plist(),
  otherwise unchanged (same merge_dicts, same plist output shape).
  whitelabel.py's own PlistManager call site (imports the class from
  this file) updated to match -- it was about to silently feed YAML
  paths into a JSON-only loader.
- file_mappings.yaml (all three envs): dropped the now-unused `files:`
  (YAML) list, kept `json_files:`.
- NetworkAssembly.swift: InstanceApiServiceProtocol's URL now comes
  from the bundled config.json directly (new
  InstanceConfigLoader.bundledCatalogURL()), not
  ConfigProtocol.instancesCatalogURL -- that property is left in place
  but unused for now, to be removed together with the rest of
  ConfigProtocol's app-level shrinkage (firebase/facebook/etc. moving
  off YAML-via-Config) rather than regenerating all 9 Mockolo
  ConfigProtocolMocks twice for two separate small removals.

Deliberately NOT touched: config_settings.yaml and file_mappings.yaml
themselves stay YAML -- build-routing metadata, not app config content,
never part of what was agreed to move to JSON. Documentation/
CONFIGURATION_MANAGEMENT.md still describes the old YAML-based flow in
detail and needs a real rewrite, not just the one-line README pointer
fixed here -- flagging rather than attempting that in this commit.
…IENT_ID as a temporary bridge

Config (the sole ConfigProtocol implementation still wired into DI
pre-PR-9) hard-fatalError()s on launch if these 4 top-level plist keys
are missing. Removing them from config.yaml in the YAML-retirement
commit broke app launch entirely in the current (PR-4-without-PR-9)
state, since InstanceAwareConfig doesn't exist yet to take over.

Bridge values mirror the example instance's own values (dev) / the
same placeholder pattern already used elsewhere in this file (stage,
prod). Remove once PR-9's InstanceAwareConfig replaces Config as the
wired-in ConfigProtocol and these top-level keys are no longer read.
@openedx-webhooks openedx-webhooks added open-source-contribution PR author is not from Axim or 2U core contributor PR author is a Core Contributor (who may or may not have write access to this repo). labels Sep 15, 2026
@openedx-webhooks

openedx-webhooks commented Sep 15, 2026

Copy link
Copy Markdown

Thanks for the pull request, @RawanMatar89!

This repository is currently maintained by @openedx/openedx-mobile-maintainers.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

  • If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
    • This process (including the steps you'll need to take) is documented here.
  • If it doesn't, simply proceed with the next step.
🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

  • Dependencies

    This PR must be merged before / after / at the same time as ...

  • Blockers

    This PR is waiting for OEP-1234 to be accepted.

  • Timeline information

    This PR must be merged by XX date because ...

  • Partner information

    This is for a course on edx.org.

  • Supporting documentation
  • Relevant Open edX discussion forum threads
🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details
Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

  • The size and impact of the changes that it introduces
  • The need for product review
  • Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

@github-project-automation github-project-automation Bot moved this to Needs Triage in Contributions Sep 15, 2026
@RawanMatar89
RawanMatar89 marked this pull request as ready for review September 15, 2026 13:20
@codecov

codecov Bot commented Sep 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.00%. Comparing base (0c72bd4) to head (62c5488).

Additional details and impacted files
@@      Coverage Diff       @@
##   develop   #676   +/-   ##
==============================
==============================

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

IvanStepanok
IvanStepanok previously approved these changes Sep 17, 2026

@IvanStepanok IvanStepanok left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@RawanMatar89
RawanMatar89 dismissed IvanStepanok’s stale review September 20, 2026 16:33

The merge-base changed after approval.

…config-fetch

# Conflicts:
#	Core/Core.xcodeproj/project.pbxproj
#	Core/Core/Configuration/Config/InstancesConfig.swift
#	default_config/dev/config.yaml
@RawanMatar89
RawanMatar89 merged commit 377f981 into openedx:develop Sep 21, 2026
8 checks passed
@github-project-automation github-project-automation Bot moved this from Ready for Review to Done in Contributions Sep 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core contributor PR author is a Core Contributor (who may or may not have write access to this repo). open-source-contribution PR author is not from Axim or 2U

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants