Use proper 2xx HTTP codes on connect

[jbpros]

We currently use a redirect (30x) HTTP status code on a connect request for a repo that was already connected. I believe this is an odd use of a redirect status. The 3xx (Redirection) class of status code indicates that further action needs to be taken by the user agent in order to fulfill the request. In the case of a connect request on git-en-boîte for an already connected repo, the client is not expected to take any further action to connect the repo.

I'm unclear as to why we need to tell the client about the internal differences: does it matter if the repo was already connected or not?

If it does, then I suggest we use 201 Created as the status for a connect that succeeded and actually connected a new repo, 200 OK for a successful connect for an existing repo.

If it does not matter, then we just return 200 OK on all successful connects (this has my preference for now, but I might be missing a good reason for the differentiation).

We'll need to adapt Git::GitEnBoite in CucumberStudio when we make this change.

Checklist:

• Decide if we want different statuses for new/existing repos.
• Get rid of the redirect and use 20x statuses.
• Adapt CucumberStudio's Git::GitEnBoite.connect
• POST to /repos/:repoId

[mattwynne]

Agreed. I think we should also return a location header with the location of the newly created resource.

Arguably, if we already know the repoId we should be POSTing directly to /repos/:repoId to create it.

[mattwynne]

IMO on the 201/200 I'm not sure I see a reason yet why a client would need it, but on the other hand it's good to be communicative if we have that information to hand.

[jbpros]

Location is to be used with 3xx and 201 and is highly focussed on automatic redirection. Link headers are a good fit for telling the client about related resources (like the related repo, or its branches or whatever relevant stuff). It is typically used in HATEOAS systems. Hypermedia has many great advantages, especially for public facing APIs like Git-en-Boîte (one of them is virtually no need for API versioning).

[jbpros]

I agree about the post request to a specific endpoint for the known repo ID.

[mattwynne]

💯 on the HATEOS thing. Let's do that before we add any more end-points.

[stale]

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed if no further activity occurs. Thank you for your contributions 🙏.

[stale]

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed if no further activity occurs. Thank you for your contributions 🙏.

[mattwynne]

bump

[mattwynne]

We're working on this code now as part of https://github.com/hiptest/cucumberstudio/issues/1434 I suggest we take the opportunity to clean it up a little.

I would starty by changing this line to send a PUT request to /repos/{repoId}:

git-en-boite/packages/acceptance-tests/src/step_definitions/steps.ts

Line 79 in ec0ffa9

await this.request.post('/repos').send(repoInfo).expect(202)

That can then drive a change to this unit test for the router/controller:

https://github.com/SmartBear/git-en-boite/blob/main/packages/web/src/routes/repos/update.spec.ts#L30

It should be changed to behave more like this endpoint, so that it creates the repo. It should also be changed to update the repo if it already exists, with the new repoUrl.

https://github.com/SmartBear/git-en-boite/blob/main/packages/web/src/routes/repos/create.spec.ts#L41

We can be confident about changing the /repos/{repoId} endpoint - the only production client we have is CucumberStudio at the moment, which uses the POST /repos endpoint so as long as we leave that alone we won't break anything.

[jbpros]

It should also be changed to update the repo if it already exists, with the new repoId.

Do you mean the new repoUrl?

[mattwynne]

Do you mean the new repoUrl?

Yes! I've updated the comment for clarity.

[mattwynne]

@jbpros I think this is done in 57aaa63. I kept the POST /repos endpoint but if you post here now, you just pass the remoteUrl and it will generate a repoId for you, then pass you back the URL in the Link header. I used the item rel for this.

It now returns a 201 created. The redirect stuff is gone.

WDYT?