Skip to content
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

SOLR-17062: Create JS client for Admin UI use #2050

Merged
merged 9 commits into from
Nov 13, 2023

Conversation

gerlowskija
Copy link
Contributor

https://issues.apache.org/jira/browse/SOLR-17062

Description

Currently, Solr only offers one "first-party" client, SolrJ, which can only be used within JVM environments. This is obviously limiting for our users.

Historically, keeping client bindings in multiple languages up to date has been too daunting for the project to undertake. But now that Solr produces an OpenAPI spec for its upcoming "v2" API, we have the ability to generate client bindings from that for a variety of languages with very little additional maintenance cost.

There's extra incentive for us to experiment with languages that we ourselves use internally, as that gives us the opportunity to vet/dogfood the generated clients in our own code.

Solution

This PR uses the 'openapi-generator' gradle plugin to create a Javascript client based on our OpenAPI spec. A few details:

  • ./gradlew solr:api:buildJSClient to generate the client
  • We're relying on the OpenAPI Generator project's 'javascript-closure-angular' generator here, but there are other options available if this is a bad fit. In particular, the project offers a vanilla 'javascript' generator which might serve us better?
  • The generated client only supports those APIs that are represented in our OpenAPI spec - i.e. the v2 APIs that have been converted to JAX-RS. Currently, this is limited to select admin APIs. Users that cannot find an API they'd like should consider migrating the code for that API to JAX-RS.
  • Once created, client is available at solr/api/build/generated/js. Not sure how packaging works in Javascript code, but the generated code doesn't appear to be packaged in any way - it's "just" a bundle of JS files.

Tests

The code-generator has an option to generate tests for the generated client code. I've disabled that currently, trusting the generator to do its thing, but we can enable that if it's something we'd feel more comfortable using.

Checklist

Please review the following and check all that apply:

  • I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
  • I have created a Jira issue and added the issue ID to my pull request title.
  • I have given Solr maintainers access to contribute to my PR branch. (optional but recommended)
  • I have developed this patch against the main branch.
  • I have run ./gradlew check.
  • I have added tests for my changes.
  • I have added documentation for the Reference Guide

This uses the 'javascript-closure-angular' generator from the OpenAPI
Generators project.  Not sure if this is the right choice - they also
offer a vanilla 'javascript' option that might be more suitable.

As a JS novice, I'm not in a great position to evaluate the client
itself.  But I can offer it up as a draft PR and see whether others more
familiar with the ecosystem can vet the generated code at all.
@madrob
Copy link
Contributor

madrob commented Oct 30, 2023

@MarcusSorealheis care to take a look?

@MarcusSorealheis
Copy link
Contributor

I'll take a look now.

// Non-Java client generation tasks below:

