-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Proposal: The Fyne URI Philosophy
Following the discussion on 2020-12-18 relating to #1509 and #1665, several decisions have been made relating to what methods should be part of the URI
interface, and which should be helper methods in the storage
package. This proposal summarizes the decisions made, the rationale, and invites further discussion by the Fyne community. Please add any discussion to the Discussion section below.
The URI
and ListableURI
interfaces should fully encapsulate all implementation functions required to interact with both the URI itself, as well as the resource that it references. Helper methods placed in the storage
package will be used only to handle meta-level considerations, such as passing data between multiple URI implementations. A future "registry" system will be used for the purpose of allowing specific URI implementations to be associated with URI schemes for the purpose of hydrating URIs from strings.
Note however that it is possible and allowed for a specific URI implementation to handle multiple schemes. For example, a URI of a given scheme might return a URI of a different scheme as it's parent. However, it is obviously necessary that the URI implementation must internally define or otherwise be aware of all other URI schemes it needs to interact with in such cases. It is expected that this will come up rarely.
This means that nearly all current URI-related functionality will be rolled into either the URI
or ListableURI
implementations. Methods including Parent
, Child
, ReaderFrom
, and WriterTo
will be folded into the interface implementations (as part of #1665). Methods that cannot be supported by the URI scheme will simply throw errors (and new error types will be created to allow this to be checked for in particular). We note that it may be possible to read or write data to a ListableURI
, even though this may seem unintuitive since it is normally impossible to obtain a reader or writer for a filesystem directory.
The Parent
of any URI must definitionally be listable, therefore Parent
should return a ListableURI
. Taken the other way around, it follows that only a ListableURI
can have a Child()
. Going forward, Driver.ListerForURI
will be removed, and listing a URI will cause the produced list of children to be of the ListableURI
type if they are listable (ListerForURI
will become a utility method that performs a type conversion, to avoid breaking the API).
We further agreed that we should lean towards having more API endpoints to expose more semantic information to the underlying URI optimization. For example a Rename
operation on a remote filesystem gives the implementation the opportunity to perform the rename on the remote end.
Changed items are marked with a ! symbol.
URI
Extension() string
Name() string
MimeType() string
Scheme() string
-
!
Parent() (ListableURI, error)
-
!
ReaderFrom() (URIReadCloser, error)
-
!
WriterTo() (URIWriteCloser, error)
-
!
Copy(URI) error
- This method should copy the referenced resource to some destination URI without modifying the original
-
!
Rename(URI) error
- This method should rename the reference resource to some destination URI, deleting or otherwise overwriting the referenced URI if it exists already
-
!
Delete(URI) error
- This method should delete, remove, or otherwise destroy the referenced resource
ListableURI
URI
-
!
List() ([]URI, error)
- The method signature remains unchanged, but this method will now ensure that returned children are of type
ListableURI
if appropriate, so they can be cast back toListableURI
using a normal type assertion.
- The method signature remains unchanged, but this method will now ensure that returned children are of type
-
!
Child(string) (URI, error)
- This method should instantiate a new URI nested below the referenced resource. For example, creating a child of a directory. Again, the returned child should be of type
ListableURI
if appropriate so it can be cast back toListableURI
using a normal type assertion.
- This method should instantiate a new URI nested below the referenced resource. For example, creating a child of a directory. Again, the returned child should be of type
Storage
-
!
Move(URI, URI) error
- This method reads the contents of it's first operand, writes the read data to the second operand, and then deletes the first operand. This is similar to
URI.Rename()
, but can work across URI schemes that do not know about each other.
- This method reads the contents of it's first operand, writes the read data to the second operand, and then deletes the first operand. This is similar to
-
!
Duplicate(URI, URI) error
- This method works identically to
Move()
, but does not delete it's first operand. It is the counterpart toURI.Copy()
.
- This method works identically to
-
!
RegisterURIImplementation(string, URIImplementation)
- This method registers a URI implementation associated with a string scheme.
-
!
NewURI(string) (URI, error)
- This method uses an internal registry to call the appropriate
NewURI()
method based on the scheme of the string URI.
- This method uses an internal registry to call the appropriate
! URIImplementation
-
!
NewURI(string) (URI, error)
- This method should create a new URI backed by the underlying URI implementation. If the URI is listable, it should be created as a
ListableURI
and returned as aURI
, so it can later be upcast if needed.
- This method should create a new URI backed by the underlying URI implementation. If the URI is listable, it should be created as a
New error types:
-
URIOperationNotSupported
- the requestedURI
orListableURI
API endpoint is not supported by the underlying implementation. For example, attempting to callReaderFrom()
on aListableURI
instance representing a filesystem directory would fail with this error. -
URIOperationImpossible
- the requested operation is supported by the underlying implementation, but violates some constraint internal to that implementation. For example, attempting to create an unrepresentable state on an LDAP server might fail with this error.
There was considerable dialog in the 2020-12-18 meeting about the tension between what should be accomplished via registration (having the developer register a callback function associated with a scheme), versus what can be accomplished by adding methods to interfaces. At various points, we tried to divide these ideas up along various lines, such as operations done on the URI, versus operations done on the resource that the URI references. The ultimate conclusion was that the vast majority of Fyne's users (developers) will be using URIs but not implementing them, and on that basis we should prioritize creating the easiest to use API, and that moving as much functionality as possible into the URI/ListableURI interfaces would make a more ergonomic API.
This decision was made with the knowledge in mind that:
- The basis on which functionality is moved into the URI/ListableURI interface is arbitrary and based on a subjective notion of what makes an API pleasant to use.
- This will make it more difficult for people to implement new URIs.
- 2020-12-18 - Charles - Initial Version
- 2020-12-18 - Stuart - Proposals and Typo Fixes
Andy: This seems to capture our discussion well. NewURI
probably does not need the ! next to it, as it's broadly the same definition as before (though it will look up a registry too).
Andy: I wonder if URI.ReaderFrom
and WriterTo
might be nicer as Reader
and Writer
names? Just a thought.
Stuart: Agreed
Andy: Also should this cover the deprecated items for removal, or is that covered elsewhere do you think?
Stuart: I don't see a huge upside to separating them
Stuart: Could Duplicate
be called Clone
?
Stuart: Could RegisterURIImplementation
/ URIImplementation
be shortened?
Stuart: URIOperationImpossible
sounds like it is outside the realm of possibility rather than something was attempted and did not succeed. How about URIOperationFailed
, or just return the underlying error?
Stuart: Will there still be a storage.NewFileURI
utility to avoid all the duplication of storage.NewURI("file://"+path)
in the common case of file handling?
Charles:
Stuart: Could
Duplicate
be calledClone
?
Fine with me.
Stuart: Could
RegisterURIImplementation
/URIImplementation
be shortened?
Does anyone have a suggestion here? Perhaps Handler instead of Implementation?
Stuart:
URIOperationImpossible
sounds like it is outside the realm of possibility rather than something was attempted and did not succeed. How aboutURIOperationFailed
, or just return the underlying error?
Where appropriate, the underlying error should be returned. The idea with URIOperationImpossible
was that some of the operations defined by the API may not make sense in the context of whatever resource the URI represents, and there isn't an underlying error that makes sense. It probably won't come up that much though, so we can drop it and re-add later if needed.
Stuart: Will there still be a
storage.NewFileURI
utility to avoid all the duplication ofstorage.NewURI("file://"+path)
in the common case of file handling?
I'm not opposed. Realistically dealing with local files is going to be the happy path and the common case.
Tilo:
Note: Unfortunately I missed the chat about this all. Therefore I might now have inappropriate objections that might vanish after I have a better understanding.
I think that the URI
interface offers too much functionality. IMO an URI and a resource are two different things and the URI should only be used to reference a resource. Honestly, I can’t even see the need of an interface for URI
at all. Why is one implementation not sufficient for the functionality that the current URI
interface provides?
Regarding access (reader, writer) to the resource: I think, this is something a repository should provide. It would take an URI as key and then provide access to the resource for this URI. It also would know which kind of URIs (schemes) it supports.
Regarding high-level convenience access (copy, move, delete): These should be limited to the storage package methods. I cannot see any advantage of src.Copy(dest)
over storage.Copy(src, dest)
. It just bloats the interface.
The registry should not be for URIs but for repositories, e.g. one could register an S3Repository
for the s3
scheme which provides access to S3 objects. The registration could either be done via RegisterRepostory(repo, scheme)
or RegisterRepository(repo)
where the repository provides the supported schemes. I think both would have their use-cases. While the latter is more convenient, the former allows better control, especially when there are more than one repository for a scheme around.
A repository would provide access to resources (reader, writer, rename/move (atomic), delete). I think the whole listing stuff (parent/children) would also belong to the repository.
Charles:
The points Tilo raised were indeed some of the things that came up in discussion. Unfortunately, I think my notes didn't capture things at a granular enough level...
I think that the
URI
interface offers too much functionality. IMO an URI and a resource are two different things and the URI should only be used to reference a resource. Honestly, I can’t even see the need of an interface forURI
at all. Why is one implementation not sufficient for the functionality that the currentURI
interface provides?
The duality between a resource versus a reference to a resource is something we groused about for some time. The proposed implementation is not very philosophically pure in this regard. I think the simplest way to put it is that the URI
interface is now really a way of mediating access to an implementation of all of the stuff you can do on a resource of a given type. It handles far more functionality than is required to simply manipulate a URI string.
On closer examination, we found that many seemingly simple concepts like Parent()
, Child()
, and so on are in fact not so simple, and may require functionality specific to a given URI scheme. By moving these types of methods into the URI interface, it makes it possible for the individual implementation to define the behavior they need.
Regarding access (reader, writer) to the resource: I think, this is something a repository should provide. It would take an URI as key and then provide access to the resource for this URI. It also would know which kind of URIs (schemes) it supports.
I think that is more or less equivalent, but with more steps/complexity. The notion with moving ReaderTo
and WriterFrom
into the URI
interface is that we no longer have to maintain such a repository, and also allows moving that functionality out of the driver (which a registry based approach would also solve). Ultimately, the purpose of a URI is going to be to either read or write to it (or perform some higher-order operation that boils down to reads and writes). If you will grant that the URI
interface handles functionality relevant to both the reference and to that-which-is-referenced, I think you will agree that mediating read and write access is appropriate to place here.
Regarding high-level convenience access (copy, move, delete): These should be limited to the storage package methods. I cannot see any advantage of
src.Copy(dest)
overstorage.Copy(src, dest)
. It just bloats the interface.
Irrespective of the previous points, I must disagree on this strongly. What constitutes a Copy, Move, Delete, etc. will vary by the underlying implementation. Performing a copy on an ssh://
by copying all of the data to the local end, then pushing it back up to the remote is clearly the wrong thing to do here. This was the rationale behind having Copy()
versus Duplicate()
, and Rename()
versus Move()
. The versions in the URI
interface give the implementation the opportunity to do something more optimal (such as trigger an operation on a remote server, or leveraging an OS level API).
This could also be solved by using a registry that allows URI implementors to register callbacks for these things a-la-cart. I think we (the folks on the call) all agreed that this would produce an un-ergonomic and complicated API.
For clarity to future readers: that I think what you (Tilo) is calling a repository, we are calling a registry or registration in my notes and earlier in this article.
Tilo:
I’m still not convinced that a rich URI is the way to go.
First regarding “repository”: I called it that way because this is what it is called in the Onion Architecture (which you probably already knew). In the end it does not matter if you call it repository, database, storage or whatever. I will stick to repository. And I don’t think that you meant this with “registry”. In my understanding a registry is basically some kind of mapping (like URI scheme to repository).
On closer examination, we found that many seemingly simple concepts like
Parent()
,Child()
, and so on are in fact not so simple, and may require functionality specific to a given URI scheme. By moving these types of methods into the URI interface, it makes it possible for the individual implementation to define the behavior they need.
Parent()
and Child()
(or Children()
) are not a property or concept of URI at all. They are a properties or relations that the resource might have. The resource is managed by the repository and therefore the hierarchical access to resources has to be implemented there.
I think that is more or less equivalent, but with more steps/complexity.
Yes, and no. Yes, it is equivalent. I cannot see where there are more steps involved. No, it is not more complex, only more obvious.
The notion with moving
ReaderTo
andWriterFrom
into theURI
interface is that we no longer have to maintain such a repository […].
That’s not really an advantage because you only move the implementation into the URI (where it does not belong).
Ultimately, the purpose of a URI is going to be to either read or write to it (or perform some higher-order operation that boils down to reads and writes).
I don’t think so. The ultimate purpose of a URI is already encoded in its name: “Resource Identifier”. It should identify a resource. The ultimate purpose of that resource is to be accessed in some way. But this access is not performed through the URI but through some repository.
If you will grant that the
URI
interface handles functionality relevant to both the reference and to that-which-is-referenced, I think you will agree that mediating read and write access is appropriate to place here.
I won’t grant that :). I think the URI should be restricted to what it is: an identifier. The resource access should be done by repositories.
Irrespective of the previous points, I must disagree on this strongly. What constitutes a Copy, Move, Delete, etc. will vary by the underlying implementation. Performing a copy on an
ssh://
by copying all of the data to the local end, then pushing it back up to the remote is clearly the wrong thing to do here. This was the rationale behind havingCopy()
versusDuplicate()
, andRename()
versusMove()
. The versions in theURI
interface give the implementation the opportunity to do something more optimal (such as trigger an operation on a remote server, or leveraging an OS level API).
I cannot see where a repository (let’s say an S3 repository or an SSH repository) cannot perform an optimized (server-side) copy when .Copy(dest, src)
is called. Why do you think that only an implementation in a specific URI could do that?
I still think it is better to implement a repository and register it for one or multiple URI schemes instead of bloating the interface of something that is meant to be an identifier.
Andy:
As far as I can see these are conceptually similar. One proposal is URI for identification and access, the other is that URI is identification and Repositiory handles access.
In this case the former would support shorthand like:
read := storage.NewURI(uriString).ReadFrom()
However if we expose the repository separately then it becomes
uri := storage.NewURI(uriString)
read := storage.RepositoryFor(uri.Scheme()).ReadFrom(uri)
I hope that captures the discussion. Both require a repository / registry, but one is providing syntactic sugar over the look-up. In the implementation it will not be so straight forward, but from a users point of view this is the difference.
My preference is to hide the details of the repository from day-to-day usage, though registering custom providers would still need to be a public API.
Tilo:
Good summary, Andy.
I don’t say there should not be convenience methods. I only say they should not be part of the URI interface:
uri := storage.NewURI(uriString)
read := storage.ReadFrom(uri)
Or even:
read := storage.ReadFromURIString(uriString)
Plus, I’m still not convinced that we need an URI interface at all. One could argue that special URI implementations (like MailURI
, S3URI
, HTTPSURI
, FileURI
etc.) allow special validation/parsing implementation. But also in this case, I think that this should not be part of the URI but of the repository that uses it as key. The repository knows exactly which URI values are valid. The URI implementation should not implement more than the URI RFC (3986 AFAIK).
Andy
We are talking 15 - 20 methods to handle all the functionality of a storage provider - that feels cluttered if they are all package level definitions.
We could move it to a Repository
type that the user can look up like storage.RepositoryFor(uri.Scheme())
but then you run the risk of the following, which would be really bad:
u := storage.NewFileURI("/my/path")
r := storage.RepositoryFor(u.Scheme())
u = storage.NewURI("https://google.com/favicon.ico")
r.ReadFrom(u) // this either does nothing or goes boom...
Andy
Another thought. We could keep the repository provider and URI completely seperate whilst delivering the same convenience.
type StorageRepository interface // would implement the required functionality
type URI interface // declares the helpers as with the convenience focused design
type uriImpl struct {// would implement the core URI logic, plus helpers like the following
res string
}
func (u URI) Read() (fyne.URIReadCloser, error) {
repo := repoForScheme(u.Scheme())
if repo == nil {
return nil, errUnsupportedScheme
}
return repo.Read(u)
}
This should provide a separation along with high levels of convenience. Developers looking to extend the functionality would do something like:
storage.AddStorageProvider(scheme string, repo StorageRepository)
Thu Dec 31 02:32:50 PM EST 2020 / Charles
Based on discussions on Slack and in this proposal, here is a new API, which I hope will please everyone (or at least be OK for everyone). Note that this is aspirational - if everyone agrees that this is what we should shoot for, we can then go back and consider how to go from the 1.4.X URI implementation to this.
type rfc3986 struct
- Our internal implementation of RFC3986.
- We can also consider either exporting this, or even placing it in an external project.
- In my view, we shouldn't be directly handling this type on API endpoints, but only using it internally.
-
func (rfc3986) Scheme() string
- returns the RFC3986 scheme for the URI -
func (rfc3986) Authority() string
- returns the RFC3986 authority for the URI -
func (rfc3986) Path() string
- returns the RFC3986 path for the URI -
func (rfc3986) Query() string
- returns the RFC3986 query for the URI -
func (rfc3986) Fragment() string
- returns the RFC3986 fragment for the URI - Components that are empty will be returned as empty-strings.
- We may wish to make somethings like
Authority()
also return rich types, for example the authority might need it's own type to allow easily parsing the user, port, domain, etc. However, it might be better to leave it to the storage repository to deal with/parse.
type URI struct
- Our implementation of URIs in a developer-facing way, internally backed by
rfc3986
, possibly exposing those functions.-
Repository
implementations will need access to these functions, so we must expose them one way or the other (either by wrapping them, or makingrfc3986
exported and exporting it's member in theURI
implementation. Or by some other means.
-
- Internally, these methods take the
Scheme()
of the underlying RFC3986-URI and look up an appropriate handler method in the relevant repository, or fail-over to a generic or primitive version if one is not available. - Unless otherwise noted, these methods all work as previously discussed.
- My rationale for including the URI-related operations in this type rather than in the storage package is because these are indeed all operations that act on a specific URI. I believe that by separating out RFC3986 as it's own thing, separation of concerns between "dumb" string URIs and operations done on a URI is achieved.
- By making this a struct rather than an interface, we can guarantee we are the only implementation, and can look up the underlying operations appropriately based on the repository system
- My rationale for removing
ListableURI
is that it seems like a weird distinction in retrospect. It may require operations in the repository to determine. It may change at runtime. Readable/writeable URIs are also not separated in this way. From a performance perspective, I think there is no reason to prefer a type assertion. func (URI) Extension() string
func (URI) Name() string
func (URI) MimeType() string
func (URI) Scheme() string
func (URI) Parent() (URI, error)
func (URI) ReaderFrom() (URIReadCloser, error)
func (URI) WriterTo (URIWriteCloser, error)
func (URI) Copy(URI) error
func (URI) Rename(URI) error
func (URI) Delete(URI) error
-
func (URI) Listable() bool, error
- Check if the URI can be listed, can fail if this involves an operation in the underlying repository.
func (URI) List() ([]URI, error)
func (URI) Child(string) (URI, error)
type Repository interface
- This interface allows for outside developers to implement new URI-backends.
- The methods in this interface return functions, rather than directly implementing the functions. This means that an implementation can choose not to implement a function.
- The second return, the Boolean, is
true
if thestorage
package should fail-over to using generic, internal versions of un-implemented functions. If thebool
options is omitted, it means that generic functions will be used unconditionally if none is defined (e.g. for mandatory functions). - Maybe we want to use names like
FooImpl
instead of justFoo
? - This approach makes it easy to define new URIs, especially if you are on the happy-path, you can rely on built-in/generic methods by simply returning
nil,true
for functions you don't want to implement. For example, if you haveReaderFrom()
andWriterTo()
, but notCopy()
,storage
can transparently fail over to doing this the "dumb" way. -
func NewURI(string) (URI, error)
- For obvious reasons, you must define a way to create a new URI.
func Extension() func (URI) () string
func Name() func (URI) () string
func MimeName() func (URI) () string
func Scheme() func (URI) () string
func Parent() (func (URI) () (URI, error), bool)
func ReaderFrom() (func (URI) () (URIReadCloser, error), bool)
func WriterTo() (func (URI) () (URIWriteCloser, error), bool)
func Copy() (func (URI) () error, bool)
func Rename() (func (URI) () error, bool))
func Delete() (func (URI) () error, bool))
func Listable() (func (URI) () (bool, error), bool)
func List() (func (URI) () ([]URI, error), bool)
func Child() (func (URI) (string) (URI, error), bool)
package storage
Move(URI, URI) error
Duplicate(URI, URI) error
RegisterRepository(string, Repository)
NewURI(string) (URI, error)
New error types:
-
URIOperationNotSupported
- the requestedURI
API endpoint is not supported by the underlying implementation. For example, attempting to callReaderFrom()
on aURI
instance representing a filesystem directory would fail with this error. -
URIOperationImpossible
- the requested operation is supported by the underlying implementation, but violates some constraint internal to that implementation. For example, attempting to create an unrepresentable state on an LDAP server might fail with this error. -
URINotListable
- ifList()
is called on a URi which is not listable.
Thu Dec 31 04:21:19 PM EST 2020 / Charles
Some additional comments pulled over from slack:
-
Name()
,Scheme()
, andExtension()
do not belong to theRepository
interface since they are simply URI string parsing ; they should not be implemented there (leaving the above unmodified for posterity). -
ListableURI
is really just an implicit form of caching whether or not something is listable. A better argument than used above for movingList()
to URI and addingIsListable()
is that it allows caching of this information to be handled explicitly. - On that note, some performance optimizations (e.g. avoiding calling
stat()
on files on a remote machine to checkIsListable()
by caching this information) require caching. Afunc (URI) Refresh()
should be added to theURI
struct (and a corresponding function call in theRepository
interface) as a way of explicitly telling the repository it needs to flush any caches relating to the URI. - We may want or need to add some kind of configuration method (e.g. a KVP store) to allow tuning repositories (e.g. configuring caching behavior, or passing flags to some underlying program or driver). This change can be made additively later, so is out-of-scope for this proposal.
Fri Jan 1 10:47:17 AM UTC 2021 / Tilo
The new proposal is far better but I still have some objections of different severity and also questions and proposals.
-
Questions
- I had parsing errors when trying to understand some of the function definitions. For instance in
func ReaderFrom() (func (URI) () (URIReadCloser, error), bool)
: what does the(URI)
part mean? - What is the purpose of
Repository.MimeName()
?
- I had parsing errors when trying to understand some of the function definitions. For instance in
-
Weak objections (I’m against but won’t insist)
-
Moving the
URI
into a type instead of an interface reduces the severity of my objections against its convenience methods. I still think it is wrong to have them there but at least we do not enforce others to implement them. The list of methods I don’t would introduce inURI
is:Child
Copy
Delete
List
Parent
ReaderFrom
Rename
WriterTo
-
I cannot see where the separation of
URI
andrfc3986
is an improvement. It does not affect my opinion about the URI being a dump identifier.
-
-
Strong objections (I’m against and won’t back this)
- While I won’t insist on removing the convenience methods from the
URI
, I still want to have them in thestorage
package:func Child(URI, string) (URI, error)
func Copy(URI, URI) error
func Delete(URI) error
func List(URI) ([]URI, error)
func Listable(URI) (bool, error)
func Parent(URI) (URI, error)
func ReaderFrom(URI) (URIReadCloser, error)
func Rename(URI, URI) error
func WriterTo(URI) (URIWriteCloser, error)
- While I won’t insist on removing the convenience methods from the
-
Proposals
- Rename
NewURI
toParseURI
. The latter make more explicit that there is parsing involved. Also, I don’t think that a constructor (method starting withNew
) should return an error.
- Rename
P.S.: Okay, turned out it was only one proposal and one strong objection but I won’t reformat this now :).
Fri Jan 1 02:28:13 PM EST 2021 / Charles
In response to Tilo's above points:
- 1.1 -
func ReaderFrom() (func (URI) () (URIReadCloser, error), bool)
means a function that returns a pointer-receiver to a URI (should beURI*
notURI
in retrospect), which takes no args, and returns aURIReadCloser, error
, followed by a boolean flag. - 1.2 - typo, should be
MimeType()
, I left this in since it was already there. - 2.2 - Maybe we should use a name other than
URI
. The notion was that it's separating the concerns about "what the RFC says", versus "what we have built on top of the RFC". I will advocate strongly for having this distinction internally (unexported), and weakly for exporting it. - 3.1 - I am amenable to this. I don't think this adds much complexity either way.
- 4.1 - I like this idea, agree.
Here is something I would propose, but I would like Andy and Damon to weigh in also:
- Keep methods already on URI/ListableURI for backwards-compat, but mark as deprecated.
- Expose the methods I have discussed related to the RFC (e.g.
Authority()
) on URI (or export the RFC3986 member). - Introduce the methods Tilo has discussed in the
storage
package only for now (not in theURI
type). - As we move along, we can add those methods to the URI or some other helper type in an additive way if we feel it is necessary. This could be done without breaking backwards compat if the endpoints in the
storage
package are maintained.
Sat Jan 2 18:12 UTC 2021 / Stuart
Thinking more about the longer term functionality, it becomes clearer to me that an abstraction above individual URIs such as Repository is a good idea. Consider these additions to File Dialog;
- A Repository could provide a set of URIs to display in the Favourites section
- A Repository could provide optimized Searching, Filtering, and Sorting
- A Repository may need to handle identity/account management for remote file systems or cloud storage providers
Andy 4 Jan:
Charles:
A func (URI) Refresh() should be added to the URI struct (and a corresponding function call in the Repository interface) as a way of explicitly telling the repository it needs to flush any caches relating to the URI.
This makes the assumption that the consumer of a URI or repository knows more about the external state and therefore need to refresh than the repository does. Unless you can show a real-world use-case where this is true I think that cache management should be handled internally.
As we move along, we can add those methods to the URI...
Unfortunately not - the URI is a public interface, we cannot just add to it outside of a breaking release. That is basically the whole reason to settle this design ASAP, so that we can settle on a definition of URI that will need to stay for the forseeable future.
Tilo:
While I won’t insist on removing the convenience methods from the
URI
, I still want to have them in thestorage
package
Rather than expressing an opinion that you say is blocking can you please provide the reasoning? We have discussed this before and it was regarding separation of concerns. What Charles has recently provided keeps these concerns separate - therefore I would like to understand why you object to this new design. I am, in general, against the idea of providing 2 ways to do all these operations - preferring instead the single, correct, way - that makes documentation, and support, a lot easier.
The list of methods I don’t would introduce in
URI
is:
In the list is Child
and Parent
. Unfortunately, as we have identified through some discussions, is that the parent/ child relationship of a URI is not pre-determined by string parsing. I don't see how we can choose to make these optional whilst still having a self-contained URI definition. For LDAP or other non-path-based identifiers the parent or child cannot be inferred. This surely means that the child/parent relationship needs to be returned from the URI - otherwise we start coding specific URI knowledge in other areas, which in my mind would break separation of concerns again.
Mon Jan 4 11:19:52 AM EST 2021 / Charles
This makes the assumption that the consumer of a URI or repository knows more about the external state and therefore need to refresh than the repository does. Unless you can show a real-world use-case where this is true I think that cache management should be handled internally.
OK, granted the repository could add it's own goroutine, mutexes, etc. to deal with this. If the need comes up, we can revisit and possibly add the capability to do this.
Unfortunately not - the URI is a public interface, we cannot just add to it outside of a breaking release. That is basically the whole reason to settle this design ASAP, so that we can settle on a definition of URI that will need to stay for the forseeable future.
Are you opposed to making URI a struct then? I don't see any reason it should continue to be an interface, since the functionality that would accomplish would be handled by the repository system.
Andy 4 Jan:
Are you opposed to making URI a struct then?
Whatever changes seem appropriate, and can be made in a backwards compatible or deprecation-path manner should be explored. I don't know if struct is better than interface without exploring the implementation I guess.
What I will say, however, is that using an interface at the top level allows us to pass around values of that type and yet have the implementation within the storage package. If it becomes a struct then it would follow that this implementation moves out of storage. At this point you can't simply call into storage functions from helper methods on the struct as you would create a circular dependency. The root package should not depend on any child packages - as it contains the definition of all the common types.
Tue Jan 5 01:07:46 PM EST 2021 / Charles
I think this has convinced me that we should follow Tilo's suggestion, and implement these methods in the top-level storage
package, rather than as members of the URI interface/struct. I think we want URI to stay an interface for backwards compatibility. In principle, nobody should be implementing their own interface, but it is part of the public API. The philosophical purity of Tilo's argument did not win me over, but the ability to be able to iterate (make additive changes to the API later) is something I think we really want. If it's going to stay as an interface, using package methods also means we have control over the methods (nobody can override URI.Copy if it's storage.Copy instead).
Aside: I have started work on implementing the Repository interface locally, nothing pushed yet. It's taking a while, since I'm also using it as a place to write documentation about what everything is supposed to do.
Tue Jan 5 04:13:09 PM EST 2021 / Charles
I have written a first pass at the Repository
interface discussed here.