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

rommserver/alias: Do not call appserviceAPI in GetRoomIDForAlias if local alias found #631 #702

Merged
merged 9 commits into from Jun 25, 2019

Conversation

Projects
None yet
3 participants
@serra-allgood
Copy link
Contributor

commented May 21, 2019

Pull Request Checklist

  • I have made sure any new dependencies have been checked into the vendor/ directory
  • Pull request includes a sign off

Changes

A conditional is added to wrap the call to appserviceAPI if a local alias is not found in the database.

Fixes #631

serra-procore and others added some commits May 14, 2019

roomserver/alias: don't call appserviceAPI in GetRoomIDForAlias if lo…
…cal alias found

The existing implementation will always make a call to appserviceAPI,
regardless of whether a local alias is found in the database.

Fixes #631

Signed-off-by: Serra Allgood <serra@allgood.dev>
@anoadragon453
Copy link
Member

left a comment

Getting there! But some changes needed.

Show resolved Hide resolved roomserver/alias/alias.go Outdated
Show resolved Hide resolved roomserver/alias/alias.go
Show resolved Hide resolved roomserver/alias/alias_test.go Outdated
Show resolved Hide resolved roomserver/alias/alias_test.go Outdated

serra-allgood added some commits May 21, 2019

roomserver/alias: don't call appserviceAPI in GetRoomIDForAlias if lo…
…cal alias found

The existing implementation will always make a call to appserviceAPI,
regardless of whether a local alias is found in the database.

Fixes #631

Signed-off-by: Serra Allgood <serra@allgood.dev>
roomserver/alias: Add test cases, load from DB if appserviceAPI says …
…AliasExists

Signed-off-by: Serra Allgood <serra@allgood.dev>
roomserver/alias: Get tests passing
Signed-off-by: Serra Allgood <serra@allgood.dev>
Merge branch 'sca/631-alias-lookup' of github.com:serra-allgood/dendr…
…ite into sca/631-alias-lookup

Signed-off-by: Serra Allgood <serra@allgood.dev>
@serra-allgood

This comment has been minimized.

Copy link
Contributor Author

commented Jun 2, 2019

@anoadragon453 Changes made!

@anoadragon453
Copy link
Member

left a comment

Nearly there! I like the miniframework you built for the unit tests :)

Show resolved Hide resolved roomserver/alias/alias.go Outdated
Show resolved Hide resolved roomserver/alias/alias.go Outdated
Show resolved Hide resolved roomserver/alias/alias_test.go Outdated
roomserver/alias: Minor fixes, consolidate test cases
Signed-off-by: Serra Allgood <serra@allgood.dev>
@serra-allgood

This comment has been minimized.

Copy link
Contributor Author

commented Jun 3, 2019

Thanks for the compliment on the tests!

@anoadragon453
Copy link
Member

left a comment

This looks good! Going to wait til our CI is all fixed before merging however, but that should hopefully be very soon.

@anoadragon453

This comment has been minimized.

Copy link
Member

commented Jun 25, 2019

@serra-allgood Can you rebase this PR upon the latest master so CI changes don't appear in the diff?

Merge branch 'master' into sca/631-alias-lookup
* master:
  Fix testfile path hardcoded in show-expected-fail-tests.sh (#719)
  Make federation state request 404 when event not in the room - fixes #625 (#716)
  Not all systems have bash (#720)
  Update dependency gomatrixserverlib to 178ed5e (#695)
  Refine config and docs for sytest (#714)
  Fix pipeline, emoji and syntax (#713)
@anoadragon453

This comment has been minimized.

Copy link
Member

commented Jun 25, 2019

Nevermind, your PR branch was accessible to maintainers so I handled it :)

@anoadragon453 anoadragon453 merged commit a0dec45 into matrix-org:master Jun 25, 2019

8 checks passed

buildkite/dendrite Build #62 passed (6 minutes, 34 seconds)
Details
buildkite/dendrite/build-slash-go-1-dot-11 Passed (1 minute, 43 seconds)
Details
buildkite/dendrite/build-slash-go-1-dot-12 Passed (1 minute, 38 seconds)
Details
buildkite/dendrite/lint-slash-go-1-dot-12 Passed (1 minute, 58 seconds)
Details
buildkite/dendrite/pipeline Passed (4 seconds)
Details
buildkite/dendrite/unit-tests-slash-go-1-dot-11 Passed (1 minute, 42 seconds)
Details
buildkite/dendrite/unit-tests-slash-go-1-dot-12 Passed (1 minute, 40 seconds)
Details
ci/circleci: dendrite Your tests passed on CircleCI!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.