Merge lp:~cmars/juju-core/no-default-series into lp:~go-bot/juju-core/trunk
- no-default-series
- Merge into trunk
| Status: | Merged |
|---|---|
| Merged at revision: | 2470 |
| Proposed branch: | lp:~cmars/juju-core/no-default-series |
| Merge into: | lp:~go-bot/juju-core/trunk |
| Diff against target: |
973 lines (+343/-137) 15 files modified
charm/charm.go (+1/-1) charm/repo.go (+46/-9) charm/repo_test.go (+2/-2) charm/testing/mockstore.go (+15/-2) charm/url.go (+100/-47) charm/url_test.go (+88/-34) cmd/juju/bootstrap_test.go (+2/-2) environs/jujutest/livetests.go (+1/-1) juju/conn_test.go (+7/-5) state/apiserver/charms.go (+6/-4) state/apiserver/client/client_test.go (+14/-9) store/lpad.go (+4/-1) store/server.go (+21/-3) store/server_test.go (+14/-10) testing/charm.go (+22/-7) |
| To merge this branch: | bzr merge lp:~cmars/juju-core/no-default-series |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Juju Engineering | Pending | ||
|
Review via email:
|
|||
Commit message
Description of the change
Resolve implicit series with charm store.
Charm URLs without a series are "unresolved". A charm repository can resolve
the series. Charm store service resolves the series by adding CanonicalURL to
the charm info if it is found & resolved. Local repository uses default
series as configured in the environment. Command-line tools interacting with
a charm repo are responsible for making requests with resolved charm URLs.
State server requires a resolved charm URL for such requests. Charm store
service initially resolves series to a hard-coded value -- to be replaced by
more advanced logic as we transition to new LTS. Updated unit tests & mocks
accordingly.
| Casey Marshall (cmars) wrote : | # |
| Casey Marshall (cmars) wrote : | # |
Please take a look.
| Casey Marshall (cmars) wrote : | # |
Please take a look.
| Casey Marshall (cmars) wrote : | # |
Please take a look.
| Casey Marshall (cmars) wrote : | # |
Tim,
Please review my proposal for eliminating default series from the juju
client (or pass it along to someone who's up for it). In a nutshell:
- Allow charm.URL to have an implied "empty" series.
- charm.Repository can Resolve them.
- The charm store "resolves" series by using a hard-coded value (for
now).
- A local repository uses the environment config default series.
- It's a client command's responsibility to make sure a charm URL is
resolved before calling the state server with it.
Thanks,
Casey
| Dave Cheney (dave-cheney) wrote : | # |
On 2014/03/17 22:55:29, cmars wrote:
> Tim,
> Please review my proposal for eliminating default series from the juju
client
> (or pass it along to someone who's up for it). In a nutshell:
> - Allow charm.URL to have an implied "empty" series.
> - charm.Repository can Resolve them.
> - The charm store "resolves" series by using a hard-coded value (for
now).
> - A local repository uses the environment config default series.
> - It's a client command's responsibility to make sure a charm URL is
resolved
> before calling the state server with it.
> Thanks,
> Casey
I think this needs to be broken out into two changes. The store changes
need to land well before the release that contains the client changes.
| Dave Cheney (dave-cheney) wrote : | # |
Some minor comments.
A larger meta comment, a lot of this change is noise due to charm.URL
being treated as a pointer where it is actually a value.
If all the references to *charm.URL were replaced with charm.URL the
amount of defensive coding in this change, and elsewhere would be
reduced.
https:/
File charm/repo.go (right):
https:/
charm/repo.go:154: curl = &*curl
I don't understand what this line does. I think it does nothing. If if
does something then it need a comment to explain the magic.
https:/
charm/repo.go:429: result := &*curl
yuk, how about
result := *curl
if !result ... {
}
return &result, nil
https:/
File store/server.go (right):
https:/
store/server.go:80: curl.Series = "precise"
please make this a constant at the top of the file with a comment
explaining the effect of changing the constant.
| Casey Marshall (cmars) wrote : | # |
Please take a look.
https:/
File charm/repo.go (right):
https:/
charm/repo.go:154: curl = &*curl
On 2014/03/18 01:01:10, dfc wrote:
> I don't understand what this line does. I think it does nothing. If if
does
> something then it need a comment to explain the magic.
It doesn't do what I think it did. :\
https:/
charm/repo.go:429: result := &*curl
On 2014/03/18 01:01:10, dfc wrote:
> yuk, how about
> result := *curl
> if !result ... {
> }
> return &result, nil
Done.
https:/
File store/server.go (right):
https:/
store/server.go:80: curl.Series = "precise"
On 2014/03/18 01:01:10, dfc wrote:
> please make this a constant at the top of the file with a comment
explaining the
> effect of changing the constant.
Done.
| Casey Marshall (cmars) wrote : | # |
On 2014/03/18 01:01:10, dfc wrote:
> Some minor comments.
> A larger meta comment, a lot of this change is noise due to charm.URL
being
> treated as a pointer where it is actually a value.
> If all the references to *charm.URL were replaced with charm.URL the
amount of
> defensive coding in this change, and elsewhere would be reduced.
I agree that referencing charm.URL as a value type would be ideal. This
morning I explored what it would take. I really don't feel comfortable
landing such a refactoring in this specific proposal -- much more of the
codebase would be affected.
| Casey Marshall (cmars) wrote : | # |
dfc wrote:
> I think this needs to be broken out into two changes. The store
changes need to
> land well before the release that contains the client changes.
I've removed all the client-side changes from this proposal. PTAL.
Thanks,
Casey
| William Reade (fwereade) wrote : | # |
A few comments, ping me when you're online.
https:/
File charm/repo.go (right):
https:/
charm/repo.go:27: CanonicalURL string `json:"
can we make it canonical-url, please, for general consistency across the
codebase
https:/
File state/apiserver
https:/
state/apiserver
resolved")
I'm a little concerned that the url definition change requires that we
be defensive like this at all possible entry points into the system.
(and, to guard against misses in any one layer, to be defensive at all
subsequent layers)
OTOH, I imagine this is forward-looking in the sense that we *will* end
up with, eg, cs:nova-compute actually referencing a cross-series charm.
If that's the case we'll just hve to grow handling for unresolved urls
through the codebase as we need it, and can then drop the defensiveness.
Is that where we're going with this? If so, +1... but if you can think
of a way to help everybody know they shouldn't be letting unresolved
urls through, I'd love to hear it. As it is it's just a subtle required
feature of the API that anybody could easily miss.
Ha. We need a "how to write API stuff" document *anyway* I guess.
https:/
File store/server.go (right):
https:/
store/server.go:19: const DefaultSeries = "precise"
So, we'll need some care for the switchover here. Are you up to date
with what needs to be done when, and where, to change this over to
trusty and get it deployed? I'm not 100% sure myself, but we need to
figure this out pretty much now...
https:/
File store/server_
https:/
store/server_
not found"},
hmm, the code in the previous file seemed to imply that we do expect to
turn cs:wordpress into cs:precise/
missing somewhere...
https:/
File testing/charm.go (right):
https:/
testing/
should we have a fallback here for stores without DefaultSeries set?
| Casey Marshall (cmars) wrote : | # |
Please take a look.
| Casey Marshall (cmars) wrote : | # |
https:/
File charm/repo.go (right):
https:/
charm/repo.go:27: CanonicalURL string `json:"
On 2014/03/20 12:13:42, fwereade wrote:
> can we make it canonical-url, please, for general consistency across
the
> codebase
Done.
https:/
File state/apiserver
https:/
state/apiserver
resolved")
On 2014/03/20 12:13:42, fwereade wrote:
> I'm a little concerned that the url definition change requires that we
be
> defensive like this at all possible entry points into the system.
(and, to guard
> against misses in any one layer, to be defensive at all subsequent
layers)
> OTOH, I imagine this is forward-looking in the sense that we *will*
end up with,
> eg, cs:nova-compute actually referencing a cross-series charm. If
that's the
> case we'll just hve to grow handling for unresolved urls through the
codebase as
> we need it, and can then drop the defensiveness. Is that where we're
going with
> this? If so, +1... but if you can think of a way to help everybody
know they
> shouldn't be letting unresolved urls through, I'd love to hear it. As
it is it's
> just a subtle required feature of the API that anybody could easily
miss.
> Ha. We need a "how to write API stuff" document *anyway* I guess.
There should not be a cross-series charm. Charms will declare the series
they support in the charm metadata. To support multiple series, a charm
will have to have per-series branches and content; they'd effectively be
distinct charms from a deployment perspective.
When a client deploys or upgrades a charm, it is responsible for
resolving the URL with the charm repository before making the API call.
As far as I know, these are the only entry points for new charm URLs
from the user into the API server. If this is naive, let's iterate on
improving the defensiveness & I'm open to suggestion.
I'd like the state server to never have to resolve a charm URL series,
and reject all unresolved URLs.
https:/
File store/server.go (right):
https:/
store/server.go:19: const DefaultSeries = "precise"
On 2014/03/20 12:13:42, fwereade wrote:
> So, we'll need some care for the switchover here. Are you up to date
with what
> needs to be done when, and where, to change this over to trusty and
get it
> deployed? I'm not 100% sure myself, but we need to figure this out
pretty much
> now...
The first step is to give the server the capability to resolve a series
for the client, land and deploy this first.
We should probably discuss how much we can get in this first landing. A
hard-coded default is probably the most pessimistic thing that could
possibly work while allowing clients to be forward compatible.
https:/
| Casey Marshall (cmars) wrote : | # |
Please take a look.
| Casey Marshall (cmars) wrote : | # |
On 2014/03/20 20:01:10, cmars wrote:
> Please take a look.
William,
This latest change has the charm.URL improvements we discussed earlier,
PTAL.
I'll hold off on the charm metadata series and associated "charm store
series solver" for a follow-up branch which I'll work on next.
Thanks,
Casey
| John A Meinel (jameinel) wrote : | # |
On Thu, Mar 20, 2014 at 6:51 PM, Casey Marshall
<email address hidden> wrote:
...
>
> When a client deploys or upgrades a charm, it is responsible for
> resolving the URL with the charm repository before making the API call.
> As far as I know, these are the only entry points for new charm URLs
> from the user into the API server. If this is naive, let's iterate on
> improving the defensiveness & I'm open to suggestion.
>
> I'd like the state server to never have to resolve a charm URL series,
> and reject all unresolved URLs.
I'd like to understand this a bit better. Given the API server already
supports "deploy cs:precise/foo" which it will then go to the charm
store and download, it seems perfectly reasonable that the client
could connect to *1* remote machine (the API Server) rather than also
needing to know who the Charm Store is and how it is configured, and
how it can resolve information there.
Why do you feel it is better to push this work into needing Clients to
be smart? (One argument, I guess, is that Juju GUI is already listing
the charm store in order to present anything to clients, so the only
other client today is juju-core.)
But given that the API Server already needs to know how to talk to the
charm store (to install cs:precise/foo) it doesn't seem that bad to
have it also resolve URLs.
John
=:->
| Casey Marshall (cmars) wrote : | # |
On 03/23/2014 04:30 AM, John A Meinel wrote:
> On Thu, Mar 20, 2014 at 6:51 PM, Casey Marshall
> <email address hidden> wrote:
> ...
>>
>> When a client deploys or upgrades a charm, it is responsible for
>> resolving the URL with the charm repository before making the API call.
>> As far as I know, these are the only entry points for new charm URLs
>> from the user into the API server. If this is naive, let's iterate on
>> improving the defensiveness & I'm open to suggestion.
>>
>> I'd like the state server to never have to resolve a charm URL series,
>> and reject all unresolved URLs.
>
> I'd like to understand this a bit better. Given the API server already
> supports "deploy cs:precise/foo" which it will then go to the charm
> store and download, it seems perfectly reasonable that the client
> could connect to *1* remote machine (the API Server) rather than also
> needing to know who the Charm Store is and how it is configured, and
> how it can resolve information there.
> Why do you feel it is better to push this work into needing Clients to
> be smart? (One argument, I guess, is that Juju GUI is already listing
> the charm store in order to present anything to clients, so the only
> other client today is juju-core.)
> But given that the API Server already needs to know how to talk to the
> charm store (to install cs:precise/foo) it doesn't seem that bad to
> have it also resolve URLs.
>
My primary goal there is to keep the state server simple, backwards-
(and forwards-) compatible. I decided to keep the current requirements
of a fully-resolved charm URL in deploy, publish, and
upgradecharm-
this is actually enforced by the type-system -- there is a distinction
between charm.URL (a fully-resolved charm location with series) and
charm.Reference (a charm location without a series resolved).
You raise a good point that the (>= 1.18) juju client will not contact
the charm store directly. What would you think of adding an API method,
call it "ResolveCharm", which exposes the charm store series-resolving
functionality back to the state client? The state client would still be
responsible for resolving the charm, but it will not need to contact the
charm store directly to resolve it.
I don't think "ResolveCharm" would necessarily need to land in this
merge proposal. It could go into the follow-up proposal with the
client-side changes.
> John
> =:->
>
-Casey
| John A Meinel (jameinel) wrote : | # |
It does mean 2 round trips for something that we are resolving in the
state server anyway, but it isn't a huge problem. Better than having
the client have to go to the Charm store directly. As you say, it
doesn't have to happen in this proposal, and this is probably still a
reasonable way forward.
On Mon, Mar 24, 2014 at 6:24 AM, Casey Marshall
<email address hidden> wrote:
> On 03/23/2014 04:30 AM, John A Meinel wrote:
>> On Thu, Mar 20, 2014 at 6:51 PM, Casey Marshall
>> <email address hidden> wrote:
>> ...
>>>
>>> When a client deploys or upgrades a charm, it is responsible for
>>> resolving the URL with the charm repository before making the API call.
>>> As far as I know, these are the only entry points for new charm URLs
>>> from the user into the API server. If this is naive, let's iterate on
>>> improving the defensiveness & I'm open to suggestion.
>>>
>>> I'd like the state server to never have to resolve a charm URL series,
>>> and reject all unresolved URLs.
>>
>> I'd like to understand this a bit better. Given the API server already
>> supports "deploy cs:precise/foo" which it will then go to the charm
>> store and download, it seems perfectly reasonable that the client
>> could connect to *1* remote machine (the API Server) rather than also
>> needing to know who the Charm Store is and how it is configured, and
>> how it can resolve information there.
>> Why do you feel it is better to push this work into needing Clients to
>> be smart? (One argument, I guess, is that Juju GUI is already listing
>> the charm store in order to present anything to clients, so the only
>> other client today is juju-core.)
>> But given that the API Server already needs to know how to talk to the
>> charm store (to install cs:precise/foo) it doesn't seem that bad to
>> have it also resolve URLs.
>>
>
> My primary goal there is to keep the state server simple, backwards-
> (and forwards-) compatible. I decided to keep the current requirements
> of a fully-resolved charm URL in deploy, publish, and
> upgradecharm-
> this is actually enforced by the type-system -- there is a distinction
> between charm.URL (a fully-resolved charm location with series) and
> charm.Reference (a charm location without a series resolved).
>
> You raise a good point that the (>= 1.18) juju client will not contact
> the charm store directly. What would you think of adding an API method,
> call it "ResolveCharm", which exposes the charm store series-resolving
> functionality back to the state client? The state client would still be
> responsible for resolving the charm, but it will not need to contact the
> charm store directly to resolve it.
>
> I don't think "ResolveCharm" would necessarily need to land in this
> merge proposal. It could go into the follow-up proposal with the
> client-side changes.
>
>> John
>> =:->
>>
>
> -Casey
>
>
> --
> https:/
> You are subscribed to branch lp:juju-core.
| William Reade (fwereade) wrote : | # |
LGTM, I think these are all trivials
https:/
File charm/url_test.go (right):
https:/
charm/url_
charm.ErrUnreso
We'd usually check for direct equality with ErrUnresolvedUrl, and
otherwise use gc.ErrorMatches to check the string.
https:/
charm/url_
Explicit test that precise-1 is not allowed, but precise1 is?
https:/
File state/apiserver
https:/
state/apiserver
errors consistent one day.
If these errors are starting to look nicer, please drop the comment (and
the one below). If they're not, would you upgrade this to a tech-debt
bug and fill in the usual TODO template (name, date, bug#, \n, short
description)?
https:/
File store/server.go (right):
https:/
store/server.go:80: return &charm.
DefaultSeries}
I'm not seeing where this is called. Dead code?
| Casey Marshall (cmars) wrote : | # |
I've restricted the series name to not include dashes, and resubmitted
the MP in launchpad with the prior branch prereq:
https:/
PTAL, thanks!
I'm a bit unsure whether/how lbox & rietveld will follow the LP
resubmit..
-Casey
https:/
File charm/url_test.go (right):
https:/
charm/url_
charm.ErrUnreso
On 2014/03/24 09:24:09, fwereade wrote:
> We'd usually check for direct equality with ErrUnresolvedUrl, and
otherwise use
> gc.ErrorMatches to check the string.
Done.
https:/
charm/url_
On 2014/03/24 09:24:09, fwereade wrote:
> Explicit test that precise-1 is not allowed, but precise1 is?
Done.
https:/
File state/apiserver
https:/
state/apiserver
errors consistent one day.
On 2014/03/24 09:24:09, fwereade wrote:
> If these errors are starting to look nicer, please drop the comment
(and the one
> below). If they're not, would you upgrade this to a tech-debt bug and
fill in
> the usual TODO template (name, date, bug#, \n, short description)?
Done.
https:/
File store/server.go (right):
https:/
store/server.go:80: return &charm.
DefaultSeries}
On 2014/03/24 09:24:09, fwereade wrote:
> I'm not seeing where this is called. Dead code?
Yes, it is now. I've removed it.
| John A Meinel (jameinel) wrote : | # |
You have to "lbox propose" again, and then it will generate a new diff on
Reitveld, it doesn't notice plain 'bzr push' to Launchpad.
On Mon, Mar 24, 2014 at 7:57 PM, Casey Marshall <
<email address hidden>> wrote:
> I've restricted the series name to not include dashes, and resubmitted
> the MP in launchpad with the prior branch prereq:
> https:/
>
> PTAL, thanks!
>
> I'm a bit unsure whether/how lbox & rietveld will follow the LP
> resubmit..
>
> -Casey
>
>
> https:/
> File charm/url_test.go (right):
>
>
> https:/
> charm/url_
> charm.ErrUnreso
> On 2014/03/24 09:24:09, fwereade wrote:
> > We'd usually check for direct equality with ErrUnresolvedUrl, and
> otherwise use
> > gc.ErrorMatches to check the string.
>
> Done.
>
>
> https:/
> charm/url_
> On 2014/03/24 09:24:09, fwereade wrote:
> > Explicit test that precise-1 is not allowed, but precise1 is?
>
> Done.
>
>
> https:/
> File state/apiserver
>
>
> https:/
> state/apiserver
> errors consistent one day.
> On 2014/03/24 09:24:09, fwereade wrote:
> > If these errors are starting to look nicer, please drop the comment
> (and the one
> > below). If they're not, would you upgrade this to a tech-debt bug and
> fill in
> > the usual TODO template (name, date, bug#, \n, short description)?
>
> Done.
>
> https:/
> File store/server.go (right):
>
>
> https:/
> store/server.go:80: return &charm.
> DefaultSeries}
> On 2014/03/24 09:24:09, fwereade wrote:
> > I'm not seeing where this is called. Dead code?
>
> Yes, it is now. I've removed it.
>
> https:/
>
> --
> https:/
> You are subscribed to branch lp:juju-core.
>
Preview Diff
| 1 | === modified file 'charm/charm.go' | |||
| 2 | --- charm/charm.go 2013-11-07 08:51:14 +0000 | |||
| 3 | +++ charm/charm.go 2014-03-20 19:57:15 +0000 | |||
| 4 | @@ -40,7 +40,7 @@ | |||
| 5 | 40 | if localRepoPath == "" { | 40 | if localRepoPath == "" { |
| 6 | 41 | return nil, errors.New("path to local repository not specified") | 41 | return nil, errors.New("path to local repository not specified") |
| 7 | 42 | } | 42 | } |
| 9 | 43 | repo = &LocalRepository{localRepoPath} | 43 | repo = &LocalRepository{Path: localRepoPath} |
| 10 | 44 | default: | 44 | default: |
| 11 | 45 | return nil, fmt.Errorf("unknown schema for charm URL %q", curl) | 45 | return nil, fmt.Errorf("unknown schema for charm URL %q", curl) |
| 12 | 46 | } | 46 | } |
| 13 | 47 | 47 | ||
| 14 | === modified file 'charm/repo.go' | |||
| 15 | --- charm/repo.go 2014-03-07 13:56:22 +0000 | |||
| 16 | +++ charm/repo.go 2014-03-20 19:57:15 +0000 | |||
| 17 | @@ -24,11 +24,12 @@ | |||
| 18 | 24 | 24 | ||
| 19 | 25 | // InfoResponse is sent by the charm store in response to charm-info requests. | 25 | // InfoResponse is sent by the charm store in response to charm-info requests. |
| 20 | 26 | type InfoResponse struct { | 26 | type InfoResponse struct { |
| 26 | 27 | Revision int `json:"revision"` // Zero is valid. Can't omitempty. | 27 | CanonicalURL string `json:"canonical-url,omitempty"` |
| 27 | 28 | Sha256 string `json:"sha256,omitempty"` | 28 | Revision int `json:"revision"` // Zero is valid. Can't omitempty. |
| 28 | 29 | Digest string `json:"digest,omitempty"` | 29 | Sha256 string `json:"sha256,omitempty"` |
| 29 | 30 | Errors []string `json:"errors,omitempty"` | 30 | Digest string `json:"digest,omitempty"` |
| 30 | 31 | Warnings []string `json:"warnings,omitempty"` | 31 | Errors []string `json:"errors,omitempty"` |
| 31 | 32 | Warnings []string `json:"warnings,omitempty"` | ||
| 32 | 32 | } | 33 | } |
| 33 | 33 | 34 | ||
| 34 | 34 | // EventResponse is sent by the charm store in response to charm-event requests. | 35 | // EventResponse is sent by the charm store in response to charm-event requests. |
| 35 | @@ -53,6 +54,7 @@ | |||
| 36 | 53 | type Repository interface { | 54 | type Repository interface { |
| 37 | 54 | Get(curl *URL) (Charm, error) | 55 | Get(curl *URL) (Charm, error) |
| 38 | 55 | Latest(curls ...*URL) ([]CharmRevision, error) | 56 | Latest(curls ...*URL) ([]CharmRevision, error) |
| 39 | 57 | Resolve(ref Reference) (*URL, error) | ||
| 40 | 56 | } | 58 | } |
| 41 | 57 | 59 | ||
| 42 | 58 | // Latest returns the latest revision of the charm referenced by curl, regardless | 60 | // Latest returns the latest revision of the charm referenced by curl, regardless |
| 43 | @@ -137,8 +139,27 @@ | |||
| 44 | 137 | return http.DefaultClient.Do(req) | 139 | return http.DefaultClient.Do(req) |
| 45 | 138 | } | 140 | } |
| 46 | 139 | 141 | ||
| 47 | 142 | // Resolve canonicalizes charm URLs, resolving references and implied series. | ||
| 48 | 143 | func (s *CharmStore) Resolve(ref Reference) (*URL, error) { | ||
| 49 | 144 | infos, err := s.Info(ref) | ||
| 50 | 145 | if err != nil { | ||
| 51 | 146 | return nil, err | ||
| 52 | 147 | } | ||
| 53 | 148 | if len(infos) == 0 { | ||
| 54 | 149 | return nil, fmt.Errorf("missing response when resolving charm URL: %q", ref) | ||
| 55 | 150 | } | ||
| 56 | 151 | if infos[0].CanonicalURL == "" { | ||
| 57 | 152 | return nil, fmt.Errorf("cannot resolve charm URL: %q", ref) | ||
| 58 | 153 | } | ||
| 59 | 154 | curl, err := ParseURL(infos[0].CanonicalURL) | ||
| 60 | 155 | if err != nil { | ||
| 61 | 156 | return nil, err | ||
| 62 | 157 | } | ||
| 63 | 158 | return curl, nil | ||
| 64 | 159 | } | ||
| 65 | 160 | |||
| 66 | 140 | // Info returns details for all the specified charms in the charm store. | 161 | // Info returns details for all the specified charms in the charm store. |
| 68 | 141 | func (s *CharmStore) Info(curls ...*URL) ([]*InfoResponse, error) { | 162 | func (s *CharmStore) Info(curls ...Location) ([]*InfoResponse, error) { |
| 69 | 142 | baseURL := s.BaseURL + "/charm-info?" | 163 | baseURL := s.BaseURL + "/charm-info?" |
| 70 | 143 | queryParams := make([]string, len(curls), len(curls)+1) | 164 | queryParams := make([]string, len(curls), len(curls)+1) |
| 71 | 144 | for i, curl := range curls { | 165 | for i, curl := range curls { |
| 72 | @@ -218,7 +239,7 @@ | |||
| 73 | 218 | } | 239 | } |
| 74 | 219 | 240 | ||
| 75 | 220 | // revisions returns the revisions of the charms referenced by curls. | 241 | // revisions returns the revisions of the charms referenced by curls. |
| 77 | 221 | func (s *CharmStore) revisions(curls ...*URL) (revisions []CharmRevision, err error) { | 242 | func (s *CharmStore) revisions(curls ...Location) (revisions []CharmRevision, err error) { |
| 78 | 222 | infos, err := s.Info(curls...) | 243 | infos, err := s.Info(curls...) |
| 79 | 223 | if err != nil { | 244 | if err != nil { |
| 80 | 224 | return nil, err | 245 | return nil, err |
| 81 | @@ -246,7 +267,7 @@ | |||
| 82 | 246 | // Latest returns the latest revision of the charms referenced by curls, regardless | 267 | // Latest returns the latest revision of the charms referenced by curls, regardless |
| 83 | 247 | // of the revision set on each curl. | 268 | // of the revision set on each curl. |
| 84 | 248 | func (s *CharmStore) Latest(curls ...*URL) ([]CharmRevision, error) { | 269 | func (s *CharmStore) Latest(curls ...*URL) ([]CharmRevision, error) { |
| 86 | 249 | baseCurls := make([]*URL, len(curls)) | 270 | baseCurls := make([]Location, len(curls)) |
| 87 | 250 | for i, curl := range curls { | 271 | for i, curl := range curls { |
| 88 | 251 | baseCurls[i] = curl.WithRevision(-1) | 272 | baseCurls[i] = curl.WithRevision(-1) |
| 89 | 252 | } | 273 | } |
| 90 | @@ -387,11 +408,27 @@ | |||
| 91 | 387 | // /path/to/repository/precise/mongodb.charm | 408 | // /path/to/repository/precise/mongodb.charm |
| 92 | 388 | // /path/to/repository/precise/wordpress/ | 409 | // /path/to/repository/precise/wordpress/ |
| 93 | 389 | type LocalRepository struct { | 410 | type LocalRepository struct { |
| 95 | 390 | Path string | 411 | Path string |
| 96 | 412 | defaultSeries string | ||
| 97 | 391 | } | 413 | } |
| 98 | 392 | 414 | ||
| 99 | 393 | var _ Repository = (*LocalRepository)(nil) | 415 | var _ Repository = (*LocalRepository)(nil) |
| 100 | 394 | 416 | ||
| 101 | 417 | // WithDefaultSeries returns a Repository with the default series set. | ||
| 102 | 418 | func (r *LocalRepository) WithDefaultSeries(defaultSeries string) Repository { | ||
| 103 | 419 | localRepo := *r | ||
| 104 | 420 | localRepo.defaultSeries = defaultSeries | ||
| 105 | 421 | return &localRepo | ||
| 106 | 422 | } | ||
| 107 | 423 | |||
| 108 | 424 | // Resolve canonicalizes charm URLs, resolving references and implied series. | ||
| 109 | 425 | func (r *LocalRepository) Resolve(ref Reference) (*URL, error) { | ||
| 110 | 426 | if r.defaultSeries == "" { | ||
| 111 | 427 | return nil, fmt.Errorf("cannot resolve, repository has no default series: %q", ref) | ||
| 112 | 428 | } | ||
| 113 | 429 | return &URL{Reference: ref, Series: r.defaultSeries}, nil | ||
| 114 | 430 | } | ||
| 115 | 431 | |||
| 116 | 395 | // Latest returns the latest revision of the charm referenced by curl, regardless | 432 | // Latest returns the latest revision of the charm referenced by curl, regardless |
| 117 | 396 | // of the revision set on curl itself. | 433 | // of the revision set on curl itself. |
| 118 | 397 | func (r *LocalRepository) Latest(curls ...*URL) ([]CharmRevision, error) { | 434 | func (r *LocalRepository) Latest(curls ...*URL) ([]CharmRevision, error) { |
| 119 | 398 | 435 | ||
| 120 | === modified file 'charm/repo_test.go' | |||
| 121 | --- charm/repo_test.go 2014-03-07 13:56:22 +0000 | |||
| 122 | +++ charm/repo_test.go 2014-03-20 19:57:15 +0000 | |||
| 123 | @@ -173,7 +173,7 @@ | |||
| 124 | 173 | // The following tests cover the low-level CharmStore-specific API. | 173 | // The following tests cover the low-level CharmStore-specific API. |
| 125 | 174 | 174 | ||
| 126 | 175 | func (s *StoreSuite) TestInfo(c *gc.C) { | 175 | func (s *StoreSuite) TestInfo(c *gc.C) { |
| 128 | 176 | charmURLs := []*charm.URL{ | 176 | charmURLs := []charm.Location{ |
| 129 | 177 | charm.MustParseURL("cs:series/good"), | 177 | charm.MustParseURL("cs:series/good"), |
| 130 | 178 | charm.MustParseURL("cs:series/better"), | 178 | charm.MustParseURL("cs:series/better"), |
| 131 | 179 | charm.MustParseURL("cs:series/best"), | 179 | charm.MustParseURL("cs:series/best"), |
| 132 | @@ -387,7 +387,7 @@ | |||
| 133 | 387 | func (s *LocalRepoSuite) SetUpTest(c *gc.C) { | 387 | func (s *LocalRepoSuite) SetUpTest(c *gc.C) { |
| 134 | 388 | s.LoggingSuite.SetUpTest(c) | 388 | s.LoggingSuite.SetUpTest(c) |
| 135 | 389 | root := c.MkDir() | 389 | root := c.MkDir() |
| 137 | 390 | s.repo = &charm.LocalRepository{root} | 390 | s.repo = &charm.LocalRepository{Path: root} |
| 138 | 391 | s.seriesPath = filepath.Join(root, "quantal") | 391 | s.seriesPath = filepath.Join(root, "quantal") |
| 139 | 392 | c.Assert(os.Mkdir(s.seriesPath, 0777), gc.IsNil) | 392 | c.Assert(os.Mkdir(s.seriesPath, 0777), gc.IsNil) |
| 140 | 393 | } | 393 | } |
| 141 | 394 | 394 | ||
| 142 | === modified file 'charm/testing/mockstore.go' | |||
| 143 | --- charm/testing/mockstore.go 2014-03-07 14:15:28 +0000 | |||
| 144 | +++ charm/testing/mockstore.go 2014-03-20 19:57:15 +0000 | |||
| 145 | @@ -6,6 +6,7 @@ | |||
| 146 | 6 | import ( | 6 | import ( |
| 147 | 7 | "bytes" | 7 | "bytes" |
| 148 | 8 | "encoding/json" | 8 | "encoding/json" |
| 149 | 9 | "fmt" | ||
| 150 | 9 | "io" | 10 | "io" |
| 151 | 10 | "net" | 11 | "net" |
| 152 | 11 | "net/http" | 12 | "net/http" |
| 153 | @@ -35,13 +36,14 @@ | |||
| 154 | 35 | Metadata []string | 36 | Metadata []string |
| 155 | 36 | InfoRequestCount int | 37 | InfoRequestCount int |
| 156 | 37 | InfoRequestCountNoStats int | 38 | InfoRequestCountNoStats int |
| 157 | 39 | DefaultSeries string | ||
| 158 | 38 | 40 | ||
| 159 | 39 | charms map[string]int | 41 | charms map[string]int |
| 160 | 40 | } | 42 | } |
| 161 | 41 | 43 | ||
| 162 | 42 | // NewMockStore creates a mock charm store containing the specified charms. | 44 | // NewMockStore creates a mock charm store containing the specified charms. |
| 163 | 43 | func NewMockStore(c *gc.C, charms map[string]int) *MockStore { | 45 | func NewMockStore(c *gc.C, charms map[string]int) *MockStore { |
| 165 | 44 | s := &MockStore{charms: charms} | 46 | s := &MockStore{charms: charms, DefaultSeries: "precise"} |
| 166 | 45 | f, err := os.Open(testing.Charms.BundlePath(c.MkDir(), "dummy")) | 47 | f, err := os.Open(testing.Charms.BundlePath(c.MkDir(), "dummy")) |
| 167 | 46 | c.Assert(err, gc.IsNil) | 48 | c.Assert(err, gc.IsNil) |
| 168 | 47 | defer f.Close() | 49 | defer f.Close() |
| 169 | @@ -98,7 +100,17 @@ | |||
| 170 | 98 | for _, url := range r.Form["charms"] { | 100 | for _, url := range r.Form["charms"] { |
| 171 | 99 | cr := &charm.InfoResponse{} | 101 | cr := &charm.InfoResponse{} |
| 172 | 100 | response[url] = cr | 102 | response[url] = cr |
| 174 | 101 | charmURL := charm.MustParseURL(url) | 103 | charmURL, err := charm.ParseURL(url) |
| 175 | 104 | if err == charm.ErrUnresolvedUrl { | ||
| 176 | 105 | ref, _, err := charm.ParseReference(url) | ||
| 177 | 106 | if err != nil { | ||
| 178 | 107 | panic(err) | ||
| 179 | 108 | } | ||
| 180 | 109 | if s.DefaultSeries == "" { | ||
| 181 | 110 | panic(fmt.Errorf("mock store lacks a default series cannot resolve charm URL: %q", url)) | ||
| 182 | 111 | } | ||
| 183 | 112 | charmURL = &charm.URL{Reference: ref, Series: s.DefaultSeries} | ||
| 184 | 113 | } | ||
| 185 | 102 | switch charmURL.Name { | 114 | switch charmURL.Name { |
| 186 | 103 | case "borken": | 115 | case "borken": |
| 187 | 104 | cr.Errors = append(cr.Errors, "badness") | 116 | cr.Errors = append(cr.Errors, "badness") |
| 188 | @@ -115,6 +127,7 @@ | |||
| 189 | 115 | cr.Revision = charmURL.Revision | 127 | cr.Revision = charmURL.Revision |
| 190 | 116 | } | 128 | } |
| 191 | 117 | cr.Sha256 = s.bundleSha256 | 129 | cr.Sha256 = s.bundleSha256 |
| 192 | 130 | cr.CanonicalURL = charmURL.String() | ||
| 193 | 118 | } else { | 131 | } else { |
| 194 | 119 | cr.Errors = append(cr.Errors, "entry not found") | 132 | cr.Errors = append(cr.Errors, "entry not found") |
| 195 | 120 | } | 133 | } |
| 196 | 121 | 134 | ||
| 197 | === modified file 'charm/url.go' | |||
| 198 | --- charm/url.go 2014-01-28 10:45:55 +0000 | |||
| 199 | +++ charm/url.go 2014-03-20 19:57:15 +0000 | |||
| 200 | @@ -13,23 +13,42 @@ | |||
| 201 | 13 | "labix.org/v2/mgo/bson" | 13 | "labix.org/v2/mgo/bson" |
| 202 | 14 | ) | 14 | ) |
| 203 | 15 | 15 | ||
| 205 | 16 | // A charm URL represents charm locations such as: | 16 | // Location represents a charm location, which must declare a path component |
| 206 | 17 | // and a string representaion. | ||
| 207 | 18 | type Location interface { | ||
| 208 | 19 | Path() string | ||
| 209 | 20 | String() string | ||
| 210 | 21 | } | ||
| 211 | 22 | |||
| 212 | 23 | // Reference represents a charm location with an unresolved, untargeted series, | ||
| 213 | 24 | // such as: | ||
| 214 | 25 | // | ||
| 215 | 26 | // cs:~joe/wordpress | ||
| 216 | 27 | // cs:wordpress-42 | ||
| 217 | 28 | type Reference struct { | ||
| 218 | 29 | Schema string // "cs" or "local" | ||
| 219 | 30 | User string // "joe" | ||
| 220 | 31 | Name string // "wordpress" | ||
| 221 | 32 | Revision int // -1 if unset, N otherwise | ||
| 222 | 33 | } | ||
| 223 | 34 | |||
| 224 | 35 | // URL represents a fully resolved charm location with a specific series, such | ||
| 225 | 36 | // as: | ||
| 226 | 17 | // | 37 | // |
| 227 | 18 | // cs:~joe/oneiric/wordpress | 38 | // cs:~joe/oneiric/wordpress |
| 228 | 19 | // cs:oneiric/wordpress-42 | 39 | // cs:oneiric/wordpress-42 |
| 229 | 20 | // local:oneiric/wordpress | 40 | // local:oneiric/wordpress |
| 230 | 21 | // | 41 | // |
| 231 | 22 | type URL struct { | 42 | type URL struct { |
| 237 | 23 | Schema string // "cs" or "local" | 43 | Reference |
| 238 | 24 | User string // "joe" | 44 | Series string // "oneiric" |
| 234 | 25 | Series string // "oneiric" | ||
| 235 | 26 | Name string // "wordpress" | ||
| 236 | 27 | Revision int // -1 if unset, N otherwise | ||
| 239 | 28 | } | 45 | } |
| 240 | 29 | 46 | ||
| 241 | 47 | var ErrUnresolvedUrl error = fmt.Errorf("charm url series is not resolved") | ||
| 242 | 48 | |||
| 243 | 30 | var ( | 49 | var ( |
| 244 | 31 | validUser = regexp.MustCompile("^[a-z0-9][a-zA-Z0-9+.-]+$") | 50 | validUser = regexp.MustCompile("^[a-z0-9][a-zA-Z0-9+.-]+$") |
| 246 | 32 | validSeries = regexp.MustCompile("^[a-z]+([a-z-]+[a-z])?$") | 51 | validSeries = regexp.MustCompile("^[a-z]+([a-z0-9-]+[a-z0-9])?$") |
| 247 | 33 | validName = regexp.MustCompile("^[a-z][a-z0-9]*(-[a-z0-9]*[a-z][a-z0-9]*)*$") | 52 | validName = regexp.MustCompile("^[a-z][a-z0-9]*(-[a-z0-9]*[a-z][a-z0-9]*)*$") |
| 248 | 34 | ) | 53 | ) |
| 249 | 35 | 54 | ||
| 250 | @@ -68,67 +87,83 @@ | |||
| 251 | 68 | // ParseURL parses the provided charm URL string into its respective | 87 | // ParseURL parses the provided charm URL string into its respective |
| 252 | 69 | // structure. | 88 | // structure. |
| 253 | 70 | func ParseURL(url string) (*URL, error) { | 89 | func ParseURL(url string) (*URL, error) { |
| 255 | 71 | u := &URL{} | 90 | r, series, err := ParseReference(url) |
| 256 | 91 | if err != nil { | ||
| 257 | 92 | return nil, err | ||
| 258 | 93 | } | ||
| 259 | 94 | if series == "" { | ||
| 260 | 95 | return nil, ErrUnresolvedUrl | ||
| 261 | 96 | } | ||
| 262 | 97 | return &URL{Reference: r, Series: series}, nil | ||
| 263 | 98 | } | ||
| 264 | 99 | |||
| 265 | 100 | // ParseReference parses the provided charm Reference string into its | ||
| 266 | 101 | // respective structure and the targeted series, if present. | ||
| 267 | 102 | func ParseReference(url string) (Reference, string, error) { | ||
| 268 | 103 | r := Reference{Schema: "cs"} | ||
| 269 | 104 | series := "" | ||
| 270 | 72 | i := strings.Index(url, ":") | 105 | i := strings.Index(url, ":") |
| 273 | 73 | if i > 0 { | 106 | if i >= 0 { |
| 274 | 74 | u.Schema = url[:i] | 107 | r.Schema = url[:i] |
| 275 | 75 | i++ | 108 | i++ |
| 276 | 109 | } else { | ||
| 277 | 110 | i = 0 | ||
| 278 | 76 | } | 111 | } |
| 279 | 77 | // cs: or local: | 112 | // cs: or local: |
| 282 | 78 | if u.Schema != "cs" && u.Schema != "local" { | 113 | if r.Schema != "cs" && r.Schema != "local" { |
| 283 | 79 | return nil, fmt.Errorf("charm URL has invalid schema: %q", url) | 114 | return Reference{}, "", fmt.Errorf("charm URL has invalid schema: %q", url) |
| 284 | 80 | } | 115 | } |
| 285 | 81 | parts := strings.Split(url[i:], "/") | 116 | parts := strings.Split(url[i:], "/") |
| 286 | 82 | if len(parts) < 1 || len(parts) > 3 { | 117 | if len(parts) < 1 || len(parts) > 3 { |
| 288 | 83 | return nil, fmt.Errorf("charm URL has invalid form: %q", url) | 118 | return Reference{}, "", fmt.Errorf("charm URL has invalid form: %q", url) |
| 289 | 84 | } | 119 | } |
| 290 | 85 | 120 | ||
| 291 | 86 | // ~<username> | 121 | // ~<username> |
| 292 | 87 | if strings.HasPrefix(parts[0], "~") { | 122 | if strings.HasPrefix(parts[0], "~") { |
| 295 | 88 | if u.Schema == "local" { | 123 | if r.Schema == "local" { |
| 296 | 89 | return nil, fmt.Errorf("local charm URL with user name: %q", url) | 124 | return Reference{}, "", fmt.Errorf("local charm URL with user name: %q", url) |
| 297 | 90 | } | 125 | } |
| 301 | 91 | u.User = parts[0][1:] | 126 | r.User = parts[0][1:] |
| 302 | 92 | if !IsValidUser(u.User) { | 127 | if !IsValidUser(r.User) { |
| 303 | 93 | return nil, fmt.Errorf("charm URL has invalid user name: %q", url) | 128 | return Reference{}, "", fmt.Errorf("charm URL has invalid user name: %q", url) |
| 304 | 94 | } | 129 | } |
| 305 | 95 | parts = parts[1:] | 130 | parts = parts[1:] |
| 306 | 96 | } | 131 | } |
| 307 | 97 | 132 | ||
| 308 | 98 | // <series> | 133 | // <series> |
| 309 | 99 | if len(parts) < 2 { | ||
| 310 | 100 | return nil, fmt.Errorf("charm URL without series: %q", url) | ||
| 311 | 101 | } | ||
| 312 | 102 | if len(parts) == 2 { | 134 | if len(parts) == 2 { |
| 316 | 103 | u.Series = parts[0] | 135 | series = parts[0] |
| 317 | 104 | if !IsValidSeries(u.Series) { | 136 | if !IsValidSeries(series) { |
| 318 | 105 | return nil, fmt.Errorf("charm URL has invalid series: %q", url) | 137 | return Reference{}, "", fmt.Errorf("charm URL has invalid series: %q", url) |
| 319 | 106 | } | 138 | } |
| 320 | 107 | parts = parts[1:] | 139 | parts = parts[1:] |
| 321 | 108 | } | 140 | } |
| 322 | 141 | if len(parts) < 1 { | ||
| 323 | 142 | return Reference{}, "", fmt.Errorf("charm URL without charm name: %q", url) | ||
| 324 | 143 | } | ||
| 325 | 109 | 144 | ||
| 326 | 110 | // <name>[-<revision>] | 145 | // <name>[-<revision>] |
| 331 | 111 | u.Name = parts[0] | 146 | r.Name = parts[0] |
| 332 | 112 | u.Revision = -1 | 147 | r.Revision = -1 |
| 333 | 113 | for i := len(u.Name) - 1; i > 0; i-- { | 148 | for i := len(r.Name) - 1; i > 0; i-- { |
| 334 | 114 | c := u.Name[i] | 149 | c := r.Name[i] |
| 335 | 115 | if c >= '0' && c <= '9' { | 150 | if c >= '0' && c <= '9' { |
| 336 | 116 | continue | 151 | continue |
| 337 | 117 | } | 152 | } |
| 339 | 118 | if c == '-' && i != len(u.Name)-1 { | 153 | if c == '-' && i != len(r.Name)-1 { |
| 340 | 119 | var err error | 154 | var err error |
| 342 | 120 | u.Revision, err = strconv.Atoi(u.Name[i+1:]) | 155 | r.Revision, err = strconv.Atoi(r.Name[i+1:]) |
| 343 | 121 | if err != nil { | 156 | if err != nil { |
| 344 | 122 | panic(err) // We just checked it was right. | 157 | panic(err) // We just checked it was right. |
| 345 | 123 | } | 158 | } |
| 347 | 124 | u.Name = u.Name[:i] | 159 | r.Name = r.Name[:i] |
| 348 | 125 | } | 160 | } |
| 349 | 126 | break | 161 | break |
| 350 | 127 | } | 162 | } |
| 353 | 128 | if !IsValidName(u.Name) { | 163 | if !IsValidName(r.Name) { |
| 354 | 129 | return nil, fmt.Errorf("charm URL has invalid charm name: %q", url) | 164 | return Reference{}, "", fmt.Errorf("charm URL has invalid charm name: %q", url) |
| 355 | 130 | } | 165 | } |
| 357 | 131 | return u, nil | 166 | return r, series, nil |
| 358 | 132 | } | 167 | } |
| 359 | 133 | 168 | ||
| 360 | 134 | // InferURL returns a charm URL inferred from src. The provided | 169 | // InferURL returns a charm URL inferred from src. The provided |
| 361 | @@ -148,9 +183,12 @@ | |||
| 362 | 148 | // when src does not include that information; similarly, a missing | 183 | // when src does not include that information; similarly, a missing |
| 363 | 149 | // schema is assumed to be 'cs'. | 184 | // schema is assumed to be 'cs'. |
| 364 | 150 | func InferURL(src, defaultSeries string) (*URL, error) { | 185 | func InferURL(src, defaultSeries string) (*URL, error) { |
| 368 | 151 | if u, err := ParseURL(src); err == nil { | 186 | r, series, err := ParseReference(src) |
| 369 | 152 | // src was a valid charm URL already | 187 | if err != nil { |
| 370 | 153 | return u, nil | 188 | return nil, err |
| 371 | 189 | } | ||
| 372 | 190 | if series != "" { | ||
| 373 | 191 | return &URL{Reference: r, Series: series}, nil | ||
| 374 | 154 | } | 192 | } |
| 375 | 155 | if strings.HasPrefix(src, "~") { | 193 | if strings.HasPrefix(src, "~") { |
| 376 | 156 | return nil, fmt.Errorf("cannot infer charm URL with user but no schema: %q", src) | 194 | return nil, fmt.Errorf("cannot infer charm URL with user but no schema: %q", src) |
| 377 | @@ -187,22 +225,37 @@ | |||
| 378 | 187 | } | 225 | } |
| 379 | 188 | 226 | ||
| 380 | 189 | func (u *URL) Path() string { | 227 | func (u *URL) Path() string { |
| 391 | 190 | if u.User != "" { | 228 | return u.path(u.Series) |
| 392 | 191 | if u.Revision >= 0 { | 229 | } |
| 393 | 192 | return fmt.Sprintf("~%s/%s/%s-%d", u.User, u.Series, u.Name, u.Revision) | 230 | |
| 394 | 193 | } | 231 | func (r Reference) path(series string) string { |
| 395 | 194 | return fmt.Sprintf("~%s/%s/%s", u.User, u.Series, u.Name) | 232 | var parts []string |
| 396 | 195 | } | 233 | if r.User != "" { |
| 397 | 196 | if u.Revision >= 0 { | 234 | parts = append(parts, fmt.Sprintf("~%s", r.User)) |
| 398 | 197 | return fmt.Sprintf("%s/%s-%d", u.Series, u.Name, u.Revision) | 235 | } |
| 399 | 198 | } | 236 | if series != "" { |
| 400 | 199 | return fmt.Sprintf("%s/%s", u.Series, u.Name) | 237 | parts = append(parts, series) |
| 401 | 238 | } | ||
| 402 | 239 | if r.Revision >= 0 { | ||
| 403 | 240 | parts = append(parts, fmt.Sprintf("%s-%d", r.Name, r.Revision)) | ||
| 404 | 241 | } else { | ||
| 405 | 242 | parts = append(parts, r.Name) | ||
| 406 | 243 | } | ||
| 407 | 244 | return strings.Join(parts, "/") | ||
| 408 | 245 | } | ||
| 409 | 246 | |||
| 410 | 247 | func (r Reference) Path() string { | ||
| 411 | 248 | return r.path("") | ||
| 412 | 200 | } | 249 | } |
| 413 | 201 | 250 | ||
| 414 | 202 | func (u *URL) String() string { | 251 | func (u *URL) String() string { |
| 415 | 203 | return fmt.Sprintf("%s:%s", u.Schema, u.Path()) | 252 | return fmt.Sprintf("%s:%s", u.Schema, u.Path()) |
| 416 | 204 | } | 253 | } |
| 417 | 205 | 254 | ||
| 418 | 255 | func (r Reference) String() string { | ||
| 419 | 256 | return fmt.Sprintf("%s:%s", r.Schema, r.Path()) | ||
| 420 | 257 | } | ||
| 421 | 258 | |||
| 422 | 206 | // GetBSON turns u into a bson.Getter so it can be saved directly | 259 | // GetBSON turns u into a bson.Getter so it can be saved directly |
| 423 | 207 | // on a MongoDB database with mgo. | 260 | // on a MongoDB database with mgo. |
| 424 | 208 | func (u *URL) GetBSON() (interface{}, error) { | 261 | func (u *URL) GetBSON() (interface{}, error) { |
| 425 | 209 | 262 | ||
| 426 | === modified file 'charm/url_test.go' | |||
| 427 | --- charm/url_test.go 2014-01-28 10:45:55 +0000 | |||
| 428 | +++ charm/url_test.go 2014-03-20 19:57:15 +0000 | |||
| 429 | @@ -6,6 +6,7 @@ | |||
| 430 | 6 | import ( | 6 | import ( |
| 431 | 7 | "encoding/json" | 7 | "encoding/json" |
| 432 | 8 | "fmt" | 8 | "fmt" |
| 433 | 9 | "strings" | ||
| 434 | 9 | 10 | ||
| 435 | 10 | "labix.org/v2/mgo/bson" | 11 | "labix.org/v2/mgo/bson" |
| 436 | 11 | gc "launchpad.net/gocheck" | 12 | gc "launchpad.net/gocheck" |
| 437 | @@ -21,38 +22,63 @@ | |||
| 438 | 21 | s, err string | 22 | s, err string |
| 439 | 22 | url *charm.URL | 23 | url *charm.URL |
| 440 | 23 | }{ | 24 | }{ |
| 448 | 24 | {"cs:~user/series/name", "", &charm.URL{"cs", "user", "series", "name", -1}}, | 25 | {"cs:~user/series/name", "", &charm.URL{charm.Reference{"cs", "user", "name", -1}, "series"}}, |
| 449 | 25 | {"cs:~user/series/name-0", "", &charm.URL{"cs", "user", "series", "name", 0}}, | 26 | {"cs:~user/series/name-0", "", &charm.URL{charm.Reference{"cs", "user", "name", 0}, "series"}}, |
| 450 | 26 | {"cs:series/name", "", &charm.URL{"cs", "", "series", "name", -1}}, | 27 | {"cs:series/name", "", &charm.URL{charm.Reference{"cs", "", "name", -1}, "series"}}, |
| 451 | 27 | {"cs:series/name-42", "", &charm.URL{"cs", "", "series", "name", 42}}, | 28 | {"cs:series/name-42", "", &charm.URL{charm.Reference{"cs", "", "name", 42}, "series"}}, |
| 452 | 28 | {"local:series/name-1", "", &charm.URL{"local", "", "series", "name", 1}}, | 29 | {"local:series/name-1", "", &charm.URL{charm.Reference{"local", "", "name", 1}, "series"}}, |
| 453 | 29 | {"local:series/name", "", &charm.URL{"local", "", "series", "name", -1}}, | 30 | {"local:series/name", "", &charm.URL{charm.Reference{"local", "", "name", -1}, "series"}}, |
| 454 | 30 | {"local:series/n0-0n-n0", "", &charm.URL{"local", "", "series", "n0-0n-n0", -1}}, | 31 | {"local:series/n0-0n-n0", "", &charm.URL{charm.Reference{"local", "", "n0-0n-n0", -1}, "series"}}, |
| 455 | 32 | {"cs:~user/name", "", &charm.URL{charm.Reference{"cs", "user", "name", -1}, ""}}, | ||
| 456 | 33 | {"cs:name", "", &charm.URL{charm.Reference{"cs", "", "name", -1}, ""}}, | ||
| 457 | 34 | {"local:name", "", &charm.URL{charm.Reference{"local", "", "name", -1}, ""}}, | ||
| 458 | 31 | 35 | ||
| 459 | 32 | {"bs:~user/series/name-1", "charm URL has invalid schema: .*", nil}, | 36 | {"bs:~user/series/name-1", "charm URL has invalid schema: .*", nil}, |
| 460 | 33 | {"cs:~1/series/name-1", "charm URL has invalid user name: .*", nil}, | 37 | {"cs:~1/series/name-1", "charm URL has invalid user name: .*", nil}, |
| 461 | 38 | {"cs:~user", "charm URL without charm name: .*", nil}, | ||
| 462 | 34 | {"cs:~user/1/name-1", "charm URL has invalid series: .*", nil}, | 39 | {"cs:~user/1/name-1", "charm URL has invalid series: .*", nil}, |
| 463 | 35 | {"cs:~user/series/name-1-2", "charm URL has invalid charm name: .*", nil}, | 40 | {"cs:~user/series/name-1-2", "charm URL has invalid charm name: .*", nil}, |
| 464 | 36 | {"cs:~user/series/name-1-name-2", "charm URL has invalid charm name: .*", nil}, | 41 | {"cs:~user/series/name-1-name-2", "charm URL has invalid charm name: .*", nil}, |
| 465 | 37 | {"cs:~user/series/name--name-2", "charm URL has invalid charm name: .*", nil}, | 42 | {"cs:~user/series/name--name-2", "charm URL has invalid charm name: .*", nil}, |
| 466 | 38 | {"cs:~user/series/huh/name-1", "charm URL has invalid form: .*", nil}, | 43 | {"cs:~user/series/huh/name-1", "charm URL has invalid form: .*", nil}, |
| 469 | 39 | {"cs:~user/name", "charm URL without series: .*", nil}, | 44 | {"cs:/name", "charm URL has invalid series: .*", nil}, |
| 468 | 40 | {"cs:name", "charm URL without series: .*", nil}, | ||
| 470 | 41 | {"local:~user/series/name", "local charm URL with user name: .*", nil}, | 45 | {"local:~user/series/name", "local charm URL with user name: .*", nil}, |
| 471 | 42 | {"local:~user/name", "local charm URL with user name: .*", nil}, | 46 | {"local:~user/name", "local charm URL with user name: .*", nil}, |
| 472 | 43 | {"local:name", "charm URL without series: .*", nil}, | ||
| 473 | 44 | } | 47 | } |
| 474 | 45 | 48 | ||
| 475 | 46 | func (s *URLSuite) TestParseURL(c *gc.C) { | 49 | func (s *URLSuite) TestParseURL(c *gc.C) { |
| 476 | 47 | for i, t := range urlTests { | 50 | for i, t := range urlTests { |
| 477 | 48 | c.Logf("test %d", i) | 51 | c.Logf("test %d", i) |
| 479 | 49 | url, err := charm.ParseURL(t.s) | 52 | url, uerr := charm.ParseURL(t.s) |
| 480 | 53 | ref, series, rerr := charm.ParseReference(t.s) | ||
| 481 | 50 | comment := gc.Commentf("ParseURL(%q)", t.s) | 54 | comment := gc.Commentf("ParseURL(%q)", t.s) |
| 484 | 51 | if t.err != "" { | 55 | if t.url != nil && t.url.Series == "" { |
| 485 | 52 | c.Check(err.Error(), gc.Matches, t.err, comment) | 56 | if t.err != "" { |
| 486 | 57 | // Expected error should match | ||
| 487 | 58 | c.Assert(rerr, gc.NotNil, comment) | ||
| 488 | 59 | c.Check(rerr.Error(), gc.Matches, t.err, comment) | ||
| 489 | 60 | } else { | ||
| 490 | 61 | // Expected charm reference should match | ||
| 491 | 62 | c.Check(ref, gc.DeepEquals, t.url.Reference, comment) | ||
| 492 | 63 | c.Check(t.url.Reference.String(), gc.Equals, t.s) | ||
| 493 | 64 | } | ||
| 494 | 65 | if rerr != nil { | ||
| 495 | 66 | // If ParseReference has an error, ParseURL should share it | ||
| 496 | 67 | c.Check(uerr.Error(), gc.Equals, rerr.Error(), comment) | ||
| 497 | 68 | } else { | ||
| 498 | 69 | // Otherwise, ParseURL with an empty series should error unresolved. | ||
| 499 | 70 | c.Check(uerr.Error(), gc.Equals, charm.ErrUnresolvedUrl.Error(), comment) | ||
| 500 | 71 | } | ||
| 501 | 53 | } else { | 72 | } else { |
| 504 | 54 | c.Check(url, gc.DeepEquals, t.url, comment) | 73 | if t.err != "" { |
| 505 | 55 | c.Check(t.url.String(), gc.Equals, t.s) | 74 | c.Assert(uerr, gc.NotNil, comment) |
| 506 | 75 | c.Check(uerr.Error(), gc.Matches, t.err, comment) | ||
| 507 | 76 | c.Check(uerr.Error(), gc.Equals, rerr.Error(), comment) | ||
| 508 | 77 | } else { | ||
| 509 | 78 | c.Check(url.Series, gc.Equals, series, comment) | ||
| 510 | 79 | c.Check(url, gc.DeepEquals, t.url, comment) | ||
| 511 | 80 | c.Check(t.url.String(), gc.Equals, t.s) | ||
| 512 | 81 | } | ||
| 513 | 56 | } | 82 | } |
| 514 | 57 | } | 83 | } |
| 515 | 58 | } | 84 | } |
| 516 | @@ -82,31 +108,35 @@ | |||
| 517 | 82 | comment := gc.Commentf("InferURL(%q, %q)", t.vague, "defseries") | 108 | comment := gc.Commentf("InferURL(%q, %q)", t.vague, "defseries") |
| 518 | 83 | inferred, ierr := charm.InferURL(t.vague, "defseries") | 109 | inferred, ierr := charm.InferURL(t.vague, "defseries") |
| 519 | 84 | parsed, perr := charm.ParseURL(t.exact) | 110 | parsed, perr := charm.ParseURL(t.exact) |
| 521 | 85 | if parsed != nil { | 111 | if perr == nil { |
| 522 | 86 | c.Check(inferred, gc.DeepEquals, parsed, comment) | 112 | c.Check(inferred, gc.DeepEquals, parsed, comment) |
| 523 | 113 | c.Check(ierr, gc.IsNil) | ||
| 524 | 87 | } else { | 114 | } else { |
| 525 | 88 | expect := perr.Error() | 115 | expect := perr.Error() |
| 526 | 89 | if t.vague != t.exact { | 116 | if t.vague != t.exact { |
| 528 | 90 | expect = fmt.Sprintf("%s (URL inferred from %q)", expect, t.vague) | 117 | if colIdx := strings.Index(expect, ":"); colIdx > 0 { |
| 529 | 118 | expect = expect[:colIdx] | ||
| 530 | 119 | } | ||
| 531 | 91 | } | 120 | } |
| 533 | 92 | c.Check(ierr.Error(), gc.Equals, expect, comment) | 121 | c.Check(ierr.Error(), gc.Matches, expect+".*", comment) |
| 534 | 93 | } | 122 | } |
| 535 | 94 | } | 123 | } |
| 536 | 95 | u, err := charm.InferURL("~blah", "defseries") | 124 | u, err := charm.InferURL("~blah", "defseries") |
| 537 | 96 | c.Assert(u, gc.IsNil) | 125 | c.Assert(u, gc.IsNil) |
| 539 | 97 | c.Assert(err, gc.ErrorMatches, "cannot infer charm URL with user but no schema: .*") | 126 | c.Assert(err, gc.ErrorMatches, "charm URL without charm name: .*") |
| 540 | 98 | } | 127 | } |
| 541 | 99 | 128 | ||
| 542 | 100 | var inferNoDefaultSeriesTests = []struct { | 129 | var inferNoDefaultSeriesTests = []struct { |
| 543 | 101 | vague, exact string | 130 | vague, exact string |
| 544 | 131 | resolved bool | ||
| 545 | 102 | }{ | 132 | }{ |
| 553 | 103 | {"foo", ""}, | 133 | {"foo", "", false}, |
| 554 | 104 | {"foo-1", ""}, | 134 | {"foo-1", "", false}, |
| 555 | 105 | {"cs:foo", ""}, | 135 | {"cs:foo", "", false}, |
| 556 | 106 | {"cs:~user/foo", ""}, | 136 | {"cs:~user/foo", "", false}, |
| 557 | 107 | {"series/foo", "cs:series/foo"}, | 137 | {"series/foo", "cs:series/foo", true}, |
| 558 | 108 | {"cs:series/foo", "cs:series/foo"}, | 138 | {"cs:series/foo", "cs:series/foo", true}, |
| 559 | 109 | {"cs:~user/series/foo", "cs:~user/series/foo"}, | 139 | {"cs:~user/series/foo", "cs:~user/series/foo", true}, |
| 560 | 110 | } | 140 | } |
| 561 | 111 | 141 | ||
| 562 | 112 | func (s *URLSuite) TestInferURLNoDefaultSeries(c *gc.C) { | 142 | func (s *URLSuite) TestInferURLNoDefaultSeries(c *gc.C) { |
| 563 | @@ -122,6 +152,23 @@ | |||
| 564 | 122 | } | 152 | } |
| 565 | 123 | } | 153 | } |
| 566 | 124 | 154 | ||
| 567 | 155 | func (s *URLSuite) TestParseUnresolved(c *gc.C) { | ||
| 568 | 156 | for _, t := range inferNoDefaultSeriesTests { | ||
| 569 | 157 | if t.resolved { | ||
| 570 | 158 | url, err := charm.ParseURL(t.vague) | ||
| 571 | 159 | c.Assert(err, gc.IsNil) | ||
| 572 | 160 | c.Assert(url.Series, gc.Not(gc.Equals), "") | ||
| 573 | 161 | } else { | ||
| 574 | 162 | _, series, err := charm.ParseReference(t.vague) | ||
| 575 | 163 | c.Assert(err, gc.IsNil) | ||
| 576 | 164 | c.Assert(series, gc.Equals, "") | ||
| 577 | 165 | _, err = charm.ParseURL(t.vague) | ||
| 578 | 166 | c.Assert(err, gc.NotNil) | ||
| 579 | 167 | c.Assert(err.Error(), gc.Equals, charm.ErrUnresolvedUrl.Error()) | ||
| 580 | 168 | } | ||
| 581 | 169 | } | ||
| 582 | 170 | } | ||
| 583 | 171 | |||
| 584 | 125 | var validTests = []struct { | 172 | var validTests = []struct { |
| 585 | 126 | valid func(string) bool | 173 | valid func(string) bool |
| 586 | 127 | string string | 174 | string string |
| 587 | @@ -160,31 +207,38 @@ | |||
| 588 | 160 | {charm.IsValidSeries, "pre cise", false}, | 207 | {charm.IsValidSeries, "pre cise", false}, |
| 589 | 161 | {charm.IsValidSeries, "pre-cise", true}, | 208 | {charm.IsValidSeries, "pre-cise", true}, |
| 590 | 162 | {charm.IsValidSeries, "pre^cise", false}, | 209 | {charm.IsValidSeries, "pre^cise", false}, |
| 592 | 163 | {charm.IsValidSeries, "prec1se", false}, | 210 | {charm.IsValidSeries, "prec1se", true}, |
| 593 | 164 | {charm.IsValidSeries, "-precise", false}, | 211 | {charm.IsValidSeries, "-precise", false}, |
| 594 | 165 | {charm.IsValidSeries, "precise-", false}, | 212 | {charm.IsValidSeries, "precise-", false}, |
| 596 | 166 | {charm.IsValidSeries, "pre-c1se", false}, | 213 | {charm.IsValidSeries, "pre-c1se", true}, |
| 597 | 167 | } | 214 | } |
| 598 | 168 | 215 | ||
| 599 | 169 | func (s *URLSuite) TestValidCheckers(c *gc.C) { | 216 | func (s *URLSuite) TestValidCheckers(c *gc.C) { |
| 600 | 170 | for i, t := range validTests { | 217 | for i, t := range validTests { |
| 601 | 171 | c.Logf("test %d: %s", i, t.string) | 218 | c.Logf("test %d: %s", i, t.string) |
| 603 | 172 | c.Assert(t.valid(t.string), gc.Equals, t.expect) | 219 | c.Assert(t.valid(t.string), gc.Equals, t.expect, gc.Commentf("%s", t.string)) |
| 604 | 173 | } | 220 | } |
| 605 | 174 | } | 221 | } |
| 606 | 175 | 222 | ||
| 607 | 176 | func (s *URLSuite) TestMustParseURL(c *gc.C) { | 223 | func (s *URLSuite) TestMustParseURL(c *gc.C) { |
| 608 | 177 | url := charm.MustParseURL("cs:series/name") | 224 | url := charm.MustParseURL("cs:series/name") |
| 612 | 178 | c.Assert(url, gc.DeepEquals, &charm.URL{"cs", "", "series", "name", -1}) | 225 | c.Assert(url, gc.DeepEquals, |
| 613 | 179 | f := func() { charm.MustParseURL("local:name") } | 226 | &charm.URL{Reference: charm.Reference{"cs", "", "name", -1}, Series: "series"}) |
| 614 | 180 | c.Assert(f, gc.PanicMatches, "charm URL without series: .*") | 227 | f := func() { charm.MustParseURL("local:@@/name") } |
| 615 | 228 | c.Assert(f, gc.PanicMatches, "charm URL has invalid series: .*") | ||
| 616 | 229 | f = func() { charm.MustParseURL("cs:~user") } | ||
| 617 | 230 | c.Assert(f, gc.PanicMatches, "charm URL without charm name: .*") | ||
| 618 | 231 | f = func() { charm.MustParseURL("cs:~user") } | ||
| 619 | 232 | c.Assert(f, gc.PanicMatches, "charm URL without charm name: .*") | ||
| 620 | 233 | f = func() { charm.MustParseURL("cs:name") } | ||
| 621 | 234 | c.Assert(f, gc.PanicMatches, "charm url series is not resolved") | ||
| 622 | 181 | } | 235 | } |
| 623 | 182 | 236 | ||
| 624 | 183 | func (s *URLSuite) TestWithRevision(c *gc.C) { | 237 | func (s *URLSuite) TestWithRevision(c *gc.C) { |
| 625 | 184 | url := charm.MustParseURL("cs:series/name") | 238 | url := charm.MustParseURL("cs:series/name") |
| 626 | 185 | other := url.WithRevision(1) | 239 | other := url.WithRevision(1) |
| 629 | 186 | c.Assert(url, gc.DeepEquals, &charm.URL{"cs", "", "series", "name", -1}) | 240 | c.Assert(url, gc.DeepEquals, &charm.URL{charm.Reference{"cs", "", "name", -1}, "series"}) |
| 630 | 187 | c.Assert(other, gc.DeepEquals, &charm.URL{"cs", "", "series", "name", 1}) | 241 | c.Assert(other, gc.DeepEquals, &charm.URL{charm.Reference{"cs", "", "name", 1}, "series"}) |
| 631 | 188 | 242 | ||
| 632 | 189 | // Should always copy. The opposite behavior is error prone. | 243 | // Should always copy. The opposite behavior is error prone. |
| 633 | 190 | c.Assert(other.WithRevision(1), gc.Not(gc.Equals), other) | 244 | c.Assert(other.WithRevision(1), gc.Not(gc.Equals), other) |
| 634 | 191 | 245 | ||
| 635 | === modified file 'cmd/juju/bootstrap_test.go' | |||
| 636 | --- cmd/juju/bootstrap_test.go 2014-03-19 04:06:58 +0000 | |||
| 637 | +++ cmd/juju/bootstrap_test.go 2014-03-20 19:57:15 +0000 | |||
| 638 | @@ -292,8 +292,8 @@ | |||
| 639 | 292 | err: `invalid value "bad=wrong" for flag --constraints: unknown constraint "bad"`, | 292 | err: `invalid value "bad=wrong" for flag --constraints: unknown constraint "bad"`, |
| 640 | 293 | }, { | 293 | }, { |
| 641 | 294 | info: "bad --series", | 294 | info: "bad --series", |
| 644 | 295 | args: []string{"--series", "bad1"}, | 295 | args: []string{"--series", "1bad1"}, |
| 645 | 296 | err: `invalid value "bad1" for flag --series: invalid series name "bad1"`, | 296 | err: `invalid value "1bad1" for flag --series: invalid series name "1bad1"`, |
| 646 | 297 | }, { | 297 | }, { |
| 647 | 298 | info: "lonely --series", | 298 | info: "lonely --series", |
| 648 | 299 | args: []string{"--series", "fine"}, | 299 | args: []string{"--series", "fine"}, |
| 649 | 300 | 300 | ||
| 650 | === modified file 'environs/jujutest/livetests.go' | |||
| 651 | --- environs/jujutest/livetests.go 2014-03-14 02:19:35 +0000 | |||
| 652 | +++ environs/jujutest/livetests.go 2014-03-20 19:57:15 +0000 | |||
| 653 | @@ -450,7 +450,7 @@ | |||
| 654 | 450 | c.Logf("deploying service") | 450 | c.Logf("deploying service") |
| 655 | 451 | repoDir := c.MkDir() | 451 | repoDir := c.MkDir() |
| 656 | 452 | url := coretesting.Charms.ClonedURL(repoDir, mtools0.Version.Series, "dummy") | 452 | url := coretesting.Charms.ClonedURL(repoDir, mtools0.Version.Series, "dummy") |
| 658 | 453 | sch, err := conn.PutCharm(url, &charm.LocalRepository{repoDir}, false) | 453 | sch, err := conn.PutCharm(url, &charm.LocalRepository{Path: repoDir}, false) |
| 659 | 454 | c.Assert(err, gc.IsNil) | 454 | c.Assert(err, gc.IsNil) |
| 660 | 455 | svc, err := conn.State.AddService("dummy", "user-admin", sch) | 455 | svc, err := conn.State.AddService("dummy", "user-admin", sch) |
| 661 | 456 | c.Assert(err, gc.IsNil) | 456 | c.Assert(err, gc.IsNil) |
| 662 | 457 | 457 | ||
| 663 | === modified file 'juju/conn_test.go' | |||
| 664 | --- juju/conn_test.go 2014-03-17 22:42:51 +0000 | |||
| 665 | +++ juju/conn_test.go 2014-03-20 19:57:15 +0000 | |||
| 666 | @@ -337,10 +337,12 @@ | |||
| 667 | 337 | // Invent a URL that points to the bundled charm, and | 337 | // Invent a URL that points to the bundled charm, and |
| 668 | 338 | // test putting that. | 338 | // test putting that. |
| 669 | 339 | curl := &charm.URL{ | 339 | curl := &charm.URL{ |
| 674 | 340 | Schema: "local", | 340 | Reference: charm.Reference{ |
| 675 | 341 | Series: "quantal", | 341 | Schema: "local", |
| 676 | 342 | Name: "riak", | 342 | Name: "riak", |
| 677 | 343 | Revision: -1, | 343 | Revision: -1, |
| 678 | 344 | }, | ||
| 679 | 345 | Series: "quantal", | ||
| 680 | 344 | } | 346 | } |
| 681 | 345 | _, err = s.conn.PutCharm(curl, s.repo, true) | 347 | _, err = s.conn.PutCharm(curl, s.repo, true) |
| 682 | 346 | c.Assert(err, gc.ErrorMatches, `cannot increment revision of charm "local:quantal/riak-7": not a directory`) | 348 | c.Assert(err, gc.ErrorMatches, `cannot increment revision of charm "local:quantal/riak-7": not a directory`) |
| 683 | @@ -356,7 +358,7 @@ | |||
| 684 | 356 | } | 358 | } |
| 685 | 357 | 359 | ||
| 686 | 358 | func (s *ConnSuite) TestPutCharmUpload(c *gc.C) { | 360 | func (s *ConnSuite) TestPutCharmUpload(c *gc.C) { |
| 688 | 359 | repo := &charm.LocalRepository{c.MkDir()} | 361 | repo := &charm.LocalRepository{Path: c.MkDir()} |
| 689 | 360 | curl := coretesting.Charms.ClonedURL(repo.Path, "quantal", "riak") | 362 | curl := coretesting.Charms.ClonedURL(repo.Path, "quantal", "riak") |
| 690 | 361 | 363 | ||
| 691 | 362 | // Put charm for the first time. | 364 | // Put charm for the first time. |
| 692 | 363 | 365 | ||
| 693 | === modified file 'state/apiserver/charms.go' | |||
| 694 | --- state/apiserver/charms.go 2014-03-13 05:57:38 +0000 | |||
| 695 | +++ state/apiserver/charms.go 2014-03-20 19:57:15 +0000 | |||
| 696 | @@ -197,10 +197,12 @@ | |||
| 697 | 197 | } | 197 | } |
| 698 | 198 | // We got it, now let's reserve a charm URL for it in state. | 198 | // We got it, now let's reserve a charm URL for it in state. |
| 699 | 199 | archiveURL := &charm.URL{ | 199 | archiveURL := &charm.URL{ |
| 704 | 200 | Schema: "local", | 200 | Reference: charm.Reference{ |
| 705 | 201 | Series: series, | 201 | Schema: "local", |
| 706 | 202 | Name: archive.Meta().Name, | 202 | Name: archive.Meta().Name, |
| 707 | 203 | Revision: archive.Revision(), | 203 | Revision: archive.Revision(), |
| 708 | 204 | }, | ||
| 709 | 205 | Series: series, | ||
| 710 | 204 | } | 206 | } |
| 711 | 205 | preparedURL, err := h.state.PrepareLocalCharmUpload(archiveURL) | 207 | preparedURL, err := h.state.PrepareLocalCharmUpload(archiveURL) |
| 712 | 206 | if err != nil { | 208 | if err != nil { |
| 713 | 207 | 209 | ||
| 714 | === modified file 'state/apiserver/client/client_test.go' | |||
| 715 | --- state/apiserver/client/client_test.go 2014-03-19 03:18:41 +0000 | |||
| 716 | +++ state/apiserver/client/client_test.go 2014-03-20 19:57:15 +0000 | |||
| 717 | @@ -220,7 +220,12 @@ | |||
| 718 | 220 | { | 220 | { |
| 719 | 221 | about: "invalid URL", | 221 | about: "invalid URL", |
| 720 | 222 | url: "not-valid", | 222 | url: "not-valid", |
| 722 | 223 | err: `charm URL has invalid schema: "not-valid"`, | 223 | err: "charm url series is not resolved", |
| 723 | 224 | }, | ||
| 724 | 225 | { | ||
| 725 | 226 | about: "invalid schema", | ||
| 726 | 227 | url: "not-valid:your-arguments", | ||
| 727 | 228 | err: `charm URL has invalid schema: "not-valid:your-arguments"`, | ||
| 728 | 224 | }, | 229 | }, |
| 729 | 225 | { | 230 | { |
| 730 | 226 | about: "unknown charm", | 231 | about: "unknown charm", |
| 731 | @@ -661,8 +666,8 @@ | |||
| 732 | 661 | defer restore() | 666 | defer restore() |
| 733 | 662 | for url, expect := range map[string]string{ | 667 | for url, expect := range map[string]string{ |
| 734 | 663 | // TODO(fwereade) make these errors consistent one day. | 668 | // TODO(fwereade) make these errors consistent one day. |
| 737 | 664 | "wordpress": `charm URL has invalid schema: "wordpress"`, | 669 | "wordpress": "charm url series is not resolved", |
| 738 | 665 | "cs:wordpress": `charm URL without series: "cs:wordpress"`, | 670 | "cs:wordpress": "charm url series is not resolved", |
| 739 | 666 | "cs:precise/wordpress": "charm url must include revision", | 671 | "cs:precise/wordpress": "charm url must include revision", |
| 740 | 667 | "cs:precise/wordpress-999999": `cannot download charm ".*": charm not found in mock store: cs:precise/wordpress-999999`, | 672 | "cs:precise/wordpress-999999": `cannot download charm ".*": charm not found in mock store: cs:precise/wordpress-999999`, |
| 741 | 668 | } { | 673 | } { |
| 742 | @@ -839,8 +844,8 @@ | |||
| 743 | 839 | s.AddTestingService(c, "wordpress", s.AddTestingCharm(c, "wordpress")) | 844 | s.AddTestingService(c, "wordpress", s.AddTestingCharm(c, "wordpress")) |
| 744 | 840 | for charmUrl, expect := range map[string]string{ | 845 | for charmUrl, expect := range map[string]string{ |
| 745 | 841 | // TODO(fwereade,Makyo) make these errors consistent one day. | 846 | // TODO(fwereade,Makyo) make these errors consistent one day. |
| 748 | 842 | "wordpress": `charm URL has invalid schema: "wordpress"`, | 847 | "wordpress": "charm url series is not resolved", |
| 749 | 843 | "cs:wordpress": `charm URL without series: "cs:wordpress"`, | 848 | "cs:wordpress": "charm url series is not resolved", |
| 750 | 844 | "cs:precise/wordpress": "charm url must include revision", | 849 | "cs:precise/wordpress": "charm url must include revision", |
| 751 | 845 | "cs:precise/wordpress-999999": `cannot download charm ".*": charm not found in mock store: cs:precise/wordpress-999999`, | 850 | "cs:precise/wordpress-999999": `cannot download charm ".*": charm not found in mock store: cs:precise/wordpress-999999`, |
| 752 | 846 | } { | 851 | } { |
| 753 | @@ -1073,8 +1078,8 @@ | |||
| 754 | 1073 | s.AddTestingService(c, "wordpress", s.AddTestingCharm(c, "wordpress")) | 1078 | s.AddTestingService(c, "wordpress", s.AddTestingCharm(c, "wordpress")) |
| 755 | 1074 | for url, expect := range map[string]string{ | 1079 | for url, expect := range map[string]string{ |
| 756 | 1075 | // TODO(fwereade,Makyo) make these errors consistent one day. | 1080 | // TODO(fwereade,Makyo) make these errors consistent one day. |
| 759 | 1076 | "wordpress": `charm URL has invalid schema: "wordpress"`, | 1081 | "wordpress": "charm url series is not resolved", |
| 760 | 1077 | "cs:wordpress": `charm URL without series: "cs:wordpress"`, | 1082 | "cs:wordpress": "charm url series is not resolved", |
| 761 | 1078 | "cs:precise/wordpress": "charm url must include revision", | 1083 | "cs:precise/wordpress": "charm url must include revision", |
| 762 | 1079 | "cs:precise/wordpress-999999": `cannot download charm ".*": charm not found in mock store: cs:precise/wordpress-999999`, | 1084 | "cs:precise/wordpress-999999": `cannot download charm ".*": charm not found in mock store: cs:precise/wordpress-999999`, |
| 763 | 1080 | } { | 1085 | } { |
| 764 | @@ -1839,8 +1844,8 @@ | |||
| 765 | 1839 | 1844 | ||
| 766 | 1840 | client := s.APIState.Client() | 1845 | client := s.APIState.Client() |
| 767 | 1841 | // First test the sanity checks. | 1846 | // First test the sanity checks. |
| 770 | 1842 | err := client.AddCharm(&charm.URL{Name: "nonsense"}) | 1847 | err := client.AddCharm(&charm.URL{Reference: charm.Reference{Name: "nonsense"}}) |
| 771 | 1843 | c.Assert(err, gc.ErrorMatches, `charm URL has invalid schema: ":/nonsense-0"`) | 1848 | c.Assert(err, gc.ErrorMatches, `charm URL has invalid schema: ":nonsense-0"`) |
| 772 | 1844 | err = client.AddCharm(charm.MustParseURL("local:precise/dummy")) | 1849 | err = client.AddCharm(charm.MustParseURL("local:precise/dummy")) |
| 773 | 1845 | c.Assert(err, gc.ErrorMatches, "only charm store charm URLs are supported, with cs: schema") | 1850 | c.Assert(err, gc.ErrorMatches, "only charm store charm URLs are supported, with cs: schema") |
| 774 | 1846 | err = client.AddCharm(charm.MustParseURL("cs:precise/wordpress")) | 1851 | err = client.AddCharm(charm.MustParseURL("cs:precise/wordpress")) |
| 775 | 1847 | 1852 | ||
| 776 | === modified file 'store/lpad.go' | |||
| 777 | --- store/lpad.go 2013-07-09 10:32:23 +0000 | |||
| 778 | +++ store/lpad.go 2014-03-20 19:57:15 +0000 | |||
| 779 | @@ -69,7 +69,10 @@ | |||
| 780 | 69 | urls := []*charm.URL{curl} | 69 | urls := []*charm.URL{curl} |
| 781 | 70 | schema, name := curl.Schema, curl.Name | 70 | schema, name := curl.Schema, curl.Name |
| 782 | 71 | for _, series := range tip.OfficialSeries { | 71 | for _, series := range tip.OfficialSeries { |
| 784 | 72 | curl = &charm.URL{Schema: schema, Name: name, Series: series, Revision: -1} | 72 | curl = &charm.URL{ |
| 785 | 73 | Reference: charm.Reference{Schema: schema, Name: name, Revision: -1}, | ||
| 786 | 74 | Series: series, | ||
| 787 | 75 | } | ||
| 788 | 73 | curl.Series = series | 76 | curl.Series = series |
| 789 | 74 | curl.User = "" | 77 | curl.User = "" |
| 790 | 75 | urls = append(urls, curl) | 78 | urls = append(urls, curl) |
| 791 | 76 | 79 | ||
| 792 | === modified file 'store/server.go' | |||
| 793 | --- store/server.go 2013-11-12 14:39:29 +0000 | |||
| 794 | +++ store/server.go 2014-03-20 19:57:15 +0000 | |||
| 795 | @@ -16,6 +16,8 @@ | |||
| 796 | 16 | "launchpad.net/juju-core/log" | 16 | "launchpad.net/juju-core/log" |
| 797 | 17 | ) | 17 | ) |
| 798 | 18 | 18 | ||
| 799 | 19 | const DefaultSeries = "precise" | ||
| 800 | 20 | |||
| 801 | 19 | // Server is an http.Handler that serves the HTTP API of juju | 21 | // Server is an http.Handler that serves the HTTP API of juju |
| 802 | 20 | // so that juju clients can retrieve published charms. | 22 | // so that juju clients can retrieve published charms. |
| 803 | 21 | type Server struct { | 23 | type Server struct { |
| 804 | @@ -74,6 +76,21 @@ | |||
| 805 | 74 | return []string{kind, curl.Series, curl.Name, curl.User} | 76 | return []string{kind, curl.Series, curl.Name, curl.User} |
| 806 | 75 | } | 77 | } |
| 807 | 76 | 78 | ||
| 808 | 79 | func (s *Server) resolveSeries(ref charm.Reference) *charm.URL { | ||
| 809 | 80 | return &charm.URL{Reference: ref, Series: DefaultSeries} | ||
| 810 | 81 | } | ||
| 811 | 82 | |||
| 812 | 83 | func (s *Server) resolveURL(url string) (*charm.URL, error) { | ||
| 813 | 84 | ref, series, err := charm.ParseReference(url) | ||
| 814 | 85 | if err != nil { | ||
| 815 | 86 | return nil, err | ||
| 816 | 87 | } | ||
| 817 | 88 | if series == "" { | ||
| 818 | 89 | return &charm.URL{Reference: ref, Series: DefaultSeries}, nil | ||
| 819 | 90 | } | ||
| 820 | 91 | return &charm.URL{Reference: ref, Series: series}, nil | ||
| 821 | 92 | } | ||
| 822 | 93 | |||
| 823 | 77 | func (s *Server) serveInfo(w http.ResponseWriter, r *http.Request) { | 94 | func (s *Server) serveInfo(w http.ResponseWriter, r *http.Request) { |
| 824 | 78 | if r.URL.Path != "/charm-info" { | 95 | if r.URL.Path != "/charm-info" { |
| 825 | 79 | w.WriteHeader(http.StatusNotFound) | 96 | w.WriteHeader(http.StatusNotFound) |
| 826 | @@ -84,7 +101,7 @@ | |||
| 827 | 84 | for _, url := range r.Form["charms"] { | 101 | for _, url := range r.Form["charms"] { |
| 828 | 85 | c := &charm.InfoResponse{} | 102 | c := &charm.InfoResponse{} |
| 829 | 86 | response[url] = c | 103 | response[url] = c |
| 831 | 87 | curl, err := charm.ParseURL(url) | 104 | curl, err := s.resolveURL(url) |
| 832 | 88 | var info *CharmInfo | 105 | var info *CharmInfo |
| 833 | 89 | if err == nil { | 106 | if err == nil { |
| 834 | 90 | info, err = s.store.CharmInfo(curl) | 107 | info, err = s.store.CharmInfo(curl) |
| 835 | @@ -92,6 +109,7 @@ | |||
| 836 | 92 | var skey []string | 109 | var skey []string |
| 837 | 93 | if err == nil { | 110 | if err == nil { |
| 838 | 94 | skey = charmStatsKey(curl, "charm-info") | 111 | skey = charmStatsKey(curl, "charm-info") |
| 839 | 112 | c.CanonicalURL = curl.String() | ||
| 840 | 95 | c.Sha256 = info.BundleSha256() | 113 | c.Sha256 = info.BundleSha256() |
| 841 | 96 | c.Revision = info.Revision() | 114 | c.Revision = info.Revision() |
| 842 | 97 | c.Digest = info.Digest() | 115 | c.Digest = info.Digest() |
| 843 | @@ -132,7 +150,7 @@ | |||
| 844 | 132 | } | 150 | } |
| 845 | 133 | c := &charm.EventResponse{} | 151 | c := &charm.EventResponse{} |
| 846 | 134 | response[url] = c | 152 | response[url] = c |
| 848 | 135 | curl, err := charm.ParseURL(url) | 153 | curl, err := s.resolveURL(url) |
| 849 | 136 | var event *CharmEvent | 154 | var event *CharmEvent |
| 850 | 137 | if err == nil { | 155 | if err == nil { |
| 851 | 138 | event, err = s.store.CharmEvent(curl, digest) | 156 | event, err = s.store.CharmEvent(curl, digest) |
| 852 | @@ -169,7 +187,7 @@ | |||
| 853 | 169 | if !strings.HasPrefix(r.URL.Path, "/charm/") { | 187 | if !strings.HasPrefix(r.URL.Path, "/charm/") { |
| 854 | 170 | panic("serveCharm: bad url") | 188 | panic("serveCharm: bad url") |
| 855 | 171 | } | 189 | } |
| 857 | 172 | curl, err := charm.ParseURL("cs:" + r.URL.Path[len("/charm/"):]) | 190 | curl, err := s.resolveURL("cs:" + r.URL.Path[len("/charm/"):]) |
| 858 | 173 | if err != nil { | 191 | if err != nil { |
| 859 | 174 | w.WriteHeader(http.StatusNotFound) | 192 | w.WriteHeader(http.StatusNotFound) |
| 860 | 175 | return | 193 | return |
| 861 | 176 | 194 | ||
| 862 | === modified file 'store/server_test.go' | |||
| 863 | --- store/server_test.go 2013-11-12 14:39:29 +0000 | |||
| 864 | +++ store/server_test.go 2014-03-20 19:57:15 +0000 | |||
| 865 | @@ -21,7 +21,7 @@ | |||
| 866 | 21 | ) | 21 | ) |
| 867 | 22 | 22 | ||
| 868 | 23 | func (s *StoreSuite) prepareServer(c *gc.C) (*store.Server, *charm.URL) { | 23 | func (s *StoreSuite) prepareServer(c *gc.C) (*store.Server, *charm.URL) { |
| 870 | 24 | curl := charm.MustParseURL("cs:oneiric/wordpress") | 24 | curl := charm.MustParseURL("cs:precise/wordpress") |
| 871 | 25 | pub, err := s.store.CharmPublisher([]*charm.URL{curl}, "some-digest") | 25 | pub, err := s.store.CharmPublisher([]*charm.URL{curl}, "some-digest") |
| 872 | 26 | c.Assert(err, gc.IsNil) | 26 | c.Assert(err, gc.IsNil) |
| 873 | 27 | err = pub.Publish(&FakeCharmDir{}) | 27 | err = pub.Publish(&FakeCharmDir{}) |
| 874 | @@ -37,10 +37,12 @@ | |||
| 875 | 37 | req, err := http.NewRequest("GET", "/charm-info", nil) | 37 | req, err := http.NewRequest("GET", "/charm-info", nil) |
| 876 | 38 | c.Assert(err, gc.IsNil) | 38 | c.Assert(err, gc.IsNil) |
| 877 | 39 | 39 | ||
| 882 | 40 | var tests = []struct{ url, sha, digest, err string }{ | 40 | var tests = []struct{ url, canonical, sha, digest, err string }{ |
| 883 | 41 | {curl.String(), fakeRevZeroSha, "some-digest", ""}, | 41 | {curl.String(), curl.String(), fakeRevZeroSha, "some-digest", ""}, |
| 884 | 42 | {"cs:oneiric/non-existent", "", "", "entry not found"}, | 42 | {"cs:oneiric/non-existent", "", "", "", "entry not found"}, |
| 885 | 43 | {"cs:bad", "", "", `charm URL without series: "cs:bad"`}, | 43 | {"cs:wordpress", curl.String(), fakeRevZeroSha, "some-digest", ""}, |
| 886 | 44 | {"cs:/bad", "", "", "", `charm URL has invalid series: "cs:/bad"`}, | ||
| 887 | 45 | {"gopher:archie-server", "", "", "", `charm URL has invalid schema: "gopher:archie-server"`}, | ||
| 888 | 44 | } | 46 | } |
| 889 | 45 | 47 | ||
| 890 | 46 | for _, t := range tests { | 48 | for _, t := range tests { |
| 891 | @@ -51,9 +53,10 @@ | |||
| 892 | 51 | expected := make(map[string]interface{}) | 53 | expected := make(map[string]interface{}) |
| 893 | 52 | if t.sha != "" { | 54 | if t.sha != "" { |
| 894 | 53 | expected[t.url] = map[string]interface{}{ | 55 | expected[t.url] = map[string]interface{}{ |
| 898 | 54 | "revision": float64(0), | 56 | "canonical-url": t.canonical, |
| 899 | 55 | "sha256": t.sha, | 57 | "revision": float64(0), |
| 900 | 56 | "digest": t.digest, | 58 | "sha256": t.sha, |
| 901 | 59 | "digest": t.digest, | ||
| 902 | 57 | } | 60 | } |
| 903 | 58 | } else { | 61 | } else { |
| 904 | 59 | expected[t.url] = map[string]interface{}{ | 62 | expected[t.url] = map[string]interface{}{ |
| 905 | @@ -64,11 +67,12 @@ | |||
| 906 | 64 | obtained := map[string]interface{}{} | 67 | obtained := map[string]interface{}{} |
| 907 | 65 | err = json.NewDecoder(rec.Body).Decode(&obtained) | 68 | err = json.NewDecoder(rec.Body).Decode(&obtained) |
| 908 | 66 | c.Assert(err, gc.IsNil) | 69 | c.Assert(err, gc.IsNil) |
| 910 | 67 | c.Assert(obtained, gc.DeepEquals, expected) | 70 | c.Assert(obtained, gc.DeepEquals, expected, gc.Commentf("URL: %s", t.url)) |
| 911 | 68 | c.Assert(rec.Header().Get("Content-Type"), gc.Equals, "application/json") | 71 | c.Assert(rec.Header().Get("Content-Type"), gc.Equals, "application/json") |
| 912 | 69 | } | 72 | } |
| 913 | 70 | 73 | ||
| 915 | 71 | s.checkCounterSum(c, []string{"charm-info", curl.Series, curl.Name}, false, 1) | 74 | // 2 charm-info events, one for resolved URL, one for the reference. |
| 916 | 75 | s.checkCounterSum(c, []string{"charm-info", curl.Series, curl.Name}, false, 2) | ||
| 917 | 72 | s.checkCounterSum(c, []string{"charm-missing", "oneiric", "non-existent"}, false, 1) | 76 | s.checkCounterSum(c, []string{"charm-missing", "oneiric", "non-existent"}, false, 1) |
| 918 | 73 | } | 77 | } |
| 919 | 74 | 78 | ||
| 920 | 75 | 79 | ||
| 921 | === modified file 'testing/charm.go' | |||
| 922 | --- testing/charm.go 2014-03-07 13:56:22 +0000 | |||
| 923 | +++ testing/charm.go 2014-03-20 19:57:15 +0000 | |||
| 924 | @@ -96,10 +96,12 @@ | |||
| 925 | 96 | } | 96 | } |
| 926 | 97 | clone(dst, r.DirPath(name)) | 97 | clone(dst, r.DirPath(name)) |
| 927 | 98 | return &charm.URL{ | 98 | return &charm.URL{ |
| 932 | 99 | Schema: "local", | 99 | Reference: charm.Reference{ |
| 933 | 100 | Series: series, | 100 | Schema: "local", |
| 934 | 101 | Name: name, | 101 | Name: name, |
| 935 | 102 | Revision: -1, | 102 | Revision: -1, |
| 936 | 103 | }, | ||
| 937 | 104 | Series: series, | ||
| 938 | 103 | } | 105 | } |
| 939 | 104 | } | 106 | } |
| 940 | 105 | 107 | ||
| 941 | @@ -126,9 +128,10 @@ | |||
| 942 | 126 | // MockCharmStore implements charm.Repository and is used to isolate tests | 128 | // MockCharmStore implements charm.Repository and is used to isolate tests |
| 943 | 127 | // that would otherwise need to hit the real charm store. | 129 | // that would otherwise need to hit the real charm store. |
| 944 | 128 | type MockCharmStore struct { | 130 | type MockCharmStore struct { |
| 948 | 129 | charms map[string]map[int]*charm.Bundle | 131 | charms map[string]map[int]*charm.Bundle |
| 949 | 130 | AuthAttrs string | 132 | AuthAttrs string |
| 950 | 131 | TestMode bool | 133 | TestMode bool |
| 951 | 134 | DefaultSeries string | ||
| 952 | 132 | } | 135 | } |
| 953 | 133 | 136 | ||
| 954 | 134 | func NewMockCharmStore() *MockCharmStore { | 137 | func NewMockCharmStore() *MockCharmStore { |
| 955 | @@ -145,6 +148,18 @@ | |||
| 956 | 145 | return s | 148 | return s |
| 957 | 146 | } | 149 | } |
| 958 | 147 | 150 | ||
| 959 | 151 | func (s *MockCharmStore) WithDefaultSeries(series string) charm.Repository { | ||
| 960 | 152 | s.DefaultSeries = series | ||
| 961 | 153 | return s | ||
| 962 | 154 | } | ||
| 963 | 155 | |||
| 964 | 156 | func (s *MockCharmStore) Resolve(ref charm.Reference) (*charm.URL, error) { | ||
| 965 | 157 | if s.DefaultSeries == "" { | ||
| 966 | 158 | return nil, fmt.Errorf("missing default series, cannot resolve charm url: %q", ref) | ||
| 967 | 159 | } | ||
| 968 | 160 | return &charm.URL{Reference: ref, Series: s.DefaultSeries}, nil | ||
| 969 | 161 | } | ||
| 970 | 162 | |||
| 971 | 148 | // SetCharm adds and removes charms in s. The affected charm is identified by | 163 | // SetCharm adds and removes charms in s. The affected charm is identified by |
| 972 | 149 | // charmURL, which must be revisioned. If bundle is nil, the charm will be | 164 | // charmURL, which must be revisioned. If bundle is nil, the charm will be |
| 973 | 150 | // removed; otherwise, it will be stored. It is an error to store a bundle | 165 | // removed; otherwise, it will be stored. It is an error to store a bundle |
Reviewers: mp+211389_
Message:
Please take a look.
Description:
Resolve implicit series with charm store.
Charm URLs without a series are "unresolved". A charm repository can
resolve
the series. Charm store service resolves the series by adding
CanonicalURL to
the charm info if it is found & resolved. Local repository uses default
series as configured in the environment. Command-line tools interacting
with
a charm repo are responsible for making requests with resolved charm
URLs.
State server requires a resolved charm URL for such requests. Charm
store
service initially resolves series to a hard-coded value -- to be
replaced by
more advanced logic as we transition to new LTS. Updated unit tests &
mocks
accordingly.
https:/
(do not edit description out of merge proposal)
Please review this at https:/
Affected files (+222, -79 lines):
A [revision details]
M charm/charm.go
M charm/charm_test.go
M charm/repo.go
M charm/repo_test.go
M charm/testing/
M charm/url.go
M charm/url_test.go
M cmd/juju/deploy.go
M cmd/juju/publish.go
M cmd/juju/
M cmd/juju/
M cmd/juju/
M environs/
M environs/
M juju/conn_test.go
M state/apiserver
M state/apiserver
M store/server.go
M store/server_
M testing/charm.go