You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sling.apache.org by "Andreas Hartmann (JIRA)" <ji...@apache.org> on 2009/03/10 17:58:50 UTC

[jira] Created: (SLING-883) SlingNodeStore.newItem() adds item URI as property

SlingNodeStore.newItem() adds item URI as property
--------------------------------------------------

                 Key: SLING-883
                 URL: https://issues.apache.org/jira/browse/SLING-883
             Project: Sling
          Issue Type: Bug
          Components: Extensions
            Reporter: Andreas Hartmann
            Priority: Minor


SlingNodeStore.newItem() uses the passed item argument directly to create the Sling resource. Since the item has a "uri" field, the corresponding "uri" property of the Sling resource will be set. This causes an exception if the JCR node type doesn't support this property.

Maybe it makes sense to use a "content" field inside the item object to provide the content:

  store.newItem({
    uri: "/foo/bar/baz",
    content: {
      "jcr:primaryType": "foo:bar"
    }
  });


Index: src/main/resources/dojox/data/SlingNodeStore.js
===================================================================
--- src/main/resources/dojox/data/SlingNodeStore.js	(revision 752029)
+++ src/main/resources/dojox/data/SlingNodeStore.js	(working copy)
@@ -638,7 +638,7 @@
     
     var xhr =  xhr = dojo.xhrPost({
         url: item.uri,
-        content: item,
+        content: item.content,
         load: function(response, ioargs) {
           item.dirty = false;
           console.log("onNew");


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SLING-883) SlingNodeStore.newItem() adds item URI as property

Posted by "Rory Douglas (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SLING-883?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12680540#action_12680540 ] 

Rory Douglas commented on SLING-883:
------------------------------------

I think the problem is that non-Sling/3rd party widgets expect the item attributes to exist directly on the item, not on an attribute called "content" - in particular tables, grids etc.  Perhaps we can rather do the following:

newItem: function(/* Object? */ keywordArgs, /*Object?*/ parentInfo) {
  .......
    var item = keywordArgs;
    delete item.uri;
......

it might also make sense then to use a more obscure naming scheme for these internal housekeeping attributes (_uri, or _slingNodeStore:URI), and also allow the attribute used to be configurable on the store, to avoid conflicts with actual node item names.

> SlingNodeStore.newItem() adds item URI as property
> --------------------------------------------------
>
>                 Key: SLING-883
>                 URL: https://issues.apache.org/jira/browse/SLING-883
>             Project: Sling
>          Issue Type: Bug
>          Components: Extensions
>            Reporter: Andreas Hartmann
>            Priority: Minor
>
> SlingNodeStore.newItem() uses the passed item argument directly to create the Sling resource. Since the item has a "uri" field, the corresponding "uri" property of the Sling resource will be set. This causes an exception if the JCR node type doesn't support this property.
> Maybe it makes sense to use a "content" field inside the item object to provide the content:
>   store.newItem({
>     uri: "/foo/bar/baz",
>     content: {
>       "jcr:primaryType": "foo:bar"
>     }
>   });
> Index: src/main/resources/dojox/data/SlingNodeStore.js
> ===================================================================
> --- src/main/resources/dojox/data/SlingNodeStore.js	(revision 752029)
> +++ src/main/resources/dojox/data/SlingNodeStore.js	(working copy)
> @@ -638,7 +638,7 @@
>      
>      var xhr =  xhr = dojo.xhrPost({
>          url: item.uri,
> -        content: item,
> +        content: item.content,
>          load: function(response, ioargs) {
>            item.dirty = false;
>            console.log("onNew");

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SLING-883) SlingNodeStore.newItem() adds item URI as property

Posted by "Rory Douglas (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SLING-883?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12681821#action_12681821 ] 

Rory Douglas commented on SLING-883:
------------------------------------

Actually looking at this again, the issue with newItem() is actually improper use of dojo.data.Write API.  The docs for that API indicate the first argument is not an item, but just a keywordArgs object whose attributes are intended to be the attributes on the new item.

It's the store's responsibility to provide items, to validate that objects are items, and to provide callers with a list of attributes & attribute values given an item.  So the internal structure of a returned item is intended to be a blackbox to widgets & calling code & shouldn't be relied upon directly.

If you want to create a new item that's a copy of an existing item, you need to use the store getValues/getAttributes methods to copy all the values & attribute names off the source item into a simple object, and provide that to the newItem method.  So I think the code is correct in its current form.

> SlingNodeStore.newItem() adds item URI as property
> --------------------------------------------------
>
>                 Key: SLING-883
>                 URL: https://issues.apache.org/jira/browse/SLING-883
>             Project: Sling
>          Issue Type: Bug
>          Components: Extensions
>            Reporter: Andreas Hartmann
>            Priority: Minor
>
> SlingNodeStore.newItem() uses the passed item argument directly to create the Sling resource. Since the item has a "uri" field, the corresponding "uri" property of the Sling resource will be set. This causes an exception if the JCR node type doesn't support this property.
> Maybe it makes sense to use a "content" field inside the item object to provide the content:
>   store.newItem({
>     uri: "/foo/bar/baz",
>     content: {
>       "jcr:primaryType": "foo:bar"
>     }
>   });
> Index: src/main/resources/dojox/data/SlingNodeStore.js
> ===================================================================
> --- src/main/resources/dojox/data/SlingNodeStore.js	(revision 752029)
> +++ src/main/resources/dojox/data/SlingNodeStore.js	(working copy)
> @@ -638,7 +638,7 @@
>      
>      var xhr =  xhr = dojo.xhrPost({
>          url: item.uri,
> -        content: item,
> +        content: item.content,
>          load: function(response, ioargs) {
>            item.dirty = false;
>            console.log("onNew");

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SLING-883) SlingNodeStore.newItem() adds item URI as property

Posted by "Andreas Hartmann (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SLING-883?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12680792#action_12680792 ] 

Andreas Hartmann commented on SLING-883:
----------------------------------------

This makes sense. I patched my sandbox accordingly.

I'm not sure about the configurability, though. Maybe an obscure name would be sufficient (convention over configuration), but I'm +0 for making it configurable.

> SlingNodeStore.newItem() adds item URI as property
> --------------------------------------------------
>
>                 Key: SLING-883
>                 URL: https://issues.apache.org/jira/browse/SLING-883
>             Project: Sling
>          Issue Type: Bug
>          Components: Extensions
>            Reporter: Andreas Hartmann
>            Priority: Minor
>
> SlingNodeStore.newItem() uses the passed item argument directly to create the Sling resource. Since the item has a "uri" field, the corresponding "uri" property of the Sling resource will be set. This causes an exception if the JCR node type doesn't support this property.
> Maybe it makes sense to use a "content" field inside the item object to provide the content:
>   store.newItem({
>     uri: "/foo/bar/baz",
>     content: {
>       "jcr:primaryType": "foo:bar"
>     }
>   });
> Index: src/main/resources/dojox/data/SlingNodeStore.js
> ===================================================================
> --- src/main/resources/dojox/data/SlingNodeStore.js	(revision 752029)
> +++ src/main/resources/dojox/data/SlingNodeStore.js	(working copy)
> @@ -638,7 +638,7 @@
>      
>      var xhr =  xhr = dojo.xhrPost({
>          url: item.uri,
> -        content: item,
> +        content: item.content,
>          load: function(response, ioargs) {
>            item.dirty = false;
>            console.log("onNew");

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (SLING-883) SlingNodeStore.newItem() adds item URI as property

Posted by "Andreas Hartmann (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SLING-883?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Andreas Hartmann resolved SLING-883.
------------------------------------

    Resolution: Invalid

Rory, thanks for clarifying!

> SlingNodeStore.newItem() adds item URI as property
> --------------------------------------------------
>
>                 Key: SLING-883
>                 URL: https://issues.apache.org/jira/browse/SLING-883
>             Project: Sling
>          Issue Type: Bug
>          Components: Extensions
>            Reporter: Andreas Hartmann
>            Priority: Minor
>
> SlingNodeStore.newItem() uses the passed item argument directly to create the Sling resource. Since the item has a "uri" field, the corresponding "uri" property of the Sling resource will be set. This causes an exception if the JCR node type doesn't support this property.
> Maybe it makes sense to use a "content" field inside the item object to provide the content:
>   store.newItem({
>     uri: "/foo/bar/baz",
>     content: {
>       "jcr:primaryType": "foo:bar"
>     }
>   });
> Index: src/main/resources/dojox/data/SlingNodeStore.js
> ===================================================================
> --- src/main/resources/dojox/data/SlingNodeStore.js	(revision 752029)
> +++ src/main/resources/dojox/data/SlingNodeStore.js	(working copy)
> @@ -638,7 +638,7 @@
>      
>      var xhr =  xhr = dojo.xhrPost({
>          url: item.uri,
> -        content: item,
> +        content: item.content,
>          load: function(response, ioargs) {
>            item.dirty = false;
>            console.log("onNew");

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (SLING-883) SlingNodeStore.newItem() adds item URI as property

Posted by "Andreas Hartmann (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SLING-883?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12681028#action_12681028 ] 

Andreas Hartmann commented on SLING-883:
----------------------------------------

Is the underscore syntax for the parameter name (_slingNodeStore:URI) a best practise? I've only seen it for private fields/methods by now.
Otherwise I'd just suggest to use something like sling:uri or slingResourceUri.

BTW, strictly speaking - the items correspond to Sling resources rather than (JCR) nodes, don't they? Is there even a concept of nodes in Sling? Now it might become a bit tricky from a REST point of view, where resources/URIs don't have an implicit hierarchy ...

> SlingNodeStore.newItem() adds item URI as property
> --------------------------------------------------
>
>                 Key: SLING-883
>                 URL: https://issues.apache.org/jira/browse/SLING-883
>             Project: Sling
>          Issue Type: Bug
>          Components: Extensions
>            Reporter: Andreas Hartmann
>            Priority: Minor
>
> SlingNodeStore.newItem() uses the passed item argument directly to create the Sling resource. Since the item has a "uri" field, the corresponding "uri" property of the Sling resource will be set. This causes an exception if the JCR node type doesn't support this property.
> Maybe it makes sense to use a "content" field inside the item object to provide the content:
>   store.newItem({
>     uri: "/foo/bar/baz",
>     content: {
>       "jcr:primaryType": "foo:bar"
>     }
>   });
> Index: src/main/resources/dojox/data/SlingNodeStore.js
> ===================================================================
> --- src/main/resources/dojox/data/SlingNodeStore.js	(revision 752029)
> +++ src/main/resources/dojox/data/SlingNodeStore.js	(working copy)
> @@ -638,7 +638,7 @@
>      
>      var xhr =  xhr = dojo.xhrPost({
>          url: item.uri,
> -        content: item,
> +        content: item.content,
>          load: function(response, ioargs) {
>            item.dirty = false;
>            console.log("onNew");

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Closed: (SLING-883) SlingNodeStore.newItem() adds item URI as property

Posted by "Andreas Hartmann (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SLING-883?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Andreas Hartmann closed SLING-883.
----------------------------------


> SlingNodeStore.newItem() adds item URI as property
> --------------------------------------------------
>
>                 Key: SLING-883
>                 URL: https://issues.apache.org/jira/browse/SLING-883
>             Project: Sling
>          Issue Type: Bug
>          Components: Extensions
>            Reporter: Andreas Hartmann
>            Priority: Minor
>
> SlingNodeStore.newItem() uses the passed item argument directly to create the Sling resource. Since the item has a "uri" field, the corresponding "uri" property of the Sling resource will be set. This causes an exception if the JCR node type doesn't support this property.
> Maybe it makes sense to use a "content" field inside the item object to provide the content:
>   store.newItem({
>     uri: "/foo/bar/baz",
>     content: {
>       "jcr:primaryType": "foo:bar"
>     }
>   });
> Index: src/main/resources/dojox/data/SlingNodeStore.js
> ===================================================================
> --- src/main/resources/dojox/data/SlingNodeStore.js	(revision 752029)
> +++ src/main/resources/dojox/data/SlingNodeStore.js	(working copy)
> @@ -638,7 +638,7 @@
>      
>      var xhr =  xhr = dojo.xhrPost({
>          url: item.uri,
> -        content: item,
> +        content: item.content,
>          load: function(response, ioargs) {
>            item.dirty = false;
>            console.log("onNew");

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.