immutables / immutables Goto Github PK

I totally appreciate that the goal is to use the custom marshalling approach and this makes sense.

But is it also possible to make the implementation work with Jackson's ObjectMapper out of the box? This would be really useful for moving legacy code towards using Immutables (which was a real pain point when I was trying out Immutables).

That way, when you have an existing instance you can marshall it to JSON either using the Immutables Marshalling approach, or using ObjectMapper.

For example:

ValueType value = ImmutableValueType.builder().foo(42).build();

ObjectMapper om = ...
String json = om.writeValueAsString(value);

I think you can annotate the generated instance to support this in Jackson, without really hurting anything.

You can also do something similar for deserialising:

ValueType value = om.readValue(json, ImmutableValueType.class);

Obviously this isn't as nice because now you need to know about the concrete implementation.

Also, I guess you end up having a "back-door" way of creating instances that violates the builder pattern.

Maybe this could be an optional argument to the GenerateImmutable processor?

Jackson and bson4jackson upgraded well, but version of Mongo driver 2.12.3 breaks something in our internal hacks :/
For now should stick to 2.11.3 or investigate in details

Minor suggestion here.

Might it be better to have the constructor of() and builder build() methods return the value type instead of the implementation type?

e.g.

public static Value of(String foo) { ... }

instead of

public static ImmutableValue of(String foo) { ... }

The reason I suggest this is that when using Guava collection libraries it makes things a bit more complex due to type inference. e.g.:

// Doesn't work because it will create List <ImmutableValue>
List <Value> foo = ImmutableList.of(ImmutableValue.of("a"), ImmutableValue.of("b"));

// Workaround is possible but a bit ugly.
List <Value> foo = ImmutableList.of((Value)ImmutableValue.of("a"), ImmutableValue.of("b"));

My argument is that I almost never care about the ImmutableXXX implementation type. I pretty much always want to deal with the XXX type.

My guess is the most usual case is wanting XXX, so I'd rather have the library optimise for the most common case.

(PS: I'm submitting a lot of suggestions here ... please let me know if you'd rather they were just informal emails/forum posts or something :) )

John

Given an attribute of type Set<T>, there is currently no way to define a default value for it.

If I try to declare it as a Set:

  @Value.Default
  public Set<Flavor> flavors() {
    return ImmutableSet.of(Flavor.DEFAULT);
  }

compilation will fail in the generated immutable subclass because it tries to set the field to the value returned by the superclass' accessor:

  private final ImmutableSet<Flavor> flavors;
...
    this.flavors = Preconditions.checkNotNull(super.flavors());

If I try to declare it as an ImmutableSet:

  @Value.Default
  public ImmutableSet<Flavor> flavors() {
    return ImmutableSet.of(Flavor.DEFAULT);
  }

compilation will succeed, but the builder will not have the addFlavors(Flavor... flavors) generated collection methods.

Provide basic integration for JDBI using custom binding.

Currently idea is to reuse JSON marshaling mechanism if appopriate

There's idea to create repository view as a generic repository interface with simple set of operations (findAll, findById, deleteAll, delete, upsert, update). This generic repository could be passed along other repositories for some generified processing

MyRepository repository = ...

GenericRepository<MyRepository> r =  repository.asGeneric();
FluentFuture<MyRepository> doc12 = r.findById(12);

Questions left: Do we need parametrize by id or left it as Object? Specific set of exposed operations and naming.

UPDATE Expanding scope of this to be able to create repositories not only for mongodb, but for some other database technologies as well. Excerpt from the email on the mailing list:

What would be really nice would be for immutables to build a generic repository with some "most common" subset of your MongoDB builder functions that could then be easily adapted for a wider set of uses.

Repository<Bean> repo = repo.all_of().a_is("x").b_in(Arrays.asList("x", "y")).c_in_closed_open_range("z", "zx"); 

