Go: Horizon: Add all endpoints to root '_links' response

[JakeUrban]

What problem does your feature solve?

The Ruby SDK uses Horizon's hal+json content via Hyperclient, an API client for auto-discovering endpoints. It reads horizon's root endpoint response and constructs methods for each endpoint listed under _links.

However, many endpoints are simply missing, making it difficult to access these endpoints via the Ruby SDK.

What would you like to see?

All endpoints listed in Horizon's API Reference included in the response to GET https://horizon.stellar.org under _links.

What alternatives are there?

The Ruby SDK doesn't have any alternative other than building explicitly defined requests for each endpoint, which would be a ton of work and not worth the effort compared to this fix.

[ire-and-curses]

Can you enumerate which endpoints have this problem?

[JakeUrban]

It would be easier to list what is already present. If you click this you'll see we have properties like account_transactions under _links. The Ruby SDK takes these and automatically builds methods:

client = Stellar::Client.default
client.horizon.account_transactions(account_id:'<account_id')

But for endpoints like account/{account_id}/offers, there is no such entry under _links and therefore the Ruby SDK's client doesn't have a natural way to hit it.

[tomerweller]

Endpoints linked in the root resource (WIP)

Top level endpoints

• [x] Account Details
• [x] Accounts
• [x] All Assets
• [x] All Effects
• [x] Fee Stats
• [x] All Ledgers
• [x] Ledger Details
• [ ] Metrics
• [x] Offer Details
• [x] Offers
• [x] All Operations
• [x] Operation Details
• [x] Orderbook Details
• [x] Strict Receive Payment Paths
• [x] Strict Send Payment Paths
• [ ] Find Payment Paths
• [x] All Payments
• [x] Trade Aggregations
• [x] Trades
• [x] All Transactions
• [x] Post Transaction
• [x] Transaction Details

Nested endpoints

• [ ] Payments for Account
• [ ] Payments for Ledger
• [ ] Payments for Transaction
• [ ] Trades for Account
• [ ] Trades for Offer
• [ ] Data for Account
• [ ] Effects for Account
• [ ] Effects for Ledger
• [ ] Effects for Operation
• [ ] Effects for Transaction
• [ ] Offers for Account
• [ ] Operations for Account
• [ ] Operations for Ledger
• [ ] Operations for Transaction
• [x] Transactions for Account
• [ ] Transactions for Ledger

[tomerweller]

I think that adding top level endpoints (that are not deprecated) to the root resource is a no-brainer for self discoverability/hyperclient.

The nested endpoints are not show stoppers as they're linked from the subresources (I haven't actually gone through all the subresources to verify this). Though it's a bit silly that you have to go through more http calls than required just to collect links.

[2opremio]

Thanks for the clarification @tomerweller . I will make sure that:

1. All the top-level links are present in the root resource
2. Nested endpoint links are reachable from subresources

[2opremio]

I am running into a couple of problems:

1. AFAIU, HAL doesn't distinguish between GET and POST requests (see this Stackoverflow question for more details), which makes it impossible to represent both GET /transactions and POST /transactions. @JakeUrban can you shed some light on how Hyperclient handles this? It may be enough to simply have separate entries for them, but I doubt it will be able to distinguish between GET and POST.

2. AFAICT, Horizon doesn't seem to do proper content negotiation for application/hal+json (e.g. when doing curl -H 'Accept: application/hal+json' horizonhost/transactions Horizon responds to the endpoint _normally_ instead of responding with the hal+json metadata). This would require fixing if we want to continue with the top-level+nested resources approach (which can take a while). Albeit, @tomerweller mentioned that _The nested endpoints are not show stoppers as they're linked from the subresources_, so maybe I am missing something? Alternatively we could simply put all the links at the top level.

[2opremio]

Regardless, I am going to create a PR with what I have now to move things forward.

[JakeUrban]

@2opremio POSTing to /transactions doesn't seem to be an issue for the ruby client:

@horizon.transactions._post(tx: envelope_base64)

GET requests would look slightly different:

@horizon.transactions().get_

I don't think you need to represent POST requests. HAL only specifies the request URL parameters, not POST body parameters.

[JakeUrban]

I think we can close this issue since the changes have been merged. When will this be deployed?

[2opremio]

@JakeUrban are you sure that the merged changes are enough? Note that most (if not all) the nested endpoints lack HAL info.

[2opremio]

I think we can close this issue since the changes have been merged. When will this be deployed?

With Horizon 1.1.0 (planned for April 6, see https://github.com/stellar/go/milestones )

[JakeUrban]

@2opremio Leaf endpoints (those that don't have subresources) don't need HAL since there are no more nested resources.

Do you mean the top-level resources don't have HAL describing their subresources? /accounts seems to have it.

[2opremio]

Ahhhh, I was (wrongly) assuming that you had to be able to query HAL metadata without making an actual query (hence my confusion about content negotiation). I will review the endpoints again and reopen if needed.

[JakeUrban]

@2opremio thanks for helping me get horizon set up locally. After taking a look and playing around with the client it looks good for the most part, but I found one issue:

The data in ledger should be moved to ledgers, and ledger should contain a href of http://localhost:8000/ledgers/{sequence}. This is what it is currently:

    "ledger": {
      "href": "http://localhost:8000/ledgers{?cursor,limit,order}",
      "templated": true
    },
    "ledgers": {
      "href": ""
    },

[2opremio]

Good catch. Unfortunately this won't be fixed for horizon 1.1.0 since it's already frozen (CC @ire-and-curses ).

[ire-and-curses]

@2opremio we're still in testing, if you want to push a bug fix for this I'm ok with it (it's low risk).

[2opremio]

@JakeUrban it should be fixed now. Thanks for noticing!