Hi! Here is my weekly issue/pr :P

I'm opening this issue because I found something weird with fields of type dict with default values (maybe list are affected as well)

Example

I have a custom content-type like this:

class ICustomContentType(IItem):
    images = schema.Dict(
        key_type=schema.TextLine(),
        value_type=schema.TextLine(),
        required=False,
        default={}
    )

And I implemented an endpoint that adds a key-value pair to field images.

@configure.service(
    method="PATCH",
    name="@update-dict",
    permission="guillotina.AddContent"
)
async def update_default_dict(context, request):
    context.images['new-key'] = 'value'
    request._txn.register(context)
    assert len(context.images) == 0  # wat?

After this endpoint retrieve the object and the field images is {}.

BUT if I replace the previous code with context.images = {'new-key': 'value'} then everything works fine.

Is this a bug or I'm not using default value of field properly?

Reproduce

I implemented a test that reproduces the problem: https://github.com/masipcat/guillotina/commit/9be9d11a073f89bb526a7ee63aedb2f8aaddbe15

Thank you!

thoughts:

• __txn__ feels magical
• adding objects to container always means using parent.__txn__
• in order to make this work most of the time, you need to have the magic in transaction manager which would reuse txn objects. Otherwise, parents wouldn't be able to add children across transaction boundaries without reassigning __txn__ all the time. So it added weird cross dependencies
• conceptually, it's much easier to simply to txn.add(ob) -- explicit is better than implicit..

implications:

• maybe all object mutate api methods should accept a txn argument to be able to customize which txn to register it against(in case you are manually working with multiple transactions).
• I'd also like to find a nice way to auto retry conflict errors when manually working with txn objects. In practice, writing a generic api for this is difficult though.

The @items service filters out the result based on permissions. The pagination is made prior to filtering, leading to exotic paging information in the final result.

Things I'm working on:

1. Ensure that there are subscribers of events Added, Modified and Removed for User and Group (this is a continuation of @jordic PR)
2. Implement the IGroups utility for dbusers. The problem I see so far is that get_principal() needs to be a coroutine so we can load the group from groups folder and fill the GuillotinaGroup with the roles and permissions. (see https://github.com/plone/guillotina/pull/852/files#r368751920)

Instead of needing to reference complete behavior interfaces names, be able to register a behavior with an alias.

For example, a PATCH like this:

"guillotina.behaviors.dublincore.IDublinCore": {
                    "title": "foo",
                    "description": "bar",
                    "tags": ["one", "two"]
                }

Could be changed to:

"dublincore": {
                    "title": "foo",
                    "description": "bar",
                    "tags": ["one", "two"]
                }

Registering behavior would change to add "alias" option for the configurator.

Hi,

As orjson 3.x has many fixes, see https://github.com/ijl/orjson/releases, I can suggest to use this version instead of 2.6.0

Besides, it will fix https://github.com/the-benchmarker/web-frameworks/pull/3600

Regards,

Do we have any kind of automatic docker image builds? (On every commit to master, or on every new release)

We need it for guillotina_react. To be able to launch a guillotina (ideally) from a docker, and do some cypress testing on it.

Closes https://github.com/plone/guillotina/issues/424

This is the easiest approach I found but there are some alternative to this implementation:

1. Instead of storing schema.table_name in attribute _objects_table_name we can change all the sql statatements to accept the 'schema' as a parameter.
2. Execute SET SCHEMA {schema} when opening a connection
3. Ideas?

In the current implementation, get_object_by_oid load all the object's parents. It is a little expensive.

Although we can cache objects from redis, we can fix it better.

when load from get_object_by_oid, we can just use a lazy parent:

obj.__parent__ = LazyResource(obj)

The LazyResource is loaded from db or cache when required.

When a create_content_in_container is done on a object there is not validation that the id has been already choosen and the conflict apears on serializing at the DB.

In some situations in my internal code, I want to be able to switch the state of the workflow on behalf of other users without checking any security policies.

This goes in conjunction with this: https://github.com/plone/guillotina_elasticsearch/pull/105

With ES nowadays, if we want to index new fields using index_field the only way to update the mappings and settings of the index using the API of guillotina is to do a POST to @ catalog, which deletes the whole index, and creates it again. Then we need to reindex all the data calling POST @catalog-reindex. With this change, (regarding ES) when adding new index fields we can just call @update-catalog and delegate the logic (if necessary) to the catalog utility.

Bumps pillow from 9.0.1 to 9.3.0.

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.

Bumps codecov from 2.0.15 to 2.0.16.

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.

Currently, in a get (read) request, it is difficult to issue a patch (write) request. Is there a (easy) way/mechanism to make sub-requests, like plone.subrequest packages.

I measured that big objects >50kB take a lot of time to deserialize, about 50, 100, 200ms depending on the size.. This change avoids blocking all requests in the process during deserialization.

I'm not sure if we should consider this a breaking change or if it's ok as a patch version

I was reviewing this part and I don't understand why the strategy resolve_readcommitted (the default one) needs the tpc_vote logic from resolve strategy.

If I'm not wrong, these are the cases we are trying to mitigate:

1. Cache invalidation race condition: an object is modified but the next request that wants to modify the object reads the stale object. During the write to the storage, we want to detect that the old txn id is not the latest in postgres. This is already implemented in the UPDATE sql, using the WHERE tid = $7::bigint. If the WHERE clause doesn't match it would not update the object and we'll raise the ConflictError.

2. Two concurrent requests modify the same object "at the same time". At the same time could mean two things: a. Both requests read the same object, the first starts the pg transaction and writes the changes and finished. When the second transaction starts will try to update the object and I'd would happen the error in point 1. The Old TID doesn't match and the ConflictError is raised. b. Both requets read the same object, the first request starts the transaction and writes the changes. At the same time, before committing the pg transaction the second request starts a transaction and tries to write the same object. In that case two things could happen:

   1. same thing as previous points: TID mismatch -> Conflict.
   2. deadlock and both requests fail

My conclusion is that in all these situations postgres/SQL queries already handle the conflicts, and there is no need for the extra logic. The only case I could think this is useful is for Cockroach DB.

./cc @bloodbare @vangheem

Originally posted by @masipcat in https://github.com/plone/guillotina/pull/1154#discussion_r748486685