Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MNU RBArgumentNode>>key with parameterized Seaside 3.1 REST Method #807

Closed
GoogleCodeExporter opened this issue Mar 25, 2015 · 8 comments
Closed

Comments

@GoogleCodeExporter
Copy link

Copied from Pharo issue tracker 13246

If one writes a method with at least one parameter and a <path ...> in a class 
subclassed from WARestfulHandler he gets an "MessageNotUnderstood 
RBArgumentNode>>key" error.

I think this comes from GRPharoPlatform>>argumentNamesOf:
argumentNamesOf: aCompiledMethod
^ aCompiledMethod methodNode arguments collect: [ :each | each key ]

whereas this should (maybe!?)
^ aCompiledMethod methodNode arguments collect: [ :each | each token value ]

Steps to reproduce:
// ===
a) Use a fresh image (pharo-30846.image)
b) 
Gofer new
    url:'http://www.smalltalkhub.com/mc/Seaside/MetacelloConfigurations/main';
    package: 'ConfigurationOfSeaside3';
    load.
((Smalltalk at: #ConfigurationOfSeaside3) project version: #stable) load.

c)
Gofer new
squeaksource: 'Seaside31Addons';
package: 'Seaside-REST-Core';
package: 'Seaside-Pharo-REST-Core';
package: 'Seaside-Tests-REST-Core';
load.

d) FileIn
'From Pharo3.0 of 18 March 2013 [Latest update: #30846] on 5 May 2014 at 
7:56:58.462612 pm'!
WARestfulHandler subclass: #Pharo3FogbuzReport
instanceVariableNames: ''
classVariableNames: ''
poolDictionaries: ''
category: 'GHP-Pharo'!
!Pharo3FogbuzReport commentStamp: '<historical>' prior: 0!
!

!Pharo3FogbuzReport methodsFor: 'as yet unclassified' stamp: 'GeroldPummer 
5/5/2014 19:50'!
getMessageJson: aParameter
<get>
<path: '/bug/{aParameter}'>

self halt.
^'OK'! !

"-- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- -- "!

Pharo3FogbuzReport class
instanceVariableNames: ''!
!Pharo3FogbuzReport class commentStamp: '<historical>' prior: 0!
!

!Pharo3FogbuzReport class methodsFor: 'as yet unclassified' stamp: 
'GeroldPummer 5/5/2014 19:49'!
initialize
WAAdmin register: self at: 'bugreport'! !

Pharo3FogbuzReport initialize!
// ===

e) do
ZnZincServerAdaptor startOn: 8086.

f) do
Pharo3FogbuzReport initialize.

g)
point your web-browser to http://localhost:8086/bugreport/bug/somevalue

h)
The smalltalk debugger should come up with the parameter aParameter and the 
value 'somevalue' but gives a message not understood error.


"Funny" thing on this is, I changed my argumentNamesOf: Method to the new one, 
and wrote this report. Then I changed the method back to the old method and now 
I get an error "WAArgumentNotFoundError: argument "aParameter" was not found in 
an OrderedCollection(RBArgumentNode(aParameter))". I then reproduced all with a 
fresh image.
Assigned to Everyone by Gerold Pummer 5/5/2014 8:22 PM 
Edited by Marcus Denker 7/16/2014 (Today) 1:20 PM  Edit  
The method #argumentNamesOf: should be rewritten to use 

#argumentNames

Can you move this issue to the issue tracker of Seaside?
Edited by Marcus Denker 7/16/2014 (Today) 1:21 PM  Edit  
argumentNamesOf: aCompiledMethod
^ aCompiledMethod argumentNames

isn't it nice to have a good API on CompiledMethod instead of writing a layer 
on top?
Resolved (Invalid) and assigned to Gerold Pummer by Marcus Denker 7/16/2014 
(Today) 1:24 PM 
Status changed from 'Work Needed' to 'Resolved (Invalid)'.
Milestone changed from 'Later' to 'Pharo4.0: 31/03/2015'.
Closed by Marcus Denker 7/16/2014 (Today) 1:26 PM  Edit  
I close it here as there is no action possible for Pharo. It needs to be fixed 
in Grease.
Edited by Stephan Eggermont 7/16/2014 (Today) 2:24 PM  Edit  
Some remarks:

