Page MenuHomePhabricator

Fill in Conduit support infrastructure for ApplicationEditor changes
Closed, ResolvedPublic

Description

ApplicationEditor introduced new *.edit API methods (e.g., paste.edit, maniphest.edit). Before we can really switch over to these, I need to do some infrastructure work:

Arcanist: arc commands like arc paste and arc todo need to switch to the new methods before we can throw out the old methods.

They also need to switch to new read endpoints after T7715. Since I'd prefer not to do two switches, I'm going to try to get the read endpoints in place now, then switch both reads and writes in one fell swoop. This should reduce the total engineering cost of these switches.

Special Cases: The new endpoints can nearly edit everything, but there are a few special sorts of values they can't yet edit, like Paths in Owners. I want to make these *.edit endpoints full-power.

I have some plans for doing this, but the, e.g., Owners case is sort of moot if you can't read the paths. So this also motivates getting the new read endpoints in order.

Phame: We're close to un-beta'ing Phame but I'd rather not unbeta it with a bunch of about-to-be-killed-off API methods, so this motivates getting the read endpoints sorted out, too.


Here's my plan to move forward:

  • Do the infrastructure work on T7715 to let us build new *.search endpoints as companions to the new *.edit endpoints.
  • Implement paste.search, owners.search, maniphest.search.
  • Increase the power of both search and edit endpoints to interact with special fields (like Paste content and Owners paths).
  • Swap Phame to EditEngine, implement phame.search and phame.edit (or phame.blog.edit + phame.post.edit, I suppose), throw away old Phame APIs.
  • Move arc todo, arc tasks and arc paste to new endpoints.

Revisions and Commits

rP Phabricator
D14798
D14796
D14791
D14790
D14788
D14777
D14776
D14775
D14774
D14773
D14772
D14769
D14768
D14767
D14766
D14765
D14764
D14763
D14761
D14760
D14758
D14743

Event Timeline

epriestley claimed this task.
epriestley raised the priority of this task from to Normal.
epriestley updated the task description. (Show Details)
epriestley changed the edit policy from "All Users" to "Community (Project)".
epriestley added subscribers: epriestley, chad, nickz.

You might already know, but I noticed that the owners search is broken if I use "path" instead of "Repositories" to query. Also it would be great if we can use the package name to search.

I can't reproduce that. Here's a search using "Path", which seems to work correctly:

Screen Shot 2015-12-11 at 3.23.39 PM.png (628×1 px, 76 KB)

Can you give me more details about how to reproduce the issue you're seeing?

I can't reproduce that. Here's a search using "Path", which seems to work correctly:

Screen Shot 2015-12-11 at 3.23.39 PM.png (628×1 px, 76 KB)

Can you give me more details about how to reproduce the issue you're seeing?

See this:

pasted_file (472×955 px, 64 KB)

I tried to search for /src/infrastructure/PhabricatorEditor.php, it was supposed to return empty results since no existing packages include this path, but it returned two packages. I am wondering if it thinks "/" should include everything no matter the specific path exists or not.

That result looks correct to me -- if a package includes "/", that means it includes everything under "/" too. So if you search for /x/y/z.cpp, any package that owns /, /x/, /x/y/ or /x/y/z.cpp will be returned, since any package owning a parent also owns the path itself.

You are right for the cases you described. But the case I mentinoed is that /src/infrastructure/PhabricatorEditor.php doesn't exist in both Arcanist and libutil.

We don't check that the path actually exists -- you might be searching for a deleted path that used to exist, or the repository might have a million branches and it only exists on one, and you might have a thousand different repositories too.

If you only want to search in specific repositories, combine "Repositories:" with "Paths:", like this:

Screen Shot 2015-12-13 at 4.20.45 PM.png (800×2 px, 110 KB)

This returns no results.

Ok, got it. Do you think it's reasonable to search using a package name?

Yes, but I have some hesitance around adding "search for a string in another string" fields without building better infrastructure first. There's some discussion in D14411#161139.

Basically, right now, we don't have a good, scalable, standard way to implement these searches. If we just issue WHERE name LIKE "%user-query%", it's sets us up for problems later:

  • even today, it will be slow in some applications (like Files) because they can not use keys and there are a lot of objects;
  • the semantics aren't ideal -- it would be better to be doing fulltext search than substring search;
  • when we do figure out the infrastructure later, that might change the semantics, which could be confusing.

No one has millions of packages and the semantics issue isn't as big there so I'm more comfortable just implementing a LIKE there for now than I am with files, but I'd really like to figure out what the long-term plan is first before implementing these fields. I'll file a followup task to look at that.

I think this stuff is basically in good shape, I'll follow up on the Phame stuff in T9897 and the arc stuff in T7715.