and then have a simple way of getting a list of those (including nested ones) for the person developing the mapping between that repo and whatever the end use is.

Given that we have state of art templates that provide huge amount of power and functionality, the problem is with core models and processing, which is clumsy, ad-hoc-ish and dirty. We have a lot more ideas on how to employ annotation processing and there's a clear need for advanced model and toolkit behind. It's not to say that templates doesn't require a cleanup, but this is totally different thing.

For this issue I also want to investigate the model in google's AutoValue project to see if there will be some touching points in design, which might lead to contributions both ways.

I'm testing the upgrade from 0.31-SNAPSHOT to 1.0 in the Buck repository.

We compile our code with ecj (the Eclipse Compiler for Java), and we found that the 1.0 release raises an error when compiling with ecj because the generated code uses @SuppressWarnings("unchecked") unnecessarily:

[javac] 1. ERROR in buck/build/immutables/com/facebook/buck/apple/ImmutableAppleSdkPaths.java (at line 18)
[javac]     @SuppressWarnings({"all", "unchecked"})
[javac]                               ^^^^^^^^^^^
[javac] Unnecessary @SuppressWarnings("unchecked")

I'm guessing this can be removed from the generated code (at least in the way we're using it).

Here's the source file:

http://pastebin.com/cpEprRZa

And here's the file it generates:

http://pastebin.com/rBed0978

Regression is due new source ordering extraction which consider only parameterless methods.

Generate immutable implementation of existing abstract value classes, especially interfaces and annotations [if unable to annotate them directly].
This should extends functionality of @Value.Nested. Could be @Value.Include({Annotation.class, ExternalInterface.class}).

The immutables library seems to have no solution for the Jackson @JsonIgnore annotation. I could see this implemented as an attribute of the @GenerateMarshaled annotation.

@GenerateMarshaled(ignore = true)

Would it be possible to implement such a feature?

This includes support for array attributes (using cloning on store and read)

Documentation states limitation of generation only public classes, however it easy to provide such functionality: generate non-public classes if abstract type was non-public.

manifests in duplicated generated field and accessor while it's clearly that they are compatible

Hi, I've made simplest possible immutable class with builder and got following classes in jar

My.class
ImmutableMy.class
ImmutableMy$Builder.class
ImmutableMy$1.class

(on jdk7 u45, using maven/javac)
Could we somehow get rid of additional class ImmutableMy$1.class being compiled? Is this somehow related to inner class private access bridge?

Clear methods on builders never felt like a good choice.

    /**
     * Clears all previously added elements for {@link ValueObject#numbers()} list.
     * @return {@code this} builder for chained invocation
     */
    public Builder clearNumbers() {
      numbersBuilder = ImmutableList.builder();
      return this;
    }

Guava collection builders doesn't have clear methods. It's something more about collections themselves and not about building. But there were the need for the clearAttributes method when you wanted to copy other object (by Builder#copy methods) with this attribute content being cleared or completely replaced. Now this will accomplished by with methods ( #11).

I spotted one potential issue with the JDBI bindings.

JDBIs in-built object binding supports using SELECT x AS y style SQL.

For example, you can have a code like this:

public class Foo {
    private String field;
    ...
} 

public interface MyJDBIThing {

     @MapResultAsBean
     @SqlQuery("SELECT some_badly_named_field AS field FROM foo")
     Foo getFoo();
}

However this doesn't work in Immutables MapperFactory class.

The issue seems to be in MapperFactory.generateTokens():

ResultSetMetaData metaData = result.getMetaData();
...
    String name = toLowerCamel(metaData.getColumnName(i));

The issue is that ResultSetMetaData.getColumnName() is used instead of getColumnLabel().

getColumnLabel() will return the "...AS" name or fall back to the standard column name (so it's safe for users NOT using "...AS").

I would also point out that JDBI's mapper uses getColumnLabel() - see org.skife.jdbi.v2.BeanMapper.map().

I'm happy to generate a pull request if required.

Thanks!

John

I'm trying to use Immutables for Android Project. But it crashes with NPE during compilation. Steps to reproduce:

1) Create new Android Project in Android Studio 0.8.8 (with default parameters)

2) Add to build.gradle

android {
..
    compileOptions {
        sourceCompatibility JavaVersion.VERSION_1_6
        targetCompatibility JavaVersion.VERSION_1_6
    }
}

dependencies {
    ...
    compile 'org.immutables:annotation:0.16'
    provided 'org.immutables:generate-tool:0.16'
}

3) Add dummy data class:

@GenerateImmutable
public abstract class Data {
    public abstract int number();
}

4) Click Run

Expected Behaviour: Successful compilation
Actual Result: NullPointerException during compilation:

22:38:51.122 [ERROR] [org.gradle.BuildExceptionReporter] Caused by: java.lang.NullPointerException
22:38:51.122 [ERROR] [org.gradle.BuildExceptionReporter]    at org.immutables.generate.internal.processing.Processor.inspectGenerateType(Processor.java:99)
22:38:51.122 [ERROR] [org.gradle.BuildExceptionReporter]    at org.immutables.generate.internal.processing.Processor.processAnnotations(Processor.java:82)
22:38:51.122 [ERROR] [org.gradle.BuildExceptionReporter]    at org.immutables.generate.internal.processing.Processor.process(Processor.java:70)
22:38:51.123 [ERROR] [org.gradle.BuildExceptionReporter]    at com.sun.tools.javac.processing.JavacProcessingEnvironment.callProcessor(JavacProcessingEnvironment.java:793)
22:38:51.123 [ERROR] [org.gradle.BuildExceptionReporter]    at com.sun.tools.javac.processing.JavacProcessingEnvironment.discoverAndRunProcs(JavacProcessingEnvironment.java:722)
22:38:51.123 [ERROR] [org.gradle.BuildExceptionReporter]    at com.sun.tools.javac.processing.JavacProcessingEnvironment.access$1700(JavacProcessingEnvironment.java:97)
22:38:51.124 [ERROR] [org.gradle.BuildExceptionReporter]    at com.sun.tools.javac.processing.JavacProcessingEnvironment$Round.run(JavacProcessingEnvironment.java:1029)
22:38:51.124 [ERROR] [org.gradle.BuildExceptionReporter]    at com.sun.tools.javac.processing.JavacProcessingEnvironment.doProcessing(JavacProcessingEnvironment.java:1163)
22:38:51.124 [ERROR] [org.gradle.BuildExceptionReporter]    at com.sun.tools.javac.main.JavaCompiler.processAnnotations(JavaCompiler.java:1108)
22:38:51.124 [ERROR] [org.gradle.BuildExceptionReporter]    at com.sun.tools.javac.main.JavaCompiler.compile(JavaCompiler.java:824)
22:38:51.125 [ERROR] [org.gradle.BuildExceptionReporter]    at com.sun.tools.javac.main.Main.compile(Main.java:439)
22:38:51.125 [ERROR] [org.gradle.BuildExceptionReporter]    ... 68 more

Environment: Android Studio 0.8.8, OS X 10.9.4, Java 7 (1.7.0_60)

Do you support Android? Any ideas how to fix this error?

Currently we have special logic to not use ? extends for Enums and boxed-primitives keys.

More correct way seems to be

• do not use ? extends for any final class, and this will auto-include Enums, boxed-primitives, String etc
• use this logic for element, key and value generic types uniformily

It should be not that hard to pass type arguments verbatim in couple of places.
Eventually, generics should work, let it be a bug rather than feature request.

Our code style currently depends on value types with accessors named "getFoo()" and builders with setters named "setFoo()".

I can emulate the getters in the generated types with @Value.Getters, but the top-level interface then isn't useful (it will be missing the getters).

I can't emulate the setters in generated builders as far as I can tell.

It'd be really nice if we could do something like:

@Value.Immutable
@Value.GettersAndSetters
public interface Type {
  int getFoo();
  String getBar();
}

which would generate an ImmutableType with getFoo() and getBar(), and an ImmutableType.Builder with setFoo() and setBar().

Umbrella issue for java 6 support in runtime.
Thanking to the @trask 's contributions
45c13c7 866891f 61d94c7

For one of the future versions, probably, before 1.0 (or for 1.0), there's intention to revise naming for the annotations, and package and module structure.
Driving factors for the changes:

• Need better modularization: annotations, packages, modules
• Simplicity and flexibility

While I liked initial straightforward @Generate* and .common - like approach for the toolkit, and other guys was ok with it, I no longer feel its a way to go.

Modules/packages:

• Split in large chunks: core immutables, JSON, repositories, etc.
• Annotation should go together with supporting classes for repositories and JSON.
• Decide if actual processor should go to separate modules (depends on testing with IDEs)

Annotation naming:

• Allow for both simple name and namespacing via annotation nesting. Currently don't know how it will work in practice, but currently I like this idea. Especially how it plays with different modules
• Examples: @Value.Immutable, @Value.Default, @Value.Parameter, @Value.Derived, @Json.Marshaler, @Json.Named, @Json.Ignore, @Mongo.Repository. This allows for autocompletion starting with enclosing annotation, and also allow import for simple annotation name, depending on style and mixing with other annotations.