task buildJSClient(type: org.openapitools.generator.gradle.plugin.tasks.GenerateTask) {
generatorName.set("javascript-closure-angular")
Copy link
Contributor

Choose a reason for hiding this comment

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

@gerlowskija a few thoughts:

  1. I strongly recommend to use the typescript or javascript generators rather than one that ties users to a specific vendor. Specifically, AngularJS is a deprecated project and the version Solr was built on doesn't even exist anymore. The Angular that sort of lives on is written in Typescript.

  2. The typescript option is listed in the docs as experimental, though I doubt it would be very experimental and typescript-node is labeled stable. In reality, both are probably more stable than thejavascript option. The javascript option may offer a slightly lower barrier to entry for future contributors for now, while the typescript one will have a growing ecosystem and offer far better maintainability in the future.

  3. The biggest benefits of the Typescript language are maintenance and developer experience, especially for a team of Java users.

Copy link
Contributor

Choose a reason for hiding this comment

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

Given we want to dog-food our current Admin UI first, perhaps the pure javascript client fits best. But for sure, once we plan to release something for customer use, TS bindings would likely be requested. Do we know whether the "typescript" generator is identical to the javascript generator plus types, or are they separate templates?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I strongly recommend to use the typescript or javascript generators

👍 - I chose 'javascript-closure-angular' somewhat arbitrarily. I knew in a vague sense that the Admin UI used Angular, and just went with that. But I like your point about vendor-neutrality. Will update this to use the 'javascript' generator.

(I was under the impression that typescript was a distinct language from Javascript, and that the two weren't necessarily interchangeable. But I'm getting the sense from you guys above that there's more interplay there than I'd imagined. I've got some reading to do I think...)

Do we know whether the "typescript" generator is identical to the javascript generator plus types, or are they separate templates?

I don't know, but it's something I can check on. The default templates for each generator are in subdirectories here, so we'd just need to find the two relevant dirs and diff them.

@MarcusSorealheis
Copy link
Contributor

MarcusSorealheis commented Oct 30, 2023

One final point:

Overall, this is a very positive and important effort. There's a wave of Typescript and Python developers at companies using Solr that would love to extend it directly, and a generated client would facilitate an easy integration point with low maintenance.

@epugh
Copy link
Contributor

epugh commented Oct 31, 2023

I liked that the http client that https://github.com/OpenAPITools/openapi-generator/blob/master/modules/openapi-generator/src/main/resources/Javascript/libraries/javascript/package.mustache uses has had a recent release.... superagent is new to me, but as long as it's maintained ;-).

Copy link
Contributor

@MarcusSorealheis MarcusSorealheis left a comment

Choose a reason for hiding this comment

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

I've thought about it. Here's my final recommendation, though you can do as you please:

  1. Create a JS Client using javascript (this PR)
  2. Create a TS Client using typescript (a new PR)
  3. Create a Python Client (this PR)

That way, we support the current UI, leave room for and encourage a new UI, and all the data scientists writing Python can extend Solr in interesting ways.

@gerlowskija gerlowskija marked this pull request as ready for review November 6, 2023 21:03
@gerlowskija
Copy link
Contributor Author

Alright, I've updated the PR to use the 'javascript' generator. The generated code looks simple enough, but I'm no JS expert.

I think my next steps here would be to look into whatever build logic is necessary to make this generated code available to the Admin UI, and then maybe change one API call in the admin UI to use the new JS client. That gives us a minimal bit of dog-fooding, and does much of the plumbing so that anyone else interested in the Admin UI can do more.

Notably, that doesn't involve publishing the client, making it a release artifact, etc. We can go down those roads in a subsequent ticket if we start getting demand for a JS client or if we're comfortable enough with the dog-fooding that we're confident it'll be worth the maintenance burden to publish in some broader way.

@epugh
Copy link
Contributor

epugh commented Nov 7, 2023

I think your idea about not publishing it as an artifact till we've tried it out makes sense!

@@ -17,6 +17,7 @@

plugins {
id 'io.swagger.core.v3.swagger-gradle-plugin' version '2.2.2'
id "org.openapi.generator" version "6.0.1"
Copy link
Contributor

Choose a reason for hiding this comment

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

BTW, I looked and there is a 7.0.1 versin https://plugins.gradle.org/plugin/org.openapi.generator, maybe we have to update gradle?

NOTE: DOESN'T WORK!  Currently on loading the Admin UI, solr/index.js
triggers a number of import errors.  At a glance, this appears to be
because the generated Javascript file uses a style of import that omits
the file-extension (i.e. '.js').  AFAICT, this extension-less style of
import is common in node.js apps, or when using a 'npm' packaged module,
but doesn't work well when using "unbundled" JS files.

We can change the generation template to include the '.js' suffix in
imports, but I'm not sure if that's the right approach.

Once the import issues are resolved, this commit includes code changes
to switch the Admin UI's collection-deletion over to using the v2 API.
Hard to test or validate this code until the import issues are resolved,
but for now it shows what example usage should looks like.
@gerlowskija
Copy link
Contributor Author

gerlowskija commented Nov 9, 2023

Alright, made some good progress here:

  1. The generated JS client is is produced via the solr:api:buildJSClient task, and assemblePackaging now copies the generated JS files into a sub-dir of the webapp's libs/ directory. So the Admin UI should be able to access it, if it can find the right way to import it (see below)
  2. I've picked "Delete Collection" as a good example for dogfooding the new client. (I chose this mostly because it's a simple API call that the Admin UI only makes in a single place). The latest commit swaps in some code to delete collections using our new client, and it looks very manageable.

But....

I'm struggling to actually test this because I can't figure out how to correctly import the library/module. 😬 I've added a script tag (<script type="module" src="libs/solr/index.js"></script>) to solr/webapp/web/index.html, which seems like it finds the generated JS code. But loading the main page of the Admin UI triggers a long run of import errors in the console:

Failed to load resource: the server responded with a status of 404 (Not Found)        ApiClient:1
Failed to load resource: the server responded with a status of 404 (Not Found).       BackupDeletionData:1
Failed to load resource: the server responded with a status of 404 (Not Found)        AddReplicaPropertyRequestBody:1
...

It looks like these errors correspond to imports near the top of libs/solr/index.js. One thing that jumps out at me in those errors is that it looks like we're importing the right filenames/paths, but with the file-extension missing entirely. Not sure what to make of that, but figured it might ring a bell for someone with more JS experience.

Possibly related, the README produced with the generated client includes some instructions on building and publishing the JS code as an NPM package. We don't run a Node server or use npm in the Admin UI as far as I can tell, so I was hoping that we could avoid any sort of "packaging", beyond copying the JS files into the webapp's lib/ directory as I've done. But maybe that's the root of the problems above in some way: that we're generating Node-flavored JS code and then trying to run it like vanilla JS code. I'm not even sure if that makes sense; clearly I'm very out of my depth here 😛

Would love some help from anyone more familiar with JS (and maybe Node) development. Maybe @janhoy or @MarcusSorealheis if either of you get a few minutes? We're so close to getting this in, I hate to see this blocked so near to the finish line.

@gerlowskija
Copy link
Contributor Author

Alright, great news: with a good deal of help from @HoustonPutman I've been able to resolve the packaging/import issues mentioned in my last comment.

Despite its name, it looks like the 'javascript' generator produces JS code that relies on a few server-side-NodeJS features (e.g. using require for module resolution/importing). It should really be named 'nodejs'. That said, we still managed to incorporate the library into the Admin UI by creating a browserify bundle and copying that into $webapp/libs/solr/index.js. So this PR now adds code to our gradle build for this purpose.

If we eventually publish this JS client we'll need to decide whether to publish in NodeJS/npm-ready or the browserify-bundled form. That'd be another great question for @MarcusSorealheis or anyone else more familiar with the JS ecosystem.


With these problems solved, the PR is ready to be merged!

@gerlowskija
Copy link
Contributor Author

I was initially planning on using the delete-collection button as the inaugural use of our v2 client, but ended up opting for the "Reload Collection" button instead.

Currently, the v2 client is exposed as the "CollectionsV2" service defined in "services.js". This PR should be a good example for anyone interested in toying with the admin UI and expanding the v2 footprint there. Most of our Solr API calls live in in JS "controller" files. Just add the "CollectionsV2" service as an arg for the controller in question (if not already present), and Angular will call the right factory functions under the hood.

@gerlowskija gerlowskija merged commit 3ebbb37 into apache:main Nov 13, 2023
1 of 2 checks passed
@gerlowskija gerlowskija deleted the SOLR-17062-generate-js-client branch November 13, 2023 19:14
@MarcusSorealheis
Copy link
Contributor

MarcusSorealheis commented Nov 16, 2023

I just got a look at the most recent question as I had some challenges to work through.

If we eventually publish this JS client we'll need to decide whether to publish in NodeJS/npm-ready or the browserify-bundled form. That'd be another great question for @MarcusSorealheis or anyone else more familiar with the JS ecosystem.

NodeJS/npm-ready is the way to go long term. Browserify is declining rapidly, though it was important for years after it was released. Webpack is a common replacement for Browserify today, and you can also do some of the bundling now without either. Webpack usually makes it easier.

@epugh
Copy link
Contributor

epugh commented Nov 16, 2023

A little late to the party, but I wanted to say how nice the V2 js code integrated! I expected something much more intrusive...

gerlowskija added a commit that referenced this pull request Dec 5, 2023
This commit adds build code to generate a JS client (using the
OpenAPI Generator's 'javascript' template) and adds the necessary
plumbing to bundle the client into our Admin UI.  See 'CollectionsV2' in
services.js as an example.

Note that nothing in this commit adds this JS client as a release
artifact, publishes it to npm, etc.

---------

Co-authored-by: Houston Putman <houston@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants