Consider de-lazying lazy-seqs returned from run-as, in-tx etc. macros

[pmonks]

Both the alfresco.auth/run-as and alfresco/transact/in-tx macros (and their variants) have side effects, specifically they create ThreadLocals that change how the underlying Alfresco APIs work. For this reason returning a lazy-seq out of either of these macros is unlikely to work as expected. For example, the code:

    (alfresco.transact/in-ro-tx-as (alfresco.auth/admin) 
      (map #(alfresco.nodes/property % "cm:name") 
      (alfresco.nodes/children (alfresco.nodes/company-home))))

Throws a "missing SecureContext" error, since map creates a lazy seq, and when that lazy-seq finally gets evaluated (e.g. by the REPL), we're back outside the alfresco.transact/in-ro-tx-as macro where there is indeed no SecureContext.

The solution is to ensure forms passed into these macros do not return lazy-seqs, however this is error prone and in large codebases could be quite difficult to ensure.

It would be worth considering whether these macros should just explicitly de-lazy their response, in the case where it is a lazy-seq.

[skuro]

There's no way to ensure user code will be non lazy, unless we completely change the API. Right now, the body is executed in an implicit do, meaning code like the following is valid:

(as-admin
  (map #(property % (qname :cm/name)) (children (company-home)))
  (children company-home))

in such a scenario there's no way the as-admin macro can ensure all lazy sequences generated in the macro are wrapped in a doall. I'm not in favor of constraining the APIs to only execute one single form, as you want to aggregate multiple actions and side effect in a single batch.

I will provide samples as per how to leverage lazyness in such contexts by using closures over e.g. a SecurityContext.

[pmonks]

I don't believe all of the forms needs to be wrapped in a doall - just the final one that becomes the result (since it's the only one that can be realised later on, outside the scope of the macro - the others effectively get chucked).

Or am I misunderstanding how the multi-form case works?

[skuro]

My point is that you want to be able to have a single in-tx wrapping a number of different actions, as in (pseudo code):

(in-tx
  (map #(create-child parent %) children-seq)
  (update-properties parent {:my/prop "foobar"}))

In such a scenario you want map to complete its job, but you are not getting its lazy seq back as a return value. Wrapping the final form in a doall is of no use in such cases. This simple example can expand by adding other function calls in the in-tx body, possibly all of them lazy.

Bottom line is: it's impossible for a macro to discern which function calls are going to return lazy seq, and eventually force them. Nor it's useful to force the return value, as you might miss lazy sequences up above.

It's quite normal in the Clojure world that attention has to be payed when dealing with lazyness and side effects (e.g. Alfresco DB changes). I would still "fix" this issue with proper documentation.

[pmonks]

I understand and agree with the requirement to support multiple forms passed to run-as or in-tx. And I see why it may be necessary to "de-lazy" all of the forms, not just the last one, due to their potential use of repo-mutating function calls.

That said, I'm also fairly certain that it is indeed possible for run-as and in-tx to "de-lazy" all of the forms they're passed, not just the last one.

So while we should document this for now (and I'll update the docs in those two macros to make this clear), I'm also going to have a go at updating the macros themselves to forcibly "de-lazy" any unrealised lazy-seqs returned from any of the forms they're passed.

[pmonks]

BTW it looks like run-as and friends don't support multiple forms - is that worth fixing to be in line with in-tx et al?

[skuro]

Absolutely, as soon as I have proper network connection again I'll commit some changes to run-as and as-admin

[skuro]

I committed now the code to allow for multiple forms in the auth fns. If you're going to try and de-lazy where necessary, I would be already surprised if you could cope with such a situation:

(t/in-tx-as (a/admin)
  (let [updated-nodes (map update-node (s/search "TYPE:\"my:type\""))
        ; suppose one of them is special
        special-node (first (drop-while regular-node? updated-nodes))]
    (update-special special-node)))

In a world where the library takes care to realize the lazy sequence for you, you would expect updated-nodes to be fully realized, with all the related side effects in place.

Moreover, you should also protect the library from something like:

(t/in-tx-as (a/admin)
  (let [counts (range)]
    (map #(update-node %1 %2) nodes counts))) 

Where the programmer would expect the returned seq to be fully realized, while the infinite seq counts to have only "enough" elements being realized.

Again, I can't see this happening. But if you're still up to give it a try, go ahead :-)

[pmonks]

How would the behaviour of realizing versions of in-tx-* be worse than the the current non-realizing versions when presented with these examples?

We know the current non-realizing versions will break when presented with these two examples - both will error out with missing SecureContext errors at some undefined later time, making it very difficult for the developer to track down what they did wrong and (more importantly) where.

In comparison, the (for now hypothetical) realizing versions of in-tx-* would force the bugs in the developer's logic to surface, at the time the macro is invoked (vs at some undefined time later on).

The upshot is that these examples end up being arguments in favour of realizing versions of in-tx-*! ;-)

[pmonks]

And FWIW, I haven't had time to look further at developing realizing versions of in-tx-* and run-as-*, though I do have a sketch in my head of what's needed.

[skuro]

Well, don't get me wrong, I would be happy if the system could understand which lazy sequences need to be realized and which to leave alone. I just feel like you simply cannot, or can only handle a subset of the possible use cases. And if that's the case, although you can document that "it will de-lazy your seqs in this and that case", you open the door for subtle bugs the user will discover only the hard way, as he would expect the library to gracefully handle his lazy code.

Again, if you can come up with a solution I'll welcome the code and be happy to be proven wrong.

[pmonks]

Conditional de-lazying is the worst of both worlds - I'm definitely not advocating that.