- The Seaside31Addons are now on smalltalkhub in Seaside/Seaside31
- It is an extension method in *seaside-pharo-rest-core
- Fix needs a change in ConfigurationOfSeaside

Original issue reported on code.google.com by step...@stack.nl on 16 Jul 2014 at 12:36

@GoogleCodeExporter
Copy link
Author

Just to be sure it should be

argumentNamesOf: aCompiledMethod
^ aCompiledMethod argumentNames

not

^ aCompiledMethod methodNode arguments collect: [ :each | each token value ]

Can you provide some version information? What is the first Pharo version where 
this works? What is the first Pharo version where we need to do this? 4?

Original comment by philippe...@gmail.com on 16 Jul 2014 at 4:09

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Yes, it should be 

argumentNamesOf: aCompiledMethod
^ aCompiledMethod argumentNames

in Pharo 3 and 4

So I guess in Pharo 3 and 4 we should have a Seaside-Pharo30-REST-Core
and use that to replace the Seaside-Pharo20-REST-Core

The reference to Seaside-Pharo-REST-Core should not have been loaded, according 
to 
ConfigurationOfSeaside3>>baseline311rest:

so there we should have
    spec for: #('pharo2.x') do:[
        spec
            package: 'Seaside-Pharo20-REST-Core';
            package: 'Seaside-REST-Core' with:[
                spec includes: #('Seaside-Pharo20-REST-Core')].
    spec for: #('pharo3.x' 'pharo4.x') do:[
        spec
            package: 'Seaside-Pharo30-REST-Core';
            package: 'Seaside-REST-Core' with:[
                spec includes: #('Seaside-Pharo30-REST-Core')]

Original comment by step...@stack.nl on 16 Jul 2014 at 5:23

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Just took a look.

The code in Seaside-Pharo20-REST-Core already did exactly that. It does not 
call #argumentNames, but the implementation of 
GRPharoPlatform>>argumentNamesOf: is exactly the same as the #argumentNames 
method on a compiled method. 

This bug is because the wrong package was loaded (i.e. the package for Pharo 
1.x was loaded in a Pharo 3.0 or 2.0). 

For me this is invalid, unless the bug occurs with the correct package loaded 
but I cannot reconstruct it.

Original comment by jo...@yesplan.be on 20 Jul 2014 at 9:33

  • Changed state: Invalid
  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

GRPharoPlatform>>argumentNamesOf: uses low level details of the AST 
implementation that are not there anymore in Pharo3 and Pharo4.

We do have #argumentNames on CompiledMethod directly.

(this bug report should be re-opened)

Original comment by marcus.d...@gmail.com on 17 Aug 2014 at 7:10

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Just to infer more specifics from Marcus' comments on [Pharo-dev], this would 
seem seem to require this change... 

GRPharoPlatform>>argumentNamesOf: aCompiledMethod
^ aCompiledMethod argumentNames

Original comment by benjamin...@gmail.com on 17 Aug 2014 at 8:16

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Original comment by philippe...@gmail.com on 17 Aug 2014 at 9:56

  • Changed state: Started
  • Added labels: Platform-Pharo
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Name: Seaside-Pharo20-REST-Core-pmm.3
Author: pmm
Time: 17 August 2014, 11:04:08.79203 am
UUID: a0974771-23b3-4db9-ab0f-f44041ecac6c
Ancestors: Seaside-Pharo20-REST-Core-JohanBrichau.2

Issue 807:  MNU RBArgumentNode>>key with parameterized Seaside 3.1 REST Method
 - argumentNames is present in Pharo 2.0 as well

Original comment by philippe...@gmail.com on 17 Aug 2014 at 10:04

  • Added labels: ****
  • Removed labels: ****

@GoogleCodeExporter
Copy link
Author

Original comment by philippe...@gmail.com on 17 Aug 2014 at 10:06

  • Changed state: Fixed
  • Added labels: ****
  • Removed labels: ****

@marschall marschall added this to the 3.2 milestone Jul 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants