Opened 7 years ago

Closed 7 years ago

#2049 closed task (fixed)

Project-specific annotations

Reported by: Nicklas Nordborg Owned by: Nicklas Nordborg
Priority: critical Milestone: BASE 3.10
Component: core Version:
Keywords: Cc:

Description (last modified by Nicklas Nordborg)

This is a new feature that allows an annotation type to be configured as "project-specific". The main idea here is that annotations on items can be entered on a per-project basis. Switching to another project would make it possible to enter a different value that is only visible in that project.

At the core level this must be implemented in a way that doesn't upset existing client code (web client/extensions/plugins).

  1. This feature must be enabled per annotation type. Existing annotation types will have it disabled by default. Once this feature has been enabled it will not be possible to disabled it unless all existing project-specific annotations are first deleted.
  2. The current core API should be usable and behave more or less in a backwards compatible way for annotation types that have the option enabled. Exception: The existing API is not enough to be able to create a project-specific annotation when a default value already exists.
  3. Existing annotation values doesn't belong to any project and should be treated as default values if the project-specific setting is enabled for an annotation type.
  4. When a users is working without an active project only the default values are visible/editable for annotations that are project-specific.
  5. If an annotation has a default value but not any project-specific value when a project is active, the default value is visible and should be treated as if it was a project-specific value.
  6. Default values are not editable (unless maybe via new API methods) when a project is active. Trying to change a default value results in a project-specific value being created instead. The default value is then no longer visible and should not match queries either.

The core API needs to be updated for this behavior. The existing API will modified the default annotation values even when a project is active.

  1. Deleting a project-specific value makes the default value visible again.
  2. Deleting a default value can only be done when no project is active (unless maybe via new API methods).
  3. It is important that queries are working in a sensible way. When no project is active only default values can be searched. When a project is active both project-specific and default values are searched but default values that have been replaced with a project specific value should be ignored.
  4. If a default value exists it is not possible to "unset" the annotation in a project, only to change it to a different value.
  5. Inherited annotations always belong to the same project as the parent annotation. No exceptions! This means that inheriting a default value always create a default inherited value even if a project is active. If a project-specific value is later created on the parent item it will not automatically be inherited. This may cause some confusion since the child item may display one value (the default) but navigating to the parent item will display the project-specific value.
  6. Inheriting a project-specific value should override an earlier inherited default value.

The caching functionality used for speeding up annotation retrieval must be updated to follow the same rules as when using the regular API.

The batch functionality for speeding up annotation changes must also be updated.

Change history logging might be affected as well. Should changes be visible across projects?

Some implementation details The approach taken to implement this feature is to add two columns to the Annotations table.

  • project_id: The id of the project the annotation belongs to or 0 for default values
  • override_id: The id of the annotation that contains the default value. 0 if this annotation is a default value or if there is no default value.

When searching for annotations we can introduce filters in HQL/SQL statements:

  • where ... project_id = <pid> to find only project-specific annotations (or 0 to only find default values).
  • where ... project_id = <pid> OR (project_id = 0 and id not in (select override_id from "Annotations" where ....)) to find project-specific annotations and default values that are not overidden. The subquery may have a different where clause depending on what we are looking for (for example, all annotation in an annotation set or all annotations of a specific annotation type).

The project_id column is easy to handle since we always know if a project is active or not. The override_id column is more complicated since annotations may be created/updated/removed in any order. For example, creating a default value on an item that already has project-specific values must update the override_id column on all project-specific annotations to point to the newly created default value.

Inherited annotations are even more complex since we must go up and check the parent annotations in order to know what to set for the override_id column.

More....

Change History (30)

comment:1 by Nicklas Nordborg, 7 years ago

Owner: changed from everyone to Nicklas Nordborg
Status: newassigned

comment:2 by Nicklas Nordborg, 7 years ago

(In [7244]) References #2049: Project-specific annotations

Implemented a flag option for annotation types that enabled project-specific annotations.

comment:3 by Nicklas Nordborg, 7 years ago

(In [7245]) References #2049: Project-specific annotations

Implemented a test case designed for testing project-specific annotations. The test will create one project-specific annotation type and two projects.

  • An array design is created and a default annotation value is set
  • A project-specific annotation is set for one of the projects

Tests

  • Check that the expected value is returned by the AnnotationSet API.
  • Check that the expected value is returned by the SnapshotManager API.
  • Check that a query looking for the expected value is returning the correct item.

Of course, the tests currently fail.

comment:4 by Nicklas Nordborg, 7 years ago

(In [7246]) References #2049: Project-specific annotations

The test must also check that searching for a value that has been changed by a project-specific annotation doesn't match default values.

comment:5 by Nicklas Nordborg, 7 years ago

(In [7247]) References #2049: Project-specific annotations

Changes the order of the tests so that the query tests are made before the snapshot manager tests.

comment:6 by Nicklas Nordborg, 7 years ago

(In [7248]) References #2049: Project-specific annotations

Added a column override_id to keep a reference back to the default value for project-specific annotations that override a default value. Both this and the project_id column has been changed to NOT NULL since it will be easier to handle 0 instead of null when comparing values. Both columns are also update=false. The project id will never change. The override id may change, but that is going to be handled by batch SQL updates.

comment:7 by Nicklas Nordborg, 7 years ago

(In [7249]) References #2049: Project-specific annotations

Started to implement this in the core. It is now possible to create a project-specific annotation. Currently it only works if there is no default annotation already. However, it is possible to create a default annotation if it is done after the project-specific annotation.

The snapshot manager and queries do not understand this yet so they may experience unexpected behaviour.

comment:8 by Nicklas Nordborg, 7 years ago

(In [7250]) References #2049: Project-specific annotations

The snapshot manager should now be able to find the correct annotation values. It is still a bit difficult to test since the web interface also depend on queries which are currently not returning correct results.

comment:9 by Nicklas Nordborg, 7 years ago

(In [7251]) References #2049: Project-specific annotations

The AnnotationSet.getAnnotations() method should now work with project-specific annotations. It is mainly used in some places when editing annotations for a single item.

comment:10 by Nicklas Nordborg, 7 years ago

(In [7252]) References #2049: Project-specific annotations

AnnotationRestriction and AnnotationJoin has now been updated to work with project-specific annotations. Both are used in queries generated by list pages when searching.

The join support to handle default vs. project-specific overrides was a bit complicated since regular HQL doesn't support subquires inside the WITH statement. The workaround was to invent a way to inject native SQL into HQL queries by defining the native() function that is implemented by NativeSQLFunction class.

comment:11 by Nicklas Nordborg, 7 years ago

(In [7253]) References #2049: Project-specific annotations

Removing deprecated functionality that is too difficult to fix.

comment:12 by Nicklas Nordborg, 7 years ago

Description: modified (diff)

comment:13 by Nicklas Nordborg, 7 years ago

(In [7254]) References #2049: Project-specific annotations

Started to work on inherited annotations that are project-specific. It seems to work so far as long as project-specific annotations are inherited before default annotations (the same as for primary annotations).

comment:14 by Nicklas Nordborg, 7 years ago

(In [7255]) References #2049: Project-specific annotations

The code for keeping the "override_id" column i sync with everything was getting out of hand. A new approach collects all of this into ProjectSpecificAnnotationsManager which will do all of it's work just before the transaction is committed. All other annotationd data should already have sent to the database by Hibernate.

comment:15 by Nicklas Nordborg, 7 years ago

(In [7256]) References #2049: Project-specific annotations

If a project is deleted all project-specific annotations for that project are also deleted. This currently happens in the main transaction but in a big project with lots of annotations this might not be a good idea since it can take a long time. It might be better to just ignore that the project-specific annotations are there and then make regular cleanups instead.

comment:16 by Nicklas Nordborg, 7 years ago

(In [7257]) References #2049: Project-specific annotations

Added a check to block setting projecAnnotations=false if there are existing project-specific annotations for an annotation type. The only other option is to delete the annotations but that feels a bit too complicated.

comment:17 by Nicklas Nordborg, 7 years ago

Description: modified (diff)

comment:18 by Nicklas Nordborg, 7 years ago

(In [7258]) References #2049: Project-specific annotations

Updated the API so that it is possible to create a project-specific annotation when a default value already exists. Unfortunately it was not possible to do this in a fully backwards compatible way. This means that existing code (clients, extensions, etc.) that uses the core API must be modified to be able to create project-specific annotations when a default value already exists. If not change is made, the old code will simply modify the existing default value instead.

If not default value exists or if a project-specific annotation already exists, the existing code will work as expected.

comment:19 by Nicklas Nordborg, 7 years ago

Description: modified (diff)

comment:20 by Nicklas Nordborg, 7 years ago

(In [7259]) References #2049: Project-specific annotations

The annotation batcher has been updated with support for project-specific annotations. For annotation types without the feature enabled it should work as before.

If the feature is enabled but no project is active it should:

  • Only work with (create/update/delete) default annotations
  • Make sure that the "override_id" column is properly updated when new default values are created or deleted

If the feature is enabled and a project is active it should:

  • Only work with (create/update/delete) project-specific annotations
  • If a default annotation exists, only create a project-specific annotation if the values are different
  • Not delete the default annotation

I also realized that the changes made in [7258] are not optimal and should be changed. The problem is that if a default annotation exists the new API will always create a project-specific annotation no matter if the values have changed or not. We need a different approach and make sure that the Annotation.setValuesIfDifferent() method gets a chance to decide if a project-specific annotation should be created or not.

comment:21 by Nicklas Nordborg, 7 years ago

(In [7260]) References #2049: Project-specific annotations

Re-design of changes made in [7258]. The Annotation.setValuesIfDifferent() has been changed to handle project-specific annotation in a different way.

If the annotation is a default value for a project-specific annotation type, and a project is active the method will not modify the default instead. Instead it will locate and update an existing project-specific annotation or create a new project-specific annotation. But only if the values are different. This may be somewhat confusing to other code that is using this method since the method can return TRUE without changing the values in the current annotation.

comment:22 by Nicklas Nordborg, 7 years ago

(In [7261]) References #2049: Project-specific annotations

Updated the AnnotationSet.copyFrom() to only copy annotations for the current project (if the annotation type uses project-specific annotations).

comment:23 by Nicklas Nordborg, 7 years ago

(In [7262]) References #2049: Project-specific annotations

Cleaning up project-specific annotations after a deleted project has now been moved to the regular (daily) cleanup that is initiated from the Application.DbCleaner task.

The Project class no longer need to tell the ProjectSpecificAnnotationsManager that a project has been deleted. This will be done automatically by looking for project_id values with no matching entry in the Projects table.

The deletion process should now also work with a large number of annotations since the query will be split if it contains more than DbEngine.getMaxParametersInQuery().

comment:24 by Nicklas Nordborg, 7 years ago

(In [7263]) References #2049: Project-specific annotations

Adding index on the project_id and override_id columns to improve the performance of some queries. Since most values will have a 0 in those columns a special fix was done to create a "partial index" of only the non-zero values in PostgreSQL. This is not supported in MySQL so there a regular index is used.

comment:25 by Nicklas Nordborg, 7 years ago

(In [7276]) References #2049: Project-specific annotations

Added an update warning in the documentation regarding this feature.

comment:26 by Nicklas Nordborg, 7 years ago

(In [7279]) References #2049: Project-specific annotations

Calling HibernateUtil.flush() before HibernateUtil.commit() inside the DbControl.commit() method (see [7255]) results in different exceptions being thrown in some cases. For example, the TestUser.test_duplicate() method fails due not detecting a DatabaseException.

comment:27 by Nicklas Nordborg, 7 years ago

(In [7282]) References #2049: Project-specific annotations

We need a different query in MySQL to make sure project-specific annotations are correctly referenced. There are several other similar queries in use. This was the only one that happened to fail in the TestAnnotation test. I think we need to manually try to test that the other queries are working as well. If they don't it would be good to design test cases that trigger them to be executed.

See also http://stackoverflow.com/questions/10359534/mysql-update-with-subquery-of-same-table

Last edited 7 years ago by Nicklas Nordborg (previous) (diff)

comment:28 by Nicklas Nordborg, 7 years ago

(In [7283]) References #2049: Project-specific annotations

Updated the TestAnnotation code with tests for inheriting and removing annotations. Fixed problems with !MySQL queries.

comment:29 by Nicklas Nordborg, 7 years ago

(In [7285]) References #2049: Project-specific annotations

Fixes a minor issue in the "Edit annotation" dialog that caused it to select a different annotation than the one that was clicked on in the following circumstances:

  • A project-specific annotation was selected to be inherited from a parent item
  • The default annotation was already inherited
  • The annotation type can be used on both the parent and child item type but it has no current value on the child item

When clicking on the project-specific annotation that is about to be inherited the annotation type without a value was selected instead.

Saving seems to work as expected and re-opening the edit dialog gets rid of the problem.

The cause of the problem was that more than one entry was created with the same id (=the id of the annotation type).

comment:30 by Nicklas Nordborg, 7 years ago

Resolution: fixed
Status: assignedclosed
Note: See TracTickets for help on using tickets.