As a "spoiler": all these exercises will be much more reasonable when new annotation processing and modeling toolkit will be ready (#17.)

Earlier, it was like a good idea to not provide any specific support for WriteConcern and ReadPreference.
In general, database defaults could be set on DB instance and repositories will just use that. Separated repositories of the same class may be configured to use different default options for WriteConcern and ReadPreference.

After some additional hands-on experience with other mongodb drivers, it is could be beneficial to add minimal support for per-operation overrides:

• safeWrite() for overriding write concern
• readSecondaries() for overriding read preference

Just as an example. insert, deleteAll, upsert etc — doesn't have configuration object, nowhere to stick this configurator. This will require some API overhaul, which is not very desirable. Not that we cannot change things, but it may complicate repository API.

It's something that we wanted long before. You usually want to have a lot of small value classes in one file when model messages and documents. In Java this naturally accomplished by nesting those classes under umbrella top level class, and we need to be able to generate immutable subclasses for nested (static inner classes). In addition we want to be able to generate immutable classes out abstract value types defined as interfaces and nested interfaces in the same manner.

Need to add Float.floatToIntBits and Double.doubleToLongBits conversions in generated equals

This I consider number one feature for the 1.0 release, even a blocker. We need an cutting edge diagnostic and error handling for any JSON marshaling and unmarshaling problems. It should be dead simple, minimal API, maximum value!

I came across this awesome library when you posted that message on the Guava mailing list the other day. Too bad they haven't answered you yet.

I would like to start using it for generating objects that are exchanged via JAX-RS in combination with the bean validation API for which Jersey has build-in support. Unfortunately that requires me to follow the JavaBean convention of prefixing the method names with 'get'/'is':

http://docs.jboss.org/hibernate/validator/5.1/reference/en-US/html_single/#d0e349

Would you consider prefixing the generated builder method names with 'set' if the method names start with 'get'/'is'? Setters with getter method names in a builder, that's just wrong.

I understand where you're coming from in eschewing this convention in your documentation (after all if there are no setters why should there by getters), but maybe in this case an exception could be made?

Anyway, thanks for sharing this great library.

As it is implemented now, classes cannot be generated from default package. We need to issue proper compilation error as we don't want to handle default package (unless people will massively ask for it)
It may be similar, but not exactly #24 issue.
Need just allow to generate things for default package, it's should not block people to try Immutables

It would be great if Immutables could serialise Json objects which are NOT themselves immutables. E.g.:

public class OldBean {
   public String getFoo() {...}
}

@Immutables.Value
public abstract class NewBean {
   public abstract OldBean getOldBean();
}

At the moment, the only way to handle this is to manually specify every old style of bean in the @Json.Import thing.

What would be great is if immutables just falls back to using a standard ObjectMapper to serialise/deserialise the objects which are not Immutables.

If a default interface method is annotated with @Value.Default the generated class contains a call to super.foo() which is invalid when calling a default method. The correct syntax would be Foo.super.foo() where Foo is the interface type.

Right now, when object builder is created, collection builders are created upfront. While generated code is pretty straightforward, it precludes following optimisations:

• Skip collection builder creation, (assing empty .of() collection in resulting object)
• Reuse single immutable collection passed in .addAll method

What could be done with collection attribute building:

1. Reuse passed or empty immutable collection when possible
2. Postpone builder creation until additional elements or non-immutable collection passed

Our mini wrapper around Hamcrest matchers was one of the first of it's kind in java, right at the time when they started to appear. Despite I saw good contenders, all of them had some flaws and that gave additional years for Hamcheck/Checkers to hang around. Now, I checked google/truth and find it more or less suitable to finally decommission Checkers.

Still, I feel that some tiny customization to google/truth need to be added, probably, to cut stack traces or make error messages a little bit better.

I also was looking into AssertJ also
No hurry BTW.

Disjunctive .or() has never been implemented. What an unfortunate omission. Need to fix that

Is should be possible to query repository by JSON-formatted string.
repository.find("{ name: '%s' }", "Nameless")
JSON format should allow unquoted identifiers and single quotes for better readability.

specify ordering

...find()
  .orderByName()
  .fetchAll()
...

actual sorting instructions are not being issues

Remarkably, Immutables have pretty decent support for the algebraic data type -like classes.
Examples:

• ADT-like types using OO inheritance and object graphs:
  https://github.com/immutables/immutables/blob/master/generator-processor/src/org/immutables/generator/processor/Trees.java
• immutable object graph transformers: https://github.com/immutables/immutables/blob/master/generator-processor/src/org/immutables/generator/processor/TypeResolver.java

Things that could be improved

1. Visitors. A well-thought-out visitor generation would be a good asset along with tree-transformers which are already implemented.
2. Documentation and examples.

The current philosophy of Immutables is to intentionally keep the number of supported containers to a minimum, but it also mentions the specific set of containers could change after discussion.

In the Buck project, we depend pretty heavily on ImmutableSortedSet in our immutable data structures which we create from filesystem listings or glob match results. We use ImmutableSortedSet so our objects behave identically regardless of the behavior of the filesystem listing order (which is OS- and filesystem-dependent).

We'd love to move these data structures to the Immutables library, but it means we have to push sorting from creation time to consumption time, which is a bit annoying (especially in tests). It'd be nice if we could do something like:

@Value.Immutable
public interface FilesystemData {
  String globPattern;
  SortedSet<String> matchingFilenames;
}

If it helps, we don't use any custom Comparators; we always use natural ordering today. (I agree it might be a bit tricky to annotate the Comparator to use for a SortedSet.)

I already thought that doing equals to check if we creating the same value may give more performance penalty that equals economy might save.

Moreover, there will be full equals check for interned object in checkPreconditions in case we need
to preserve interning.

But the thing I am thinking of is a simplistic and cheap check for the == (except, maybe, for the float and double) that may be a good trade off

Hi,

I'm just trying to write something to integrate immutables with JDBI (I will send you a copy if I can get it working :) ).

My plan was to use reflection to try and deal with the immutable.

What I've noticed is that the Value.Immutable annotation is set to SOURCE retention so it isn't visible at runtime. This makes it hard for runtime code to try and do special things with Immutables value types.

Maybe there is an argument that this kind of work should be done by a pre-processed code (like how the JSON code is generated)?

On the other hand I don't think there is any harm in making them runtime retention?

John

Only marshalers from top level value classes could be resolved in runtime (JAX-RS integration for example). Need to fix resolutions in MarshalingSupport which are out of date. In addition we should try to investigate how to improve this mapping, make it more robust, less dependent on fragile convention

Please, add the details

After the step, when it was decided to not upcast Immutable* types returned from builder and factory methods, it is quite tempting to add copy-change-single-field methods that would surface only on Immutable* API

1. Change one or small number of fields.
2. Allow move flexible way to construct objects using constructor with small number of arguments that needed often and then change some optional attributes using with methods.
3. Efficient reuse of immutable object subgraphs: regardless of #6

In MapperFactory.map() there is a case statement like this:

        case Types.DATE://$FALL-THROUGH$
        case Types.TIME://$FALL-THROUGH$
        case Types.TIMESTAMP: {
          Date object = result.getDate(i);
          if (!result.wasNull()) {
            buffer.writeFieldName(name);
            buffer.writeObject(object);
          }
          break;
        }

Unfortunately ResultSet.getDate() returns a java.sql.Date object which strips off the time information.

I think it should be something like:

        case Types.DATE:
           Date object = result.getDate(i);
            if (!result.wasNull()) {
              buffer.writeFieldName(name);
              buffer.writeObject(object);
            }
            break;
        case Types.TIME:
           Time object = result.getTime(i);
            if (!result.wasNull()) {
              buffer.writeFieldName(name);
              buffer.writeObject(object);
            }
            break
        case Types.TIMESTAMP: {
          Timestamp object = result.getTimestamp(i);
          if (!result.wasNull()) {
            buffer.writeFieldName(name);
            buffer.writeObject(object);
          }
          break;
        }

One comment from my colleagues when trying to persuade them to use Immutables to replace existing code is they don't like the use of Optional for optional fields.

Personally, I'm still undecided about this one, I can see arguments both for and against. If we were starting from a green field then I'd think I'd be happy using Optional.

The one tricky thing was that switching to Optional for legacy beans can cause really subtle bugs if you're not careful. E.g. imagine this legacy code:

class Foo {
    private String foo;
    String getFoo() {
     return foo;
    }
}


...
   if (value.getFoo() == null) {
       ... do something
   }

Now if I change to an Immutables like this...

abstract class Foo {
    abstract Optional<String> getFoo()
}

... I won't receive a compiler error, but my logic in the "if" is broken.

Could we just have, say, an @Value.Optional annotation (Or even use the @javax.annotation.Nullable one), to mark a field optional?

That way, code which uses the old nullable style can work without any changes.

Yes, I understand you're trading off one problem for another (ambiguity about when getFoo() will return null), but this might suit some people better.

Thanks!

While theres is long standing issue this the Eclipse compiler, which was highlighted in documentation: there's is a serious problem: different compilers (I hope that output will not depend on some random run times) might produce different order of elements that is used for hashcode computation and other thing [what things?].

Google's AutoValue team had developed something that is called "Eclipse Hack" to retrieve elements in definition order as javac does.

• Do we need to employ "Eclipse Hack" also
• If not
  • What else we can do?
  • Is problem as severe as I thought?

wrong check in constructor generation routine considers lazy values as ones that require value to be provided resulting in compilation error. Workaround is to use builder, not the constructor parameters @Value.Parameter

We have pretty simple import handling system that produces good generated source code, however, it is not accurate with regard to clash resolution and may fail on edge cases that depends only on a matter how other classes are named.

Need to fix this. There's no rush, but it's nice to have it someday, maybe for 1.0

Primitive numeric attributes are not considered comparable

It looks like m2e-apt project made a change which would be worth noting in the instructions at http://immutables.github.io/apt.html

See https://github.com/jbosstools/m2e-apt:

Due to some rogue annotation processors at large, we decided to disable automatic annotation processing